[6/6] secur32: Add support for SECPKG_ATTR_KEY_INFO (GnuTLS only).
Jacek Caban
jacek at codeweavers.com
Wed Apr 19 06:57:33 CDT 2017
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20170419/3b3ebd18/attachment.html>
More information about the wine-devel
mailing list