<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Akihiro,<br>
<br>
Thanks for working on that. I have a few comments to the patch.<br>
<br>
On 17.04.2017 16:26, Akihiro Sagawa wrote:<br>
</div>
<blockquote cite="mid:20170417232550.4D6A.375B48EC@gmail.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c
index 6914837..317686f 100644
--- a/dlls/secur32/schannel.c
+++ b/dlls/secur32/schannel.c
@@ -54,11 +54,14 @@ struct schan_handle
enum schan_handle_type type;
};
+#define SCHAN_KEY_NAME_MAX 8
struct schan_context
{
schan_imp_session session;
ULONG req_ctx_attr;
const CERT_CONTEXT *cert;
+ SEC_WCHAR key_signature_name[SCHAN_KEY_NAME_MAX];
+ SEC_WCHAR key_encrypt_name[SCHAN_KEY_NAME_MAX];</pre>
</div>
</blockquote>
<br>
Those should not be needed. You could for example store both WCHAR
and char strings inside alg_name_map.<br>
<br>
<blockquote cite="mid:20170417232550.4D6A.375B48EC@gmail.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
};
static struct schan_handle *schan_handle_table;
@@ -962,6 +965,53 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextA(
return ret;
}
+struct schan_alg_id_name {
+ ALG_ID alg_id;
+ const char* name;
+};
+
+static int comp_alg_id(const void *a, const void *b)
+{
+ const struct schan_alg_id_name *lhs = a;
+ const struct schan_alg_id_name *rhs = b;
+ return (int)lhs->alg_id - (int)rhs->alg_id;
+}
+
+static void fill_alg_name(ALG_ID id, void *buffer, BOOL wide)</pre>
</div>
</blockquote>
<br>
If you change the patch as I suggested above, this could be
something like:<br>
<br>
static void *get_alg_name(ALF_ID id, BOOL wide)<br>
<br>
<blockquote cite="mid:20170417232550.4D6A.375B48EC@gmail.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+{
+ static const struct schan_alg_id_name alg_name_map[] = {
+ { CALG_ECDSA, "ECDSA" },
+ { CALG_RSA_SIGN, "RSA" },
+ { CALG_DES, "DES" },
+ { CALG_RC2, "RC2" },
+ { CALG_3DES, "3DES" },
+ { CALG_AES_128, "AES" },
+ { CALG_AES_192, "AES" },
+ { CALG_AES_256, "AES" },
+ { CALG_RC4, "RC4" },
+ };
+ const struct schan_alg_id_name *res;
+ struct schan_alg_id_name key;
+ const char *name;
+
+ key.alg_id = id;
+ res = bsearch(&key, alg_name_map,
+ sizeof(alg_name_map)/sizeof(alg_name_map[0]), sizeof(alg_name_map[0]),
+ comp_alg_id);
+ if (res)
+ name = res->name;
+ else
+ {
+ TRACE("Unknown ALG_ID %04x\n", id);
+ name = "???";
+ }</pre>
</div>
</blockquote>
<br>
A FIXME would would be nice here. Also, I think you could just
return NULL in this case.<br>
<br>
<blockquote cite="mid:20170417232550.4D6A.375B48EC@gmail.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap="">
+
+ if (wide)
+ MultiByteToWideChar(CP_ACP, 0, name, -1, (SEC_WCHAR*)buffer, SCHAN_KEY_NAME_MAX);
+ else
+ strcpy((SEC_CHAR*)buffer, name);
+}
+
static SECURITY_STATUS ensure_remote_cert(struct schan_context *ctx)
{
HCERTSTORE cert_store;
@@ -989,6 +1039,7 @@ static SECURITY_STATUS SEC_ENTRY schan_QueryContextAttributesW(
if (!context_handle) return SEC_E_INVALID_HANDLE;
ctx = schan_get_object(context_handle->dwLower, SCHAN_HANDLE_CTX);
+ if (!ctx) return SEC_E_INVALID_HANDLE;</pre>
</div>
</blockquote>
<br>
This has an effect outside of scope of this patch. This should be be
a separated patch, with a test case.<br>
<br>
<br>
<blockquote cite="mid:20170417232550.4D6A.375B48EC@gmail.com"
type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-western">
<pre wrap=""> switch(attribute)
{
@@ -1016,6 +1067,19 @@ static SECURITY_STATUS SEC_ENTRY schan_QueryContextAttributesW(
return status;
}
+ case SECPKG_ATTR_KEY_INFO:
+ {
+ SecPkgContext_KeyInfoW *info = buffer;
+ SECURITY_STATUS status = schan_imp_get_key_info(ctx->session, info);
+ if (status == SEC_E_OK)
+ {
+ info->sSignatureAlgorithmName = ctx->key_signature_name;
+ info->sEncryptAlgorithmName = ctx->key_encrypt_name;
+ fill_alg_name(info->SignatureAlgorithm, info->sSignatureAlgorithmName, TRUE);
+ fill_alg_name(info->EncryptAlgorithm, info->sEncryptAlgorithmName, TRUE);
+ }
+ return status;
+ }
case SECPKG_ATTR_REMOTE_CERT_CONTEXT:
{
PCCERT_CONTEXT *cert = buffer;
@@ -1091,6 +1155,24 @@ static SECURITY_STATUS SEC_ENTRY schan_QueryContextAttributesA(
{
case SECPKG_ATTR_STREAM_SIZES:
return schan_QueryContextAttributesW(context_handle, attribute, buffer);
+ case SECPKG_ATTR_KEY_INFO:
+ {
+ SecPkgContext_KeyInfoA *info = buffer;
+ struct schan_context *ctx;
+ SECURITY_STATUS status;
+ if (!context_handle) return SEC_E_INVALID_HANDLE;
+ ctx = schan_get_object(context_handle->dwLower, SCHAN_HANDLE_CTX);
+ if (!ctx) return SEC_E_INVALID_HANDLE;
+ status = schan_imp_get_key_info(ctx->session, (SecPkgContext_KeyInfoW*)info);
+ if (status == SEC_E_OK)
+ {
+ info->sSignatureAlgorithmName = (SEC_CHAR *)ctx->key_signature_name;
+ info->sEncryptAlgorithmName = (SEC_CHAR *)ctx->key_encrypt_name;
+ fill_alg_name(info->SignatureAlgorithm, info->sSignatureAlgorithmName, FALSE);
+ fill_alg_name(info->EncryptAlgorithm, info->sEncryptAlgorithmName, FALSE);
+ }
+ return status;
+ }
case SECPKG_ATTR_REMOTE_CERT_CONTEXT:
return schan_QueryContextAttributesW(context_handle, attribute, buffer);
case SECPKG_ATTR_CONNECTION_INFO:
diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c
index 30640d3..ddf33a8 100644
--- a/dlls/secur32/schannel_gnutls.c
+++ b/dlls/secur32/schannel_gnutls.c
@@ -363,6 +363,22 @@ static ALG_ID schannel_get_kx_algid(int kx)
}
}
+static ALG_ID schannel_get_kx_signature_algid(gnutls_kx_algorithm_t kx)
+{
+ switch (kx)
+ {
+ case GNUTLS_KX_UNKNOWN: return 0;
+ case GNUTLS_KX_RSA:
+ case GNUTLS_KX_RSA_EXPORT:
+ case GNUTLS_KX_DHE_RSA:
+ case GNUTLS_KX_ECDHE_RSA: return CALG_RSA_SIGN;
+ case GNUTLS_KX_ECDHE_ECDSA: return CALG_ECDSA;
+ default:
+ FIXME("unknown algorithm %d\n", kx);
+ return 0;
+ }
+}
+
unsigned int schan_imp_get_session_cipher_block_size(schan_imp_session session)
{
gnutls_session_t s = (gnutls_session_t)session;
@@ -394,6 +410,23 @@ SECURITY_STATUS schan_imp_get_connection_info(schan_imp_session session,
return SEC_E_OK;
}
+SECURITY_STATUS schan_imp_get_key_info(schan_imp_session session, SecPkgContext_KeyInfoW *info)
+{
+ gnutls_session_t s = (gnutls_session_t)session;
+ gnutls_cipher_algorithm_t alg = pgnutls_cipher_get(s);
+ gnutls_kx_algorithm_t kx = pgnutls_kx_get(s);
+
+ TRACE("(%p,%p)\n", session, info);
+
+ info->sSignatureAlgorithmName = NULL; /* filled later */
+ info->sEncryptAlgorithmName = NULL; /* filled later */
+ info->KeySize = pgnutls_cipher_get_key_size(alg) * 8;
+ info->SignatureAlgorithm = schannel_get_kx_signature_algid(kx);
+ info->EncryptAlgorithm = schannel_get_cipher_algid(alg);
+
+ return SEC_E_OK;
+}</pre>
</div>
</blockquote>
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;" lang="x-western">
<pre wrap="">
</pre>
</div>
I don't think such helper that fills output partially is the right
solution here. You could use schan_imp_get_connection_info to get
all info you need except signature algorithm. For signature
algorithm, you may introduce a new, simpler, function.<br>
<br>
Thanks,<br>
Jacek<br>
</body>
</html>