[PATCH v4 0/3] MR8020: libs/ldap: Make GSS-SPNEGO SASL plugin work with LDAP server that uses Kerberos authentication.
-- v4: libs/ldap: sasl_decode() should treat intput buffer as data + token for Kerberos. libs/ldap: sasl_encode() should construct output buffer as data + token for Kerberos. libs/ldap: When initializing security context ask for mutual authentication. https://gitlab.winehq.org/wine/wine/-/merge_requests/8020
From: Dmitry Timoshkov <dmitry(a)baikal.ru> RFC4752 mentions it as required on the client side: https://www.rfc-editor.org/rfc/rfc4752#page-3 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55304 Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- libs/ldap/libldap/sasl_w.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libs/ldap/libldap/sasl_w.c b/libs/ldap/libldap/sasl_w.c index 48028352ef4..7478f73b900 100644 --- a/libs/ldap/libldap/sasl_w.c +++ b/libs/ldap/libldap/sasl_w.c @@ -1,5 +1,6 @@ /* * Copyright 2022 Hans Leidekker for CodeWeavers + * Copyright 2023 Dmitry Timoshkov * * SSPI based replacement for Cyrus SASL * @@ -36,6 +37,8 @@ struct connection sasl_interact_t prompts[4]; unsigned int max_token; unsigned int trailer_size; + unsigned int flags; + unsigned int qop; sasl_ssf_t ssf; char *buf; unsigned buf_size; @@ -261,7 +264,7 @@ int sasl_client_start( sasl_conn_t *handle, const char *mechlist, sasl_interact_ { 0, SECBUFFER_ALERT, NULL } }; SecBufferDesc out_buf_desc = { SECBUFFER_VERSION, ARRAYSIZE(out_bufs), out_bufs }; - ULONG attrs, flags = ISC_REQ_INTEGRITY | ISC_REQ_CONFIDENTIALITY; + ULONG attrs; SECURITY_STATUS status; int ret; @@ -276,7 +279,9 @@ int sasl_client_start( sasl_conn_t *handle, const char *mechlist, sasl_interact_ (SEC_WINNT_AUTH_IDENTITY_A *)&id, NULL, NULL, &conn->cred_handle, NULL ); if (status != SEC_E_OK) return SASL_FAIL; - status = InitializeSecurityContextA( &conn->cred_handle, NULL, conn->target, flags, + /* FIXME: flags probably should depend on LDAP_OPT_SSPI_FLAGS */ + conn->flags = ISC_REQ_INTEGRITY | ISC_REQ_CONFIDENTIALITY | ISC_REQ_MUTUAL_AUTH | ISC_REQ_EXTENDED_ERROR; + status = InitializeSecurityContextA( &conn->cred_handle, NULL, conn->target, conn->flags, 0, 0, NULL, 0, &conn->ctxt_handle, &out_buf_desc, &attrs, NULL ); if (status == SEC_E_OK || status == SEC_I_CONTINUE_NEEDED) { @@ -311,10 +316,10 @@ int sasl_client_step( sasl_conn_t *handle, const char *serverin, unsigned int se }; SecBufferDesc in_buf_desc = { SECBUFFER_VERSION, ARRAYSIZE(in_bufs), in_bufs }; SecBufferDesc out_buf_desc = { SECBUFFER_VERSION, ARRAYSIZE(out_bufs), out_bufs }; - ULONG attrs, flags = ISC_REQ_INTEGRITY | ISC_REQ_CONFIDENTIALITY; + ULONG attrs; SECURITY_STATUS status; - status = InitializeSecurityContextA( NULL, &conn->ctxt_handle, conn->target, flags, 0, 0, + status = InitializeSecurityContextA( NULL, &conn->ctxt_handle, conn->target, conn->flags, 0, 0, &in_buf_desc, 0, &conn->ctxt_handle, &out_buf_desc, &attrs, NULL ); if (status == SEC_E_OK || status == SEC_I_CONTINUE_NEEDED) { @@ -325,6 +330,7 @@ int sasl_client_step( sasl_conn_t *handle, const char *serverin, unsigned int se { conn->ssf = get_key_size( &conn->ctxt_handle ); conn->trailer_size = get_trailer_size( &conn->ctxt_handle ); + conn->qop = attrs; return SASL_OK; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8020
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Partially based on a patch by Hans Leidekker. Cyrus SASL GSSAPI plugin does it this way: https://github.com/cyrusimap/cyrus-sasl/blob/master/plugins/gssapi.c Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55304 Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- libs/ldap/libldap/sasl_w.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/libs/ldap/libldap/sasl_w.c b/libs/ldap/libldap/sasl_w.c index 7478f73b900..145e14ccdc2 100644 --- a/libs/ldap/libldap/sasl_w.c +++ b/libs/ldap/libldap/sasl_w.c @@ -39,6 +39,7 @@ struct connection unsigned int trailer_size; unsigned int flags; unsigned int qop; + unsigned short package_id; sasl_ssf_t ssf; char *buf; unsigned buf_size; @@ -106,20 +107,30 @@ int sasl_encode( sasl_conn_t *handle, const char *input, unsigned int inputlen, int ret; if ((ret = grow_buffer( conn, sizeof(len) + inputlen + conn->trailer_size )) < 0) return ret; - memcpy( conn->buf + sizeof(len) + conn->trailer_size, input, inputlen ); - bufs[0].pvBuffer = conn->buf + sizeof(len) + conn->trailer_size; - bufs[1].pvBuffer = conn->buf + sizeof(len); + if (conn->package_id == RPC_C_AUTHN_GSS_KERBEROS) + { + memcpy( conn->buf + sizeof(len), input, inputlen ); + bufs[0].pvBuffer = conn->buf + sizeof(len); + bufs[1].pvBuffer = conn->buf + sizeof(len) + inputlen; + } + else + { + memcpy( conn->buf + sizeof(len) + conn->trailer_size, input, inputlen ); + bufs[0].pvBuffer = conn->buf + sizeof(len) + conn->trailer_size; + bufs[1].pvBuffer = conn->buf + sizeof(len); + } - status = EncryptMessage( &conn->ctxt_handle, 0, &buf_desc, 0 ); + status = EncryptMessage( &conn->ctxt_handle, (conn->qop & ISC_RET_CONFIDENTIALITY) ? 0 : SECQOP_WRAP_NO_ENCRYPT, &buf_desc, 0 ); if (status == SEC_E_OK) { len = htonl( bufs[0].cbBuffer + bufs[1].cbBuffer ); memcpy( conn->buf, &len, sizeof(len) ); *output = conn->buf; *outputlen = sizeof(len) + bufs[0].cbBuffer + bufs[1].cbBuffer; + return SASL_OK; } - return (status == SEC_E_OK) ? SASL_OK : SASL_FAIL; + return SASL_FAIL; } const char *sasl_errstring( int saslerr, const char *langlist, const char **outlang ) @@ -253,6 +264,18 @@ static ULONG get_trailer_size( CtxtHandle *ctx ) return sizes.cbSecurityTrailer; } +static unsigned short get_package_id( CtxtHandle *ctx ) +{ + SecPkgContext_NegotiationInfoW info; + unsigned short id; + + memset( &info, 0, sizeof(info) ); + if (QueryContextAttributesW( ctx, SECPKG_ATTR_NEGOTIATION_INFO, &info )) return 0; + id = info.PackageInfo->wRPCID; + FreeContextBuffer( info.PackageInfo ); + return id; +} + int sasl_client_start( sasl_conn_t *handle, const char *mechlist, sasl_interact_t **prompts, const char **clientout, unsigned int *clientoutlen, const char **mech ) { @@ -293,6 +316,7 @@ int sasl_client_start( sasl_conn_t *handle, const char *mechlist, sasl_interact_ { conn->ssf = get_key_size( &conn->ctxt_handle ); conn->trailer_size = get_trailer_size( &conn->ctxt_handle ); + conn->package_id = get_package_id( &conn->ctxt_handle ); return SASL_OK; } } @@ -330,6 +354,7 @@ int sasl_client_step( sasl_conn_t *handle, const char *serverin, unsigned int se { conn->ssf = get_key_size( &conn->ctxt_handle ); conn->trailer_size = get_trailer_size( &conn->ctxt_handle ); + conn->package_id = get_package_id( &conn->ctxt_handle ); conn->qop = attrs; return SASL_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8020
From: Dmitry Timoshkov <dmitry(a)baikal.ru> Partially based on a patch by Hans Leidekker. Cyrus SALS plugin treats it this way: https://github.com/cyrusimap/cyrus-sasl/blob/master/plugins/gssapi.c Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55304 Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- libs/ldap/libldap/sasl_w.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/libs/ldap/libldap/sasl_w.c b/libs/ldap/libldap/sasl_w.c index 145e14ccdc2..21f15877189 100644 --- a/libs/ldap/libldap/sasl_w.c +++ b/libs/ldap/libldap/sasl_w.c @@ -68,28 +68,41 @@ int sasl_decode( sasl_conn_t *handle, const char *input, unsigned int inputlen, unsigned int len; SecBuffer bufs[2] = { - { conn->trailer_size, SECBUFFER_TOKEN, NULL }, - { inputlen - conn->trailer_size - sizeof(len), SECBUFFER_DATA, NULL } + { 0, SECBUFFER_DATA, NULL }, + { conn->trailer_size, SECBUFFER_TOKEN, NULL } }; SecBufferDesc buf_desc = { SECBUFFER_VERSION, ARRAYSIZE(bufs), bufs }; SECURITY_STATUS status; int ret; if (inputlen < sizeof(len) + conn->trailer_size) return SASL_FAIL; + len = ntohl( *(unsigned int *)input ); + if (inputlen < sizeof(len) + len) return SASL_FAIL; + + if ((ret = grow_buffer( conn, len )) < 0) return ret; + memcpy( conn->buf, input + sizeof(len), len ); - if ((ret = grow_buffer( conn, inputlen - sizeof(len) )) < 0) return ret; - memcpy( conn->buf, input + sizeof(len), inputlen - sizeof(len) ); - bufs[0].pvBuffer = conn->buf; - bufs[1].pvBuffer = conn->buf + conn->trailer_size; + bufs[0].cbBuffer = len - conn->trailer_size; + if (conn->package_id == RPC_C_AUTHN_GSS_KERBEROS) + { + bufs[0].pvBuffer = conn->buf; + bufs[1].pvBuffer = conn->buf + bufs[0].cbBuffer; + } + else + { + bufs[0].pvBuffer = conn->buf + conn->trailer_size; + bufs[1].pvBuffer = conn->buf; + } status = DecryptMessage( &conn->ctxt_handle, &buf_desc, 0, NULL ); if (status == SEC_E_OK) { - *output = bufs[1].pvBuffer; - *outputlen = bufs[1].cbBuffer; + *output = bufs[0].pvBuffer; + *outputlen = bufs[0].cbBuffer; + return SASL_OK; } - return (status == SEC_E_OK) ? SASL_OK : SASL_FAIL; + return SASL_FAIL; } int sasl_encode( sasl_conn_t *handle, const char *input, unsigned int inputlen, const char **output, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8020
Thanks. Can you put the qop part in a separate patch? It's not related to the buffer layout changes. And please squash the encode/decode commits since that is a single logical change. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8020#note_104079
participants (3)
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Hans Leidekker (＠hans)