Hi Akihiro,
Thanks for working on that. I have a few comments to the patch.
On 17.04.2017 16:26, Akihiro Sagawa wrote:
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];
Those should not be needed. You could for example store both WCHAR and char strings inside alg_name_map.
};
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)
If you change the patch as I suggested above, this could be something like:
static void *get_alg_name(ALF_ID id, BOOL wide)
+{
- 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 = "???";- }
A FIXME would would be nice here. Also, I think you could just return NULL in this case.
- 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;
This has an effect outside of scope of this patch. This should be be a separated patch, with a test case.
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;
+}
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.
Thanks, Jacek