From 0d7dc34ce9345c662fbb7b87b986d57dcfb2557a Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Fri, 22 Mar 2024 16:20:18 +1300 Subject: [PATCH 01/29] selftest: move some more expected failures to expectedfail.d Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Wed Apr 10 06:15:46 UTC 2024 on atb-devel-224 (cherry picked from commit 60df2a09a4394d2b494224ad3d33314079e73066) --- selftest/expectedfail.d/ldap-tlsverifypeer | 10 ++++++++++ selftest/knownfail | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) create mode 100644 selftest/expectedfail.d/ldap-tlsverifypeer diff --git a/selftest/expectedfail.d/ldap-tlsverifypeer b/selftest/expectedfail.d/ldap-tlsverifypeer new file mode 100644 index 000000000000..d124487cdde3 --- /dev/null +++ b/selftest/expectedfail.d/ldap-tlsverifypeer @@ -0,0 +1,10 @@ +# These are supposed to fail as we want to verify the "tls verify peer" +# restrictions. Note that fl2008r2dc uses a self-signed certificate +# with does not have a crl file. +# +^samba4.ldb.simple.ldaps.*SERVER_NAME.*tlsverifypeer=ca_and_name_if_available\( +^samba4.ldb.simple.ldaps.*SERVER_NAME.*tlsverifypeer=ca_and_name\( +^samba4.ldb.simple.ldaps.*SERVER_NAME.*tlsverifypeer=as_strict_as_possible\( +^samba4.ldb.simple.ldaps.*SERVER_IP.*tlsverifypeer=ca_and_name\( +^samba4.ldb.simple.ldaps.*SERVER_IP.*tlsverifypeer=as_strict_as_possible\( +^samba4.ldb.simple.ldaps.*SERVER.REALM.*tlsverifypeer=as_strict_as_possible.*fl2008r2dc diff --git a/selftest/knownfail b/selftest/knownfail index 746983691575..77f5d5d5be62 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -317,16 +317,6 @@ ^samba4.ldb.simple.ldap with SIMPLE-BIND.*ad_dc_ntvfs # ldap server require strong auth = allow_sasl_over_tls ^samba4.ldb.simple.ldap with SIMPLE-BIND.*fl2003dc # ldap server require strong auth = yes ^samba4.ldb.simple.ldaps with SASL-BIND.*fl2003dc # ldap server require strong auth = yes -# These are supposed to fail as we want to verify the "tls verify peer" -# restrictions. Note that fl2008r2dc uses a self-signed certificate -# with does not have a crl file. -# -^samba4.ldb.simple.ldaps.*SERVER_NAME.*tlsverifypeer=ca_and_name_if_available\( -^samba4.ldb.simple.ldaps.*SERVER_NAME.*tlsverifypeer=ca_and_name\( -^samba4.ldb.simple.ldaps.*SERVER_NAME.*tlsverifypeer=as_strict_as_possible\( -^samba4.ldb.simple.ldaps.*SERVER_IP.*tlsverifypeer=ca_and_name\( -^samba4.ldb.simple.ldaps.*SERVER_IP.*tlsverifypeer=as_strict_as_possible\( -^samba4.ldb.simple.ldaps.*SERVER.REALM.*tlsverifypeer=as_strict_as_possible.*fl2008r2dc # # we don't allow auth_level_connect anymore... # -- 2.34.1 From 4e53d691cfa3366ceb552de3977326b6eba29e69 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 17 Apr 2024 21:01:08 +0200 Subject: [PATCH 02/29] s4:libcli/ldap: ldap4_new_connection() requires a valid lp_ctx Otherwise we'll crash in a lot of places later. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8007569e9f7d374456a3fbd172a905173462eb5f) --- source4/libcli/ldap/ldap_client.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c index db13ad3032f1..3c9f91601e69 100644 --- a/source4/libcli/ldap/ldap_client.c +++ b/source4/libcli/ldap/ldap_client.c @@ -65,6 +65,10 @@ _PUBLIC_ struct ldap_connection *ldap4_new_connection(TALLOC_CTX *mem_ctx, return NULL; } + if (lp_ctx == NULL) { + return NULL; + } + conn = talloc_zero(mem_ctx, struct ldap_connection); if (conn == NULL) { return NULL; -- 2.34.1 From 266be5b329f9575d1f08b3ed3d056643426e788c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 17 Apr 2024 21:02:03 +0200 Subject: [PATCH 03/29] ldb_ildap: require ldb_get_opaque(ldb, "loadparm") to be valid Without a valid loadparm_context we can't connect. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 2435ab1ad7092c004df72c2cb033eb94e5bf8274) --- lib/ldb-samba/ldb_ildap.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/ldb-samba/ldb_ildap.c b/lib/ldb-samba/ldb_ildap.c index 37ef185fbbfd..0c051f143a7f 100644 --- a/lib/ldb-samba/ldb_ildap.c +++ b/lib/ldb-samba/ldb_ildap.c @@ -917,6 +917,12 @@ static int ildb_connect(struct ldb_context *ldb, const char *url, struct cli_credentials *creds; struct loadparm_context *lp_ctx; + lp_ctx = talloc_get_type(ldb_get_opaque(ldb, "loadparm"), + struct loadparm_context); + if (lp_ctx == NULL) { + return ldb_operr(ldb); + } + module = ldb_module_new(ldb, ldb, "ldb_ildap backend", &ildb_ops); if (!module) return LDB_ERR_OPERATIONS_ERROR; @@ -929,9 +935,6 @@ static int ildb_connect(struct ldb_context *ldb, const char *url, ildb->event_ctx = ldb_get_event_context(ldb); - lp_ctx = talloc_get_type(ldb_get_opaque(ldb, "loadparm"), - struct loadparm_context); - ildb->ldap = ldap4_new_connection(ildb, lp_ctx, ildb->event_ctx); if (!ildb->ldap) { -- 2.34.1 From ab50c31a455c6373eaf4edcac1a5c88591fd4e16 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 26 Jan 2024 18:07:53 +0100 Subject: [PATCH 04/29] s4:libcli/ldap: fix no memory error code in ldap_bind_sasl() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8deba427e2697501f10e80a2ac0325a657635b92) --- source4/libcli/ldap/ldap_bind.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c index 1f7308211b10..51231a615c44 100644 --- a/source4/libcli/ldap/ldap_bind.c +++ b/source4/libcli/ldap/ldap_bind.c @@ -264,7 +264,10 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, } tmp_ctx = talloc_new(conn); - if (tmp_ctx == NULL) goto failed; + if (tmp_ctx == NULL) { + status = NT_STATUS_NO_MEMORY; + goto failed; + } search = &sasl_mechs_msgs[0]->r.SearchResultEntry; if (search->num_attributes != 1) { -- 2.34.1 From 97918d7ac116ac31a54dca422a7c551982baf9a9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 26 Jan 2024 18:04:57 +0100 Subject: [PATCH 05/29] s4:libcli/ldap: force GSS-SPNEGO in ldap_bind_sasl() There's no point in asking the server for supportedSASLMechanisms, every server (we care about) supports GSS-SPNEGO. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 68f6a461e1706f03007d3c5cfc68c71383b4ff28) --- source4/libcli/ldap/ldap_bind.c | 57 ++++----------------------------- 1 file changed, 7 insertions(+), 50 deletions(-) diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c index 51231a615c44..e1a37a391c5b 100644 --- a/source4/libcli/ldap/ldap_bind.c +++ b/source4/libcli/ldap/ldap_bind.c @@ -27,7 +27,6 @@ #include "libcli/ldap/ldap_client.h" #include "lib/tls/tls.h" #include "auth/gensec/gensec.h" -#include "auth/gensec/gensec_internal.h" /* TODO: remove this */ #include "source4/auth/gensec/gensec_tstream.h" #include "auth/credentials/credentials.h" #include "lib/stream/packet.h" @@ -208,23 +207,14 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, struct cli_credentials *creds, struct loadparm_context *lp_ctx) { + const char *sasl_mech = "GSS-SPNEGO"; NTSTATUS status; TALLOC_CTX *tmp_ctx = NULL; - DATA_BLOB input = data_blob(NULL, 0); DATA_BLOB output = data_blob(NULL, 0); - - struct ldap_message **sasl_mechs_msgs; - struct ldap_SearchResEntry *search; - int count, i; bool first = true; int wrap_flags = 0; - const char **sasl_names; uint32_t old_gensec_features; - static const char *supported_sasl_mech_attrs[] = { - "supportedSASLMechanisms", - NULL - }; unsigned int logon_retries = 0; size_t queue_length; @@ -248,46 +238,12 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, goto failed; } - status = ildap_search(conn, "", LDAP_SEARCH_SCOPE_BASE, "", supported_sasl_mech_attrs, - false, NULL, NULL, &sasl_mechs_msgs); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(1, ("Failed to inquire of target's available sasl mechs in rootdse search: %s\n", - nt_errstr(status))); - goto failed; - } - - count = ildap_count_entries(conn, sasl_mechs_msgs); - if (count != 1) { - DEBUG(1, ("Failed to inquire of target's available sasl mechs in rootdse search: wrong number of replies: %d\n", - count)); - goto failed; - } - tmp_ctx = talloc_new(conn); if (tmp_ctx == NULL) { status = NT_STATUS_NO_MEMORY; goto failed; } - search = &sasl_mechs_msgs[0]->r.SearchResultEntry; - if (search->num_attributes != 1) { - DEBUG(1, ("Failed to inquire of target's available sasl mechs in rootdse search: wrong number of attributes: %d != 1\n", - search->num_attributes)); - goto failed; - } - - sasl_names = talloc_array(tmp_ctx, const char *, search->attributes[0].num_values + 1); - if (!sasl_names) { - DEBUG(1, ("talloc_arry(char *, %d) failed\n", - count)); - goto failed; - } - - for (i=0; iattributes[0].num_values; i++) { - sasl_names[i] = (const char *)search->attributes[0].values[i].data; - } - sasl_names[i] = NULL; - gensec_init(); if (conn->sockets.active == conn->sockets.tls) { @@ -373,10 +329,10 @@ try_logon_again: goto failed; } - status = gensec_start_mech_by_sasl_list(conn->gensec, sasl_names); + status = gensec_start_mech_by_sasl_name(conn->gensec, sasl_mech); if (!NT_STATUS_IS_OK(status)) { - DEBUG(1, ("None of the %d proposed SASL mechs were acceptable: %s\n", - count, nt_errstr(status))); + DBG_WARNING("gensec_start_mech_by_sasl_name(%s): %s\n", + sasl_mech, nt_errstr(status)); goto failed; } @@ -418,8 +374,9 @@ try_logon_again: } first = false; - /* Perhaps we should make gensec_start_mech_by_sasl_list() return the name we got? */ - msg = new_ldap_sasl_bind_msg(tmp_ctx, conn->gensec->ops->sasl_name, (output.data?&output:NULL)); + msg = new_ldap_sasl_bind_msg(tmp_ctx, + sasl_mech, + output.data != NULL ? &output : NULL); if (msg == NULL) { status = NT_STATUS_NO_MEMORY; goto failed; -- 2.34.1 From 5c8a99db3ca68b19928059c718d0ee34b4ee215c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 26 Jan 2024 14:27:16 +0100 Subject: [PATCH 06/29] s4:lib/tls: remove tstream_tls_push_trigger_write step At the time of https://bugzilla.samba.org/show_bug.cgi?id=7218, we tested this versions: 2.4.1 -> broken 2.4.2 -> broken 2.6.0 -> broken 2.8.0 -> broken 2.8.1 -> broken 2.8.2 -> OK 2.8.3 -> OK 2.8.4 -> OK 2.8.5 -> OK 2.8.6 -> OK 2.10.0 -> broken 2.10.1 -> broken 2.10.2 -> OK These seemed to be the fixes in gnutls upstream. Change 2.8.1 -> 2.8.2: http://git.savannah.gnu.org/gitweb/?p=gnutls.git;a=commitdiff;h=28fb34099edaf62e5472cc6e5e2749fed369ea01 Change 2.10.1 -> 2.10.2: http://git.savannah.gnu.org/gitweb/?p=gnutls.git;a=commitdiff;h=0d07d8432d57805a8354ebd6c1e7829f3ab159cb This shouldn't be a problem with recent (>= 3.6) versions of gnutls. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 5844ef27aa46cba3d343035ccd35b03525db9843) --- source4/lib/tls/tls_tstream.c | 61 +++-------------------------------- 1 file changed, 5 insertions(+), 56 deletions(-) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index 5e0c56b01020..b85c785cdb12 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -74,7 +74,6 @@ struct tstream_tls { off_t ofs; struct iovec iov; struct tevent_req *subreq; - struct tevent_immediate *im; } push; struct { @@ -159,9 +158,7 @@ static void tstream_tls_retry_trigger(struct tevent_context *ctx, tstream_tls_retry(stream, true); } -static void tstream_tls_push_trigger_write(struct tevent_context *ev, - struct tevent_immediate *im, - void *private_data); +static void tstream_tls_push_done(struct tevent_req *subreq); static ssize_t tstream_tls_push_function(gnutls_transport_ptr_t ptr, const void *buf, size_t size) @@ -172,6 +169,7 @@ static ssize_t tstream_tls_push_function(gnutls_transport_ptr_t ptr, struct tstream_tls *tlss = tstream_context_data(stream, struct tstream_tls); + struct tevent_req *subreq = NULL; uint8_t *nbuf; size_t len; @@ -205,56 +203,7 @@ static ssize_t tstream_tls_push_function(gnutls_transport_ptr_t ptr, tlss->push.buf = nbuf; memcpy(tlss->push.buf + tlss->push.ofs, buf, len); - - if (tlss->push.im == NULL) { - tlss->push.im = tevent_create_immediate(tlss); - if (tlss->push.im == NULL) { - errno = ENOMEM; - return -1; - } - } - - if (tlss->push.ofs == 0) { - /* - * We'll do start the tstream_writev - * in the next event cycle. - * - * This way we can batch all push requests, - * if they fit into a UINT16_MAX buffer. - * - * This is important as gnutls_handshake() - * had a bug in some versions e.g. 2.4.1 - * and others (See bug #7218) and it doesn't - * handle EAGAIN. - */ - tevent_schedule_immediate(tlss->push.im, - tlss->current_ev, - tstream_tls_push_trigger_write, - stream); - } - tlss->push.ofs += len; - return len; -} - -static void tstream_tls_push_done(struct tevent_req *subreq); - -static void tstream_tls_push_trigger_write(struct tevent_context *ev, - struct tevent_immediate *im, - void *private_data) -{ - struct tstream_context *stream = - talloc_get_type_abort(private_data, - struct tstream_context); - struct tstream_tls *tlss = - tstream_context_data(stream, - struct tstream_tls); - struct tevent_req *subreq; - - if (tlss->push.subreq) { - /* nothing todo */ - return; - } tlss->push.iov.iov_base = (char *)tlss->push.buf; tlss->push.iov.iov_len = tlss->push.ofs; @@ -264,13 +213,13 @@ static void tstream_tls_push_trigger_write(struct tevent_context *ev, tlss->plain_stream, &tlss->push.iov, 1); if (subreq == NULL) { - tlss->error = ENOMEM; - tstream_tls_retry(stream, false); - return; + errno = ENOMEM; + return -1; } tevent_req_set_callback(subreq, tstream_tls_push_done, stream); tlss->push.subreq = subreq; + return len; } static void tstream_tls_push_done(struct tevent_req *subreq) -- 2.34.1 From 42db844377e629310234721a947d3be5bb1e91d5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 26 Jan 2024 14:42:40 +0100 Subject: [PATCH 07/29] s3:lib/tls: we need to call tstream_tls_retry_handshake/disconnect() until all buffers are flushed Before the handshare or disconnect is over we need to wait until we delivered the lowlevel messages to the transport/kernel socket. Otherwise we'll have a problem if another tevent_context is used after the handshake. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 6688945fa03f4a448708f729083ea4a1cdd1ab88) --- source4/lib/tls/tls_tstream.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index b85c785cdb12..22aa3334974a 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -69,6 +69,10 @@ struct tstream_tls { struct tevent_immediate *retry_im; + struct { + struct tevent_req *mgmt_req; + } waiting_flush; + struct { uint8_t *buf; off_t ofs; @@ -120,6 +124,17 @@ static void tstream_tls_retry(struct tstream_context *stream, bool deferred) tstream_context_data(stream, struct tstream_tls); + if (tlss->push.subreq == NULL && tlss->pull.subreq == NULL) { + if (tlss->waiting_flush.mgmt_req != NULL) { + struct tevent_req *req = tlss->waiting_flush.mgmt_req; + + tlss->waiting_flush.mgmt_req = NULL; + + tevent_req_done(req); + return; + } + } + if (tlss->disconnect.req) { tstream_tls_retry_disconnect(stream); return; @@ -784,6 +799,11 @@ static void tstream_tls_retry_disconnect(struct tstream_context *stream) return; } + if (tlss->push.subreq != NULL || tlss->pull.subreq != NULL) { + tlss->waiting_flush.mgmt_req = req; + return; + } + tevent_req_done(req); } @@ -1476,6 +1496,11 @@ static void tstream_tls_retry_handshake(struct tstream_context *stream) } } + if (tlss->push.subreq != NULL || tlss->pull.subreq != NULL) { + tlss->waiting_flush.mgmt_req = req; + return; + } + tevent_req_done(req); } -- 2.34.1 From 85969d4a591605aa70889103a27e1311df23e1b9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 26 Jan 2024 15:30:09 +0100 Subject: [PATCH 08/29] s4:lib/tls: assert that event contexts are not mixed BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit ac4bca77039cbc31323fb10b3706ed959a0cbbcd) --- source4/lib/tls/tls_tstream.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index 22aa3334974a..e27883731657 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -413,6 +413,12 @@ static struct tevent_req *tstream_tls_readv_send(TALLOC_CTX *mem_ctx, struct tstream_tls_readv_state *state; tlss->read.req = NULL; + + if (tlss->current_ev != ev) { + SMB_ASSERT(tlss->push.subreq == NULL); + SMB_ASSERT(tlss->pull.subreq == NULL); + } + tlss->current_ev = ev; req = tevent_req_create(mem_ctx, &state, @@ -577,6 +583,12 @@ static struct tevent_req *tstream_tls_writev_send(TALLOC_CTX *mem_ctx, struct tstream_tls_writev_state *state; tlss->write.req = NULL; + + if (tlss->current_ev != ev) { + SMB_ASSERT(tlss->push.subreq == NULL); + SMB_ASSERT(tlss->pull.subreq == NULL); + } + tlss->current_ev = ev; req = tevent_req_create(mem_ctx, &state, @@ -743,6 +755,12 @@ static struct tevent_req *tstream_tls_disconnect_send(TALLOC_CTX *mem_ctx, struct tstream_tls_disconnect_state *state; tlss->disconnect.req = NULL; + + if (tlss->current_ev != ev) { + SMB_ASSERT(tlss->push.subreq == NULL); + SMB_ASSERT(tlss->pull.subreq == NULL); + } + tlss->current_ev = ev; req = tevent_req_create(mem_ctx, &state, -- 2.34.1 From 10661a2a8aa57feda17859dcc52c0521226ed121 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 12 Feb 2024 12:35:02 +0100 Subject: [PATCH 09/29] s4:lib/tls: split out tstream_tls_prepare_gnutls() Review with: git show --patience BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 60b11645b0d1c8304eabbb2aeca8a6b5190a3a2e) --- source4/lib/tls/tls_tstream.c | 233 +++++++++++++++------------------- source4/lib/tls/wscript_build | 1 + 2 files changed, 106 insertions(+), 128 deletions(-) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index e27883731657..6145037cf8de 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -29,6 +29,7 @@ #include #include +#include "lib/crypto/gnutls_helpers.h" #define DH_BITS 2048 @@ -62,6 +63,8 @@ struct tstream_tls { gnutls_session_t tls_session; + bool is_server; + enum tls_verify_peer_state verify_peer; const char *peer_name; @@ -982,40 +985,28 @@ NTSTATUS tstream_tls_params_client(TALLOC_CTX *mem_ctx, return NT_STATUS_OK; } -struct tstream_tls_connect_state { - struct tstream_context *tls_stream; -}; - -struct tevent_req *_tstream_tls_connect_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - struct tstream_context *plain_stream, - struct tstream_tls_params *_tls_params, - const char *location) +static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, + struct tstream_tls *tlss) { - struct tevent_req *req; - struct tstream_tls_connect_state *state; - const char *error_pos; - struct tstream_tls *tlss; - struct tstream_tls_params_internal *tls_params = NULL; + struct tstream_tls_params_internal *tlsp = NULL; int ret; - unsigned int flags = GNUTLS_CLIENT; - - req = tevent_req_create(mem_ctx, &state, - struct tstream_tls_connect_state); - if (req == NULL) { - return NULL; - } + unsigned int flags; - state->tls_stream = tstream_context_create(state, - &tstream_tls_ops, - &tlss, - struct tstream_tls, - location); - if (tevent_req_nomem(state->tls_stream, req)) { - return tevent_req_post(req, ev); + if (tlss->is_server) { + flags = GNUTLS_SERVER; + } else { + flags = GNUTLS_CLIENT; +#ifdef GNUTLS_NO_TICKETS + /* + * tls_tstream can't properly handle 'New Session Ticket' + * messages sent 'after' the client sends the 'Finished' + * message. GNUTLS_NO_TICKETS was introduced in GnuTLS 3.5.6. + * This flag is to indicate the session Flag session should not + * use resumption with session tickets. + */ + flags |= GNUTLS_NO_TICKETS; +#endif } - ZERO_STRUCTP(tlss); - talloc_set_destructor(tlss, tstream_tls_destructor); /* * Note we need to make sure x509_cred and dh_params @@ -1027,71 +1018,109 @@ struct tevent_req *_tstream_tls_connect_send(TALLOC_CTX *mem_ctx, * * Note: here we use talloc_reference() in a way * that does not expose it to the caller. - * */ - tls_params = talloc_reference(tlss, _tls_params->internal); - if (tevent_req_nomem(tls_params, req)) { - return tevent_req_post(req, ev); + tlsp = talloc_reference(tlss, _tlsp->internal); + if (tlsp == NULL) { + return NT_STATUS_NO_MEMORY; } - tlss->plain_stream = plain_stream; - tlss->verify_peer = tls_params->verify_peer; - if (tls_params->peer_name != NULL) { - tlss->peer_name = talloc_strdup(tlss, tls_params->peer_name); - if (tevent_req_nomem(tlss->peer_name, req)) { - return tevent_req_post(req, ev); + tlss->verify_peer = tlsp->verify_peer; + if (tlsp->peer_name != NULL) { + tlss->peer_name = talloc_strdup(tlss, tlsp->peer_name); + if (tlss->peer_name == NULL) { + return NT_STATUS_NO_MEMORY; } } - tlss->current_ev = ev; - tlss->retry_im = tevent_create_immediate(tlss); - if (tevent_req_nomem(tlss->retry_im, req)) { - return tevent_req_post(req, ev); + if (tlss->current_ev != NULL) { + tlss->retry_im = tevent_create_immediate(tlss); + if (tlss->retry_im == NULL) { + return NT_STATUS_NO_MEMORY; + } } -#ifdef GNUTLS_NO_TICKETS - /* - * tls_tstream can't properly handle 'New Session Ticket' messages - * sent 'after' the client sends the 'Finished' message. - * GNUTLS_NO_TICKETS was introduced in GnuTLS 3.5.6. This flag is to - * indicate the session Flag session should not use resumption with - * session tickets. - */ - flags |= GNUTLS_NO_TICKETS; -#endif - ret = gnutls_init(&tlss->tls_session, flags); if (ret != GNUTLS_E_SUCCESS) { - DEBUG(0,("TLS %s - %s\n", __location__, gnutls_strerror(ret))); - tevent_req_error(req, EINVAL); - return tevent_req_post(req, ev); + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); } ret = gnutls_set_default_priority(tlss->tls_session); if (ret != GNUTLS_E_SUCCESS) { - DBG_ERR("TLS %s - %s. Failed to set default priorities\n", - __location__, gnutls_strerror(ret)); - tevent_req_error(req, EINVAL); - return tevent_req_post(req, ev); + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); } - if (strlen(tls_params->tls_priority) > 0) { + if (strlen(tlsp->tls_priority) > 0) { + const char *error_pos = NULL; + ret = gnutls_priority_set_direct(tlss->tls_session, - tls_params->tls_priority, + tlsp->tls_priority, &error_pos); if (ret != GNUTLS_E_SUCCESS) { - DEBUG(0,("TLS %s - %s. Check 'tls priority' option at '%s'\n", - __location__, gnutls_strerror(ret), error_pos)); - tevent_req_error(req, EINVAL); - return tevent_req_post(req, ev); + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); } } ret = gnutls_credentials_set(tlss->tls_session, GNUTLS_CRD_CERTIFICATE, - tls_params->x509_cred); + tlsp->x509_cred); if (ret != GNUTLS_E_SUCCESS) { - DEBUG(0,("TLS %s - %s\n", __location__, gnutls_strerror(ret))); + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); + } + + if (tlss->is_server) { + gnutls_certificate_server_set_request(tlss->tls_session, + GNUTLS_CERT_REQUEST); + gnutls_dh_set_prime_bits(tlss->tls_session, DH_BITS); + } + + return NT_STATUS_OK; +} + +struct tstream_tls_connect_state { + struct tstream_context *tls_stream; +}; + +struct tevent_req *_tstream_tls_connect_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct tstream_context *plain_stream, + struct tstream_tls_params *_tls_params, + const char *location) +{ + struct tevent_req *req; + struct tstream_tls_connect_state *state; + struct tstream_tls *tlss; + NTSTATUS status; + + req = tevent_req_create(mem_ctx, &state, + struct tstream_tls_connect_state); + if (req == NULL) { + return NULL; + } + + state->tls_stream = tstream_context_create(state, + &tstream_tls_ops, + &tlss, + struct tstream_tls, + location); + if (tevent_req_nomem(state->tls_stream, req)) { + return tevent_req_post(req, ev); + } + ZERO_STRUCTP(tlss); + talloc_set_destructor(tlss, tstream_tls_destructor); + tlss->plain_stream = plain_stream; + tlss->is_server = false; + tlss->current_ev = ev; + + status = tstream_tls_prepare_gnutls(_tls_params, tlss); + if (NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)) { + tevent_req_oom(req); + return tevent_req_post(req, ev); + } + if (!NT_STATUS_IS_OK(status)) { tevent_req_error(req, EINVAL); return tevent_req_post(req, ev); } @@ -1312,9 +1341,7 @@ struct tevent_req *_tstream_tls_accept_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; struct tstream_tls_accept_state *state; struct tstream_tls *tlss; - const char *error_pos; - struct tstream_tls_params_internal *tlsp = NULL; - int ret; + NTSTATUS status; req = tevent_req_create(mem_ctx, &state, struct tstream_tls_accept_state); @@ -1332,70 +1359,20 @@ struct tevent_req *_tstream_tls_accept_send(TALLOC_CTX *mem_ctx, } ZERO_STRUCTP(tlss); talloc_set_destructor(tlss, tstream_tls_destructor); - - /* - * Note we need to make sure x509_cred and dh_params - * from tstream_tls_params_internal stay alive for - * the whole lifetime of this session! - * - * See 'man gnutls_credentials_set' and - * 'man gnutls_certificate_set_dh_params'. - * - * Note: here we use talloc_reference() in a way - * that does not expose it to the caller. - */ - tlsp = talloc_reference(tlss, _tlsp->internal); - if (tevent_req_nomem(tlsp, req)) { - return tevent_req_post(req, ev); - } - tlss->plain_stream = plain_stream; - + tlss->is_server = true; tlss->current_ev = ev; - tlss->retry_im = tevent_create_immediate(tlss); - if (tevent_req_nomem(tlss->retry_im, req)) { - return tevent_req_post(req, ev); - } - ret = gnutls_init(&tlss->tls_session, GNUTLS_SERVER); - if (ret != GNUTLS_E_SUCCESS) { - DEBUG(0,("TLS %s - %s\n", __location__, gnutls_strerror(ret))); - tevent_req_error(req, EINVAL); + status = tstream_tls_prepare_gnutls(_tlsp, tlss); + if (NT_STATUS_EQUAL(status, NT_STATUS_NO_MEMORY)) { + tevent_req_oom(req); return tevent_req_post(req, ev); } - - ret = gnutls_set_default_priority(tlss->tls_session); - if (ret != GNUTLS_E_SUCCESS) { - DBG_ERR("TLS %s - %s. Failed to set default priorities\n", - __location__, gnutls_strerror(ret)); + if (!NT_STATUS_IS_OK(status)) { tevent_req_error(req, EINVAL); return tevent_req_post(req, ev); } - if (strlen(tlsp->tls_priority) > 0) { - ret = gnutls_priority_set_direct(tlss->tls_session, - tlsp->tls_priority, - &error_pos); - if (ret != GNUTLS_E_SUCCESS) { - DEBUG(0,("TLS %s - %s. Check 'tls priority' option at '%s'\n", - __location__, gnutls_strerror(ret), error_pos)); - tevent_req_error(req, EINVAL); - return tevent_req_post(req, ev); - } - } - - ret = gnutls_credentials_set(tlss->tls_session, GNUTLS_CRD_CERTIFICATE, - tlsp->x509_cred); - if (ret != GNUTLS_E_SUCCESS) { - DEBUG(0,("TLS %s - %s\n", __location__, gnutls_strerror(ret))); - tevent_req_error(req, EINVAL); - return tevent_req_post(req, ev); - } - - gnutls_certificate_server_set_request(tlss->tls_session, - GNUTLS_CERT_REQUEST); - gnutls_dh_set_prime_bits(tlss->tls_session, DH_BITS); - gnutls_transport_set_ptr(tlss->tls_session, (gnutls_transport_ptr_t)state->tls_stream); gnutls_transport_set_pull_function(tlss->tls_session, diff --git a/source4/lib/tls/wscript_build b/source4/lib/tls/wscript_build index 7980a633574b..8fe77da84892 100644 --- a/source4/lib/tls/wscript_build +++ b/source4/lib/tls/wscript_build @@ -8,6 +8,7 @@ bld.SAMBA_SUBSYSTEM('LIBTLS', public_deps=''' talloc gnutls + GNUTLS_HELPERS samba-hostconfig LIBTSOCKET tevent -- 2.34.1 From 59103837f0c8dce528c2aded1dc8f3fa16a41fe8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 17 Apr 2024 18:16:46 +0200 Subject: [PATCH 10/29] s4:lib/tls: we no longer need ifdef GNUTLS_NO_TICKETS We require gnutls 3.6.13 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit ecdd76919132430372ef04b03304fc51d6014e2f) --- source4/lib/tls/tls_tstream.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index 6145037cf8de..da946b0a1799 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -996,7 +996,6 @@ static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, flags = GNUTLS_SERVER; } else { flags = GNUTLS_CLIENT; -#ifdef GNUTLS_NO_TICKETS /* * tls_tstream can't properly handle 'New Session Ticket' * messages sent 'after' the client sends the 'Finished' @@ -1005,7 +1004,6 @@ static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, * use resumption with session tickets. */ flags |= GNUTLS_NO_TICKETS; -#endif } /* -- 2.34.1 From 389dad1d816e4a3e903cfb29db4c9208c3c13fbf Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 15 Mar 2024 23:24:39 +0100 Subject: [PATCH 11/29] s4:lib/tls: include a TLS server name indication in the client handshake This is not strictly needed, but it might be useful for load balancers. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 15fb8fcc7b98c3eba8eab79b227127b4b71b096c) --- source4/lib/tls/tls_tstream.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index da946b0a1799..298617961077 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -991,6 +991,7 @@ static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, struct tstream_tls_params_internal *tlsp = NULL; int ret; unsigned int flags; + const char *hostname = NULL; if (tlss->is_server) { flags = GNUTLS_SERVER; @@ -1024,10 +1025,20 @@ static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, tlss->verify_peer = tlsp->verify_peer; if (tlsp->peer_name != NULL) { + bool ip = is_ipaddress(tlsp->peer_name); + tlss->peer_name = talloc_strdup(tlss, tlsp->peer_name); if (tlss->peer_name == NULL) { return NT_STATUS_NO_MEMORY; } + + if (!ip) { + hostname = tlss->peer_name; + } + + if (tlss->verify_peer < TLS_VERIFY_PEER_CA_AND_NAME) { + hostname = NULL; + } } if (tlss->current_ev != NULL) { @@ -1069,6 +1080,17 @@ static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, NT_STATUS_CRYPTO_SYSTEM_INVALID); } + if (hostname != NULL) { + ret = gnutls_server_name_set(tlss->tls_session, + GNUTLS_NAME_DNS, + hostname, + strlen(hostname)); + if (ret != GNUTLS_E_SUCCESS) { + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); + } + } + if (tlss->is_server) { gnutls_certificate_server_set_request(tlss->tls_session, GNUTLS_CERT_REQUEST); -- 2.34.1 From 48ff7a2f665f0c2b75d3af20b0f248ead85ebf0e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 12 Feb 2024 12:02:13 +0100 Subject: [PATCH 12/29] s4:lib/tls: split out tstream_tls_verify_peer() helper BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 3186cdce85a58451e9d5a05468029a13621128c3) --- source4/lib/tls/tls_tstream.c | 147 +++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 66 deletions(-) diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index 298617961077..5ce0d7c9bb11 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -1100,6 +1100,76 @@ static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, return NT_STATUS_OK; } +static NTSTATUS tstream_tls_verify_peer(struct tstream_tls *tlss) +{ + unsigned int status = UINT32_MAX; + bool ip = true; + const char *hostname = NULL; + int ret; + + if (tlss->verify_peer == TLS_VERIFY_PEER_NO_CHECK) { + return NT_STATUS_OK; + } + + if (tlss->peer_name != NULL) { + ip = is_ipaddress(tlss->peer_name); + } + + if (!ip) { + hostname = tlss->peer_name; + } + + if (tlss->verify_peer == TLS_VERIFY_PEER_CA_ONLY) { + hostname = NULL; + } + + if (tlss->verify_peer >= TLS_VERIFY_PEER_CA_AND_NAME) { + if (hostname == NULL) { + DEBUG(1,("TLS %s - no hostname available for " + "verify_peer[%s] and peer_name[%s]\n", + __location__, + tls_verify_peer_string(tlss->verify_peer), + tlss->peer_name)); + return NT_STATUS_IMAGE_CERT_REVOKED; + } + } + + ret = gnutls_certificate_verify_peers3(tlss->tls_session, + hostname, + &status); + if (ret != GNUTLS_E_SUCCESS) { + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); + } + + if (status != 0) { + DEBUG(1,("TLS %s - check failed for " + "verify_peer[%s] and peer_name[%s] " + "status 0x%x (%s%s%s%s%s%s%s%s)\n", + __location__, + tls_verify_peer_string(tlss->verify_peer), + tlss->peer_name, + status, + status & GNUTLS_CERT_INVALID ? "invalid " : "", + status & GNUTLS_CERT_REVOKED ? "revoked " : "", + status & GNUTLS_CERT_SIGNER_NOT_FOUND ? + "signer_not_found " : "", + status & GNUTLS_CERT_SIGNER_NOT_CA ? + "signer_not_ca " : "", + status & GNUTLS_CERT_INSECURE_ALGORITHM ? + "insecure_algorithm " : "", + status & GNUTLS_CERT_NOT_ACTIVATED ? + "not_activated " : "", + status & GNUTLS_CERT_EXPIRED ? + "expired " : "", + status & GNUTLS_CERT_UNEXPECTED_OWNER ? + "unexpected_owner " : "")); + return NT_STATUS_IMAGE_CERT_REVOKED; + } + + return NT_STATUS_OK; +} + struct tstream_tls_connect_state { struct tstream_context *tls_stream; }; @@ -1415,6 +1485,7 @@ static void tstream_tls_retry_handshake(struct tstream_context *stream) tstream_context_data(stream, struct tstream_tls); struct tevent_req *req = tlss->handshake.req; + NTSTATUS status; int ret; if (tlss->error != 0) { @@ -1443,72 +1514,16 @@ static void tstream_tls_retry_handshake(struct tstream_context *stream) return; } - if (tlss->verify_peer >= TLS_VERIFY_PEER_CA_ONLY) { - unsigned int status = UINT32_MAX; - bool ip = true; - const char *hostname = NULL; - - if (tlss->peer_name != NULL) { - ip = is_ipaddress(tlss->peer_name); - } - - if (!ip) { - hostname = tlss->peer_name; - } - - if (tlss->verify_peer == TLS_VERIFY_PEER_CA_ONLY) { - hostname = NULL; - } - - if (tlss->verify_peer >= TLS_VERIFY_PEER_CA_AND_NAME) { - if (hostname == NULL) { - DEBUG(1,("TLS %s - no hostname available for " - "verify_peer[%s] and peer_name[%s]\n", - __location__, - tls_verify_peer_string(tlss->verify_peer), - tlss->peer_name)); - tlss->error = EINVAL; - tevent_req_error(req, tlss->error); - return; - } - } - - ret = gnutls_certificate_verify_peers3(tlss->tls_session, - hostname, - &status); - if (ret != GNUTLS_E_SUCCESS) { - DEBUG(1,("TLS %s - %s\n", __location__, gnutls_strerror(ret))); - tlss->error = EIO; - tevent_req_error(req, tlss->error); - return; - } - - if (status != 0) { - DEBUG(1,("TLS %s - check failed for " - "verify_peer[%s] and peer_name[%s] " - "status 0x%x (%s%s%s%s%s%s%s%s)\n", - __location__, - tls_verify_peer_string(tlss->verify_peer), - tlss->peer_name, - status, - status & GNUTLS_CERT_INVALID ? "invalid " : "", - status & GNUTLS_CERT_REVOKED ? "revoked " : "", - status & GNUTLS_CERT_SIGNER_NOT_FOUND ? - "signer_not_found " : "", - status & GNUTLS_CERT_SIGNER_NOT_CA ? - "signer_not_ca " : "", - status & GNUTLS_CERT_INSECURE_ALGORITHM ? - "insecure_algorithm " : "", - status & GNUTLS_CERT_NOT_ACTIVATED ? - "not_activated " : "", - status & GNUTLS_CERT_EXPIRED ? - "expired " : "", - status & GNUTLS_CERT_UNEXPECTED_OWNER ? - "unexpected_owner " : "")); - tlss->error = EINVAL; - tevent_req_error(req, tlss->error); - return; - } + status = tstream_tls_verify_peer(tlss); + if (NT_STATUS_EQUAL(status, NT_STATUS_IMAGE_CERT_REVOKED)) { + tlss->error = EINVAL; + tevent_req_error(req, tlss->error); + return; + } + if (!NT_STATUS_IS_OK(status)) { + tlss->error = EIO; + tevent_req_error(req, tlss->error); + return; } if (tlss->push.subreq != NULL || tlss->pull.subreq != NULL) { -- 2.34.1 From a11af8beda32e946e4009688fd10447318cd61b6 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Feb 2024 16:36:27 +0100 Subject: [PATCH 13/29] s4:lib/tls: add tstream_tls_params_client_lpcfg() This will be able simplify the callers a lot... BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 604413b98a23f28288ec4af11023717a9239e0fe) --- source4/lib/tls/tls.h | 5 ++++ source4/lib/tls/tls_tstream.c | 47 +++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/source4/lib/tls/tls.h b/source4/lib/tls/tls.h index d9b18ff4d083..5634cce516c2 100644 --- a/source4/lib/tls/tls.h +++ b/source4/lib/tls/tls.h @@ -63,6 +63,11 @@ NTSTATUS tstream_tls_params_client(TALLOC_CTX *mem_ctx, const char *peer_name, struct tstream_tls_params **_tlsp); +NTSTATUS tstream_tls_params_client_lpcfg(TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + const char *peer_name, + struct tstream_tls_params **tlsp); + NTSTATUS tstream_tls_params_server(TALLOC_CTX *mem_ctx, const char *dns_host_name, bool enabled, diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index 5ce0d7c9bb11..1d933bd6b65c 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -26,6 +26,7 @@ #include "../lib/tsocket/tsocket_internal.h" #include "../lib/util/util_net.h" #include "lib/tls/tls.h" +#include "lib/param/param.h" #include #include @@ -985,6 +986,52 @@ NTSTATUS tstream_tls_params_client(TALLOC_CTX *mem_ctx, return NT_STATUS_OK; } +NTSTATUS tstream_tls_params_client_lpcfg(TALLOC_CTX *mem_ctx, + struct loadparm_context *lp_ctx, + const char *peer_name, + struct tstream_tls_params **tlsp) +{ + TALLOC_CTX *frame = talloc_stackframe(); + const char *ptr = NULL; + char *ca_file = NULL; + char *crl_file = NULL; + const char *tls_priority = NULL; + enum tls_verify_peer_state verify_peer = + TLS_VERIFY_PEER_AS_STRICT_AS_POSSIBLE; + NTSTATUS status; + + ptr = lpcfg__tls_cafile(lp_ctx); + if (ptr != NULL) { + ca_file = lpcfg_tls_cafile(frame, lp_ctx); + if (ca_file == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + } + + ptr = lpcfg__tls_crlfile(lp_ctx); + if (ptr != NULL) { + crl_file = lpcfg_tls_crlfile(frame, lp_ctx); + if (crl_file == NULL) { + TALLOC_FREE(frame); + return NT_STATUS_NO_MEMORY; + } + } + + tls_priority = lpcfg_tls_priority(lp_ctx); + verify_peer = lpcfg_tls_verify_peer(lp_ctx); + + status = tstream_tls_params_client(mem_ctx, + ca_file, + crl_file, + tls_priority, + verify_peer, + peer_name, + tlsp); + TALLOC_FREE(frame); + return status; +} + static NTSTATUS tstream_tls_prepare_gnutls(struct tstream_tls_params *_tlsp, struct tstream_tls *tlss) { -- 2.34.1 From 522611287d33d651c190d8f14547cefcee136844 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Feb 2024 16:50:23 +0100 Subject: [PATCH 14/29] s3:rpc_server/mdssvc: make use of tstream_tls_params_client_lpcfg() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit b8b874ef5e40d266a54501ba4523c6af7032ca00) --- source3/rpc_server/mdssvc/mdssvc_es.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/source3/rpc_server/mdssvc/mdssvc_es.c b/source3/rpc_server/mdssvc/mdssvc_es.c index 8460b48b80aa..d51441092b42 100644 --- a/source3/rpc_server/mdssvc/mdssvc_es.c +++ b/source3/rpc_server/mdssvc/mdssvc_es.c @@ -29,6 +29,7 @@ #include "mdssvc.h" #include "mdssvc_es.h" #include "rpc_server/mdssvc/es_parser.tab.h" +#include "lib/param/param.h" #include @@ -246,18 +247,18 @@ static struct tevent_req *mds_es_connect_send( use_tls ? "S" : "", state->server_addr, state->server_port); if (use_tls) { - const char *ca_file = lp__tls_cafile(); - const char *crl_file = lp__tls_crlfile(); - const char *tls_priority = lp_tls_priority(); - enum tls_verify_peer_state verify_peer = lp_tls_verify_peer(); - - status = tstream_tls_params_client(state, - ca_file, - crl_file, - tls_priority, - verify_peer, - state->server_addr, - &state->tls_params); + struct loadparm_context *lp_ctx = NULL; + + lp_ctx = loadparm_init_s3(state, loadparm_s3_helpers()); + if (tevent_req_nomem(lp_ctx, req)) { + return tevent_req_post(req, ev); + } + + status = tstream_tls_params_client_lpcfg(state, + lp_ctx, + state->server_addr, + &state->tls_params); + TALLOC_FREE(lp_ctx); if (!NT_STATUS_IS_OK(status)) { DBG_ERR("Failed tstream_tls_params_client - %s\n", nt_errstr(status)); -- 2.34.1 From 9bdef923c048b5e534d1f517c3f6f780bca996d4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Feb 2024 16:52:56 +0100 Subject: [PATCH 15/29] s4:librpc/rpc: make use of tstream_tls_params_client_lpcfg() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 493d35a6910d9d9b70f55c2273f4e8a6c93a3bf5) --- source4/librpc/rpc/dcerpc_roh.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/source4/librpc/rpc/dcerpc_roh.c b/source4/librpc/rpc/dcerpc_roh.c index 3aa7551034a1..82ed5315805d 100644 --- a/source4/librpc/rpc/dcerpc_roh.c +++ b/source4/librpc/rpc/dcerpc_roh.c @@ -287,21 +287,13 @@ struct tevent_req *dcerpc_pipe_open_roh_send(struct dcecli_connection *conn, /* Initialize TLS */ if (use_tls) { - char *ca_file = lpcfg_tls_cafile(state, lp_ctx); - char *crl_file = lpcfg_tls_crlfile(state, lp_ctx); - const char *tls_priority = lpcfg_tls_priority(lp_ctx); - enum tls_verify_peer_state verify_peer = - lpcfg_tls_verify_peer(lp_ctx); - - status = tstream_tls_params_client(state->roh, - ca_file, crl_file, - tls_priority, - verify_peer, - state->rpc_proxy, - &state->tls_params); + status = tstream_tls_params_client_lpcfg(state->roh, + lp_ctx, + state->rpc_proxy, + &state->tls_params); if (!NT_STATUS_IS_OK(status)) { - DEBUG(0,("%s: Failed tstream_tls_params_client - %s\n", - __func__, nt_errstr(status))); + DBG_ERR("Failed tstream_tls_params_client_lpcfg - %s\n", + nt_errstr(status)); tevent_req_nterror(req, status); return tevent_req_post(req, conn->event_ctx); } -- 2.34.1 From ecfa770dd58dad1653735bfc1fb40e513903bc5f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Feb 2024 16:53:15 +0100 Subject: [PATCH 16/29] s4:libcli/ldap: make use of tstream_tls_params_client_lpcfg() BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c200cf1b5f430f686b39df8513a6b7e3c592ed43) --- source4/libcli/ldap/ldap_client.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c index 3c9f91601e69..61a49d345028 100644 --- a/source4/libcli/ldap/ldap_client.c +++ b/source4/libcli/ldap/ldap_client.c @@ -510,20 +510,12 @@ _PUBLIC_ struct composite_context *ldap_connect_send(struct ldap_connection *con conn->port = port; if (conn->ldaps) { - char *ca_file = lpcfg_tls_cafile(state, conn->lp_ctx); - char *crl_file = lpcfg_tls_crlfile(state, conn->lp_ctx); - const char *tls_priority = lpcfg_tls_priority(conn->lp_ctx); - enum tls_verify_peer_state verify_peer = - lpcfg_tls_verify_peer(conn->lp_ctx); NTSTATUS status; - status = tstream_tls_params_client(state, - ca_file, - crl_file, - tls_priority, - verify_peer, - conn->host, - &state->tls_params); + status = tstream_tls_params_client_lpcfg(state, + conn->lp_ctx, + conn->host, + &state->tls_params); if (!NT_STATUS_IS_OK(status)) { composite_error(result, status); return result; -- 2.34.1 From ad8114d14651d2a90710745f16e952ad640c6dd2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Mar 2024 09:55:47 +0100 Subject: [PATCH 17/29] lib/crypto: add legacy_gnutls_server_end_point_cb() if needed gnutls_session_channel_binding(GNUTLS_CB_TLS_SERVER_END_POINT) is only available with gnutls 3.7.2, but we still want to support older gnutls versions and that's easily doable... BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 2f2af3aa8a0366e6502751415a08413bf28ba0cb) --- lib/crypto/gnutls_helpers.h | 6 ++ lib/crypto/gnutls_server_end_point_cb.c | 130 ++++++++++++++++++++++++ lib/crypto/wscript | 6 +- wscript_configure_system_gnutls | 5 + 4 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 lib/crypto/gnutls_server_end_point_cb.c diff --git a/lib/crypto/gnutls_helpers.h b/lib/crypto/gnutls_helpers.h index 0362d5ee7822..6699ebc01968 100644 --- a/lib/crypto/gnutls_helpers.h +++ b/lib/crypto/gnutls_helpers.h @@ -233,4 +233,10 @@ NTSTATUS samba_gnutls_sp800_108_derive_key( uint8_t *KO, size_t KO_len); +#ifndef HAVE_GNUTLS_CB_TLS_SERVER_END_POINT +int legacy_gnutls_server_end_point_cb(gnutls_session_t session, + bool is_server, + gnutls_datum_t * cb); +#endif /* HAVE_GNUTLS_CB_TLS_SERVER_END_POINT */ + #endif /* _GNUTLS_HELPERS_H */ diff --git a/lib/crypto/gnutls_server_end_point_cb.c b/lib/crypto/gnutls_server_end_point_cb.c new file mode 100644 index 000000000000..c90919746408 --- /dev/null +++ b/lib/crypto/gnutls_server_end_point_cb.c @@ -0,0 +1,130 @@ +/* + * Copyright (C) 2002-2016 Free Software Foundation, Inc. + * Copyright (C) 2014-2016 Nikos Mavrogiannopoulos + * Copyright (C) 2015-2018 Red Hat, Inc. + * + * Author: Nikos Mavrogiannopoulos + * + * This file is part of GnuTLS. + * + * The GnuTLS is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public License + * as published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see + * + */ + +#include "replace.h" +#include "gnutls_helpers.h" +#include +#include + +int legacy_gnutls_server_end_point_cb(gnutls_session_t session, + bool is_server, + gnutls_datum_t * cb) +{ + /* + * copied from the logic in gnutls_session_channel_binding() + * introduced by gnutls commit (as LGPL 2.1+): + * + * commit 9ebee00c793e40e3e8c797c645577c9e025b9f1e + * Author: Ruslan N. Marchenko + * Date: Sat May 1 23:05:54 2021 +0200 + * + * Add tls-server-end-point tls channel binding implementation. + * ... + */ + const gnutls_datum_t *ders = NULL; + unsigned int num_certs = 1; + int ret; + size_t rlen; + gnutls_x509_crt_t cert; + gnutls_digest_algorithm_t algo; + + /* Only X509 certificates are supported for this binding type */ + ret = gnutls_certificate_type_get(session); + if (ret != GNUTLS_CRT_X509) { + return GNUTLS_E_UNIMPLEMENTED_FEATURE; + } + + if (is_server) { + ders = gnutls_certificate_get_ours(session); + } else { + ders = gnutls_certificate_get_peers(session, &num_certs); + } + + /* Previous check indicated we have x509 but you never know */ + if (!ders || num_certs == 0) { + return GNUTLS_E_UNIMPLEMENTED_FEATURE; + } + + ret = gnutls_x509_crt_list_import(&cert, + &num_certs, + ders, + GNUTLS_X509_FMT_DER, + 0); + /* Again, this is not supposed to happen (normally) */ + if (ret < 0 || num_certs == 0) { + return GNUTLS_E_CHANNEL_BINDING_NOT_AVAILABLE; + } + + /* Obtain signature algorithm used by certificate */ + ret = gnutls_x509_crt_get_signature_algorithm(cert); + if (ret < 0 || ret == GNUTLS_SIGN_UNKNOWN) { + gnutls_x509_crt_deinit(cert); + return GNUTLS_E_UNIMPLEMENTED_FEATURE; + } + + /* obtain hash function from signature and normalize it */ + algo = gnutls_sign_get_hash_algorithm(ret); + switch (algo) { + case GNUTLS_DIG_MD5: + case GNUTLS_DIG_SHA1: + algo = GNUTLS_DIG_SHA256; + break; + case GNUTLS_DIG_UNKNOWN: + case GNUTLS_DIG_NULL: + case GNUTLS_DIG_MD5_SHA1: + /* double hashing not supported either */ + gnutls_x509_crt_deinit(cert); + return GNUTLS_E_UNIMPLEMENTED_FEATURE; + default: + break; + } + + /* preallocate 512 bits buffer as maximum supported digest */ + rlen = 64; + cb->data = gnutls_malloc(rlen); + if (cb->data == NULL) { + gnutls_x509_crt_deinit(cert); + return GNUTLS_E_MEMORY_ERROR; + } + + ret = gnutls_x509_crt_get_fingerprint(cert, + algo, + cb->data, + &rlen); + if (ret == GNUTLS_E_SHORT_MEMORY_BUFFER) { + cb->data = gnutls_realloc(cb->data, cb->size); + if (cb->data == NULL) { + gnutls_x509_crt_deinit(cert); + return GNUTLS_E_MEMORY_ERROR; + } + ret = gnutls_x509_crt_get_fingerprint(cert, + algo, + cb->data, + &rlen); + } + + cb->size = rlen; + gnutls_x509_crt_deinit(cert); + return ret; +} diff --git a/lib/crypto/wscript b/lib/crypto/wscript index eaa18c552578..5f8c47474037 100644 --- a/lib/crypto/wscript +++ b/lib/crypto/wscript @@ -2,6 +2,10 @@ def build(bld): + legacy_gnutls_helpers = '' + if not bld.CONFIG_SET('HAVE_GNUTLS_CB_TLS_SERVER_END_POINT'): + legacy_gnutls_helpers += ' gnutls_server_end_point_cb.c' + bld.SAMBA_SUBSYSTEM("GNUTLS_HELPERS", source=''' gnutls_error.c @@ -9,7 +13,7 @@ def build(bld): gnutls_arcfour_confounded_md5.c gnutls_weak_crypto.c gnutls_sp800_108.c - ''', + ''' + legacy_gnutls_helpers, deps="gnutls samba-errors") bld.SAMBA_SUBSYSTEM('LIBCRYPTO', diff --git a/wscript_configure_system_gnutls b/wscript_configure_system_gnutls index 1983a0cdfe94..f62de5560d63 100644 --- a/wscript_configure_system_gnutls +++ b/wscript_configure_system_gnutls @@ -34,6 +34,11 @@ conf.SET_TARGET_TYPE('gnutls', 'SYSLIB') if (gnutls_version > parse_version('3.6.14')): conf.DEFINE('ALLOW_GNUTLS_AEAD_CIPHER_ENCRYPTV2_AES_CCM', 1) +# GNUTLS_CB_TLS_SERVER_END_POINT is available with +# 3.7.2 +if (gnutls_version >= parse_version('3.7.2')): + conf.DEFINE('HAVE_GNUTLS_CB_TLS_SERVER_END_POINT', 1) + # Check if gnutls has fips mode support # gnutls_fips140_mode_enabled() is available since 3.3.0 fragment = ''' -- 2.34.1 From 1af12862a85389c9e1b05f567aa1d06bde701d62 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 28 Sep 2023 12:34:35 +0200 Subject: [PATCH 18/29] s4:lib/tls: add tstream_tls_channel_bindings() This is based on GNUTLS_CB_TLS_SERVER_END_POINT and is the value that is required for channel bindings in LDAP of active directory domain controllers. For gnutls versions before 3.7.2 we basically copied the code from the GNUTLS_CB_TLS_SERVER_END_POINT implementation as it only uses public gnutls functions and it was easy to re-implement. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit cbd7ce44121246167e0c8a6d905180d82df1a2ef) --- source4/lib/tls/tls.h | 2 ++ source4/lib/tls/tls_tstream.c | 66 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/source4/lib/tls/tls.h b/source4/lib/tls/tls.h index 5634cce516c2..03063204e853 100644 --- a/source4/lib/tls/tls.h +++ b/source4/lib/tls/tls.h @@ -81,6 +81,8 @@ NTSTATUS tstream_tls_params_server(TALLOC_CTX *mem_ctx, bool tstream_tls_params_enabled(struct tstream_tls_params *params); +const DATA_BLOB *tstream_tls_channel_bindings(struct tstream_context *tls_tstream); + struct tevent_req *_tstream_tls_connect_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct tstream_context *plain_stream, diff --git a/source4/lib/tls/tls_tstream.c b/source4/lib/tls/tls_tstream.c index 1d933bd6b65c..e34937481203 100644 --- a/source4/lib/tls/tls_tstream.c +++ b/source4/lib/tls/tls_tstream.c @@ -69,6 +69,8 @@ struct tstream_tls { enum tls_verify_peer_state verify_peer; const char *peer_name; + DATA_BLOB channel_bindings; + struct tevent_context *current_ev; struct tevent_immediate *retry_im; @@ -889,6 +891,63 @@ bool tstream_tls_params_enabled(struct tstream_tls_params *tls_params) return tlsp->tls_enabled; } +static NTSTATUS tstream_tls_setup_channel_bindings(struct tstream_tls *tlss) +{ + gnutls_datum_t cb = { .size = 0 }; + int ret; + +#ifdef HAVE_GNUTLS_CB_TLS_SERVER_END_POINT + ret = gnutls_session_channel_binding(tlss->tls_session, + GNUTLS_CB_TLS_SERVER_END_POINT, + &cb); +#else /* not HAVE_GNUTLS_CB_TLS_SERVER_END_POINT */ + ret = legacy_gnutls_server_end_point_cb(tlss->tls_session, + tlss->is_server, + &cb); +#endif /* not HAVE_GNUTLS_CB_TLS_SERVER_END_POINT */ + if (ret != GNUTLS_E_SUCCESS) { + return gnutls_error_to_ntstatus(ret, + NT_STATUS_CRYPTO_SYSTEM_INVALID); + } + + if (cb.size != 0) { + /* + * Looking at the OpenLDAP implementation + * for LDAP_OPT_X_SASL_CBINDING_TLS_ENDPOINT + * revealed that we need to prefix it with + * 'tls-server-end-point:' + */ + const char endpoint_prefix[] = "tls-server-end-point:"; + size_t prefix_size = strlen(endpoint_prefix); + size_t size = prefix_size + cb.size; + + tlss->channel_bindings = data_blob_talloc_named(tlss, NULL, size, + "tls_channel_bindings"); + if (tlss->channel_bindings.data == NULL) { + gnutls_free(cb.data); + return NT_STATUS_NO_MEMORY; + } + memcpy(tlss->channel_bindings.data, endpoint_prefix, prefix_size); + memcpy(tlss->channel_bindings.data + prefix_size, cb.data, cb.size); + gnutls_free(cb.data); + } + + return NT_STATUS_OK; +} + +const DATA_BLOB *tstream_tls_channel_bindings(struct tstream_context *tls_tstream) +{ + struct tstream_tls *tlss = + talloc_get_type(_tstream_context_data(tls_tstream), + struct tstream_tls); + + if (tlss == NULL) { + return NULL; + } + + return &tlss->channel_bindings; +} + NTSTATUS tstream_tls_params_client(TALLOC_CTX *mem_ctx, const char *ca_file, const char *crl_file, @@ -1573,6 +1632,13 @@ static void tstream_tls_retry_handshake(struct tstream_context *stream) return; } + status = tstream_tls_setup_channel_bindings(tlss); + if (!NT_STATUS_IS_OK(status)) { + tlss->error = EIO; + tevent_req_error(req, tlss->error); + return; + } + if (tlss->push.subreq != NULL || tlss->pull.subreq != NULL) { tlss->waiting_flush.mgmt_req = req; return; -- 2.34.1 From dde23e5979e328455a83e32ccb82fdaee6cf87e9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Mar 2024 10:30:55 +0100 Subject: [PATCH 19/29] third_party/heimdal: import lorikeet-heimdal-202404171655 (commit 28a56d818074e049f0361ef74d7017f2a9391847) BUG: https://bugzilla.samba.org/show_bug.cgi?id=15603 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 See also: https://github.com/heimdal/heimdal/pull/1234 https://github.com/heimdal/heimdal/pull/1238 https://github.com/heimdal/heimdal/pull/1240 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 9b92cbacac11fb64cca2c4770cbdce789525b87a) --- third_party/heimdal/lib/gssapi/krb5/8003.c | 5 + .../lib/gssapi/krb5/init_sec_context.c | 10 ++ third_party/heimdal/lib/gssapi/test_context.c | 4 + third_party/heimdal/lib/krb5/build_auth.c | 100 +++++++++++++++++- third_party/heimdal/lib/krb5/mk_req_ext.c | 1 + .../heimdal/tests/gss/check-context.in | 35 ++++++ 6 files changed, 153 insertions(+), 2 deletions(-) diff --git a/third_party/heimdal/lib/gssapi/krb5/8003.c b/third_party/heimdal/lib/gssapi/krb5/8003.c index 0b91cf83b82d..74ff349ab7b3 100644 --- a/third_party/heimdal/lib/gssapi/krb5/8003.c +++ b/third_party/heimdal/lib/gssapi/krb5/8003.c @@ -87,6 +87,11 @@ _gsskrb5_create_8003_checksum ( { u_char *p; +#define _GSS_C_NON_8003_WIRE_FLAGS \ + GSS_C_CHANNEL_BOUND_FLAG + + flags &= ~_GSS_C_NON_8003_WIRE_FLAGS; + /* * see rfc1964 (section 1.1.1 (Initial Token), and the checksum value * field's format) */ diff --git a/third_party/heimdal/lib/gssapi/krb5/init_sec_context.c b/third_party/heimdal/lib/gssapi/krb5/init_sec_context.c index 7749fc6cadd8..fbf9e5521c2f 100644 --- a/third_party/heimdal/lib/gssapi/krb5/init_sec_context.c +++ b/third_party/heimdal/lib/gssapi/krb5/init_sec_context.c @@ -498,6 +498,7 @@ init_auth_restart krb5_data fwd_data, timedata; int32_t offset = 0, oldoffset = 0; uint32_t flagmask; + krb5_boolean channel_bound = FALSE; krb5_data_zero(&outbuf); krb5_data_zero(&fwd_data); @@ -587,6 +588,11 @@ init_auth_restart } flags |= GSS_C_TRANS_FLAG; + if (req_flags & GSS_C_CHANNEL_BOUND_FLAG) { + flags |= GSS_C_CHANNEL_BOUND_FLAG; + channel_bound = TRUE; + } + if (ret_flags) *ret_flags = flags; ctx->flags = flags; @@ -626,6 +632,7 @@ init_auth_restart enctype, ctx->kcred, &cksum, + channel_bound, &authenticator, KRB5_KU_AP_REQ_AUTH); @@ -787,6 +794,9 @@ repl_mutual return GSS_S_FAILURE; } } + if (ret != GSS_S_COMPLETE) { + return ret; + } kret = krb5_rd_rep (context, ctx->auth_context, &indata, diff --git a/third_party/heimdal/lib/gssapi/test_context.c b/third_party/heimdal/lib/gssapi/test_context.c index 907e9c4d2086..e6d505033c3c 100644 --- a/third_party/heimdal/lib/gssapi/test_context.c +++ b/third_party/heimdal/lib/gssapi/test_context.c @@ -82,6 +82,7 @@ static int token_split = 0; static int version_flag = 0; static int verbose_flag = 0; static int help_flag = 0; +static int i_channel_bound = 0; static char *i_channel_bindings = NULL; static char *a_channel_bindings = NULL; @@ -287,6 +288,8 @@ loop(gss_OID mechoid, flags |= GSS_C_DELEG_FLAG; if (policy_deleg_flag) flags |= GSS_C_DELEG_POLICY_FLAG; + if (i_channel_bound) + flags |= GSS_C_CHANNEL_BOUND_FLAG; input_token.value = rk_UNCONST(target); input_token.length = strlen(target); @@ -904,6 +907,7 @@ static struct getargs args[] = { {"client-name", 0, arg_string, &client_name, "client name", NULL }, {"client-password", 0, arg_string, &client_password, "client password", NULL }, {"anonymous", 0, arg_flag, &anon_flag, "anonymous auth", NULL }, + {"i-channel-bound",0, arg_flag, &i_channel_bound, "initiator channel bound", NULL }, {"i-channel-bindings", 0, arg_string, &i_channel_bindings, "initiator channel binding data", NULL }, {"a-channel-bindings", 0, arg_string, &a_channel_bindings, "acceptor channel binding data", NULL }, {"limit-enctype",0, arg_string, &limit_enctype_string, "enctype", NULL }, diff --git a/third_party/heimdal/lib/krb5/build_auth.c b/third_party/heimdal/lib/krb5/build_auth.c index 3e0012562a3c..ae757f7b043b 100644 --- a/third_party/heimdal/lib/krb5/build_auth.c +++ b/third_party/heimdal/lib/krb5/build_auth.c @@ -86,6 +86,7 @@ add_etypelist(krb5_context context, static krb5_error_code add_ap_options(krb5_context context, + krb5_boolean channel_bound, krb5_authdata *auth_data) { krb5_error_code ret; @@ -98,6 +99,9 @@ add_ap_options(krb5_context context, "client_aware_channel_bindings", NULL); + if (channel_bound) + require_cb = TRUE; + if (!require_cb) return 0; @@ -115,8 +119,90 @@ add_ap_options(krb5_context context, return ret; } +static krb5_error_code +add_target_principal(krb5_context context, + krb5_const_principal server, + krb5_authdata *auth_data) +{ + krb5_error_code ret; + AuthorizationDataElement ade; + char *s, *s2 = NULL; + size_t s2_len; + + if (server == NULL) { + return 0; + } + + ret = krb5_unparse_name_flags(context, server, + KRB5_PRINCIPAL_UNPARSE_DISPLAY, + &s); + if (ret) + return ret; + + { + size_t ucs2_len; + uint16_t *ucs2; + unsigned int flags; + + ret = wind_utf8ucs2_length(s, &ucs2_len); + if (ret) { + krb5_set_error_message(context, ret, "Principal %s is not valid UTF-8", s); + free(s); + return ret; + } + + ucs2 = malloc(sizeof(ucs2[0]) * ucs2_len); + if (ucs2 == NULL) { + free(s); + return krb5_enomem(context); + } + + ret = wind_utf8ucs2(s, ucs2, &ucs2_len); + if (ret) { + free(ucs2); + krb5_set_error_message(context, ret, "Principal %s is not valid UTF-8", s); + free(s); + return ret; + } else + free(s); + + s2_len = (ucs2_len + 1) * 2; + s2 = malloc(s2_len); + if (s2 == NULL) { + free(ucs2); + return krb5_enomem(context); + } + + flags = WIND_RW_LE; + ret = wind_ucs2write(ucs2, ucs2_len, + &flags, s2, &s2_len); + free(ucs2); + if (ret) { + free(s2); + krb5_set_error_message(context, ret, "Failed to write to UCS-2 buffer"); + return ret; + } + + /* + * we do not want zero termination + */ + s2_len = ucs2_len * 2; + } + + ade.ad_type = KRB5_AUTHDATA_TARGET_PRINCIPAL; + ade.ad_data.length = s2_len; + ade.ad_data.data = s2; + + ret = add_AuthorizationData(auth_data, &ade); + free(s2); + + return ret; +} + static krb5_error_code make_ap_authdata(krb5_context context, + krb5_boolean channel_bound, + krb5_const_principal server, krb5_authdata **auth_data) { krb5_error_code ret; @@ -136,7 +222,13 @@ make_ap_authdata(krb5_context context, * in the AP authenticator when looking for AD-AP-OPTIONS. Make sure to * bundle it together with etypes. */ - ret = add_ap_options(context, &ad); + ret = add_ap_options(context, channel_bound, &ad); + if (ret) { + free_AuthorizationData(&ad); + return ret; + } + + ret = add_target_principal(context, server, &ad); if (ret) { free_AuthorizationData(&ad); return ret; @@ -165,6 +257,7 @@ _krb5_build_authenticator (krb5_context context, krb5_enctype enctype, krb5_creds *cred, Checksum *cksum, + krb5_boolean channel_bound, krb5_data *result, krb5_key_usage usage) { @@ -221,7 +314,10 @@ _krb5_build_authenticator (krb5_context context, * This is not GSS-API specific, we only enable it for * GSS for now */ - ret = make_ap_authdata(context, &auth.authorization_data); + ret = make_ap_authdata(context, + channel_bound, + cred->server, + &auth.authorization_data); if (ret) goto fail; } diff --git a/third_party/heimdal/lib/krb5/mk_req_ext.c b/third_party/heimdal/lib/krb5/mk_req_ext.c index a8a07f1c718f..09c116cd97b2 100644 --- a/third_party/heimdal/lib/krb5/mk_req_ext.c +++ b/third_party/heimdal/lib/krb5/mk_req_ext.c @@ -117,6 +117,7 @@ _krb5_mk_req_internal(krb5_context context, ac->keyblock->keytype, in_creds, c_opt, + FALSE, /* channel_bound */ &authenticator, encrypt_usage); if (c_opt) diff --git a/third_party/heimdal/tests/gss/check-context.in b/third_party/heimdal/tests/gss/check-context.in index 2b866d2f7242..9863d2fbe8f0 100644 --- a/third_party/heimdal/tests/gss/check-context.in +++ b/third_party/heimdal/tests/gss/check-context.in @@ -390,6 +390,41 @@ for mech in krb5 spnego; do --mech-type=$mech host@lucid.test.h5l.se 2>/dev/null && \ { eval "$testfailed"; } + echo "${mech}: initiator null bindings bound (client-aware-flag)" ; > messages.log + ${context} -v --i-channel-bound \ + --mech-type=$mech host@lucid.test.h5l.se > cbinding.log || \ + { eval "$testfailed"; } + grep "sflags:" cbinding.log | grep "channel-bound" > /dev/null && \ + { echo "channel-bound flag unexpected"; eval "$testfailed"; } + + echo "${mech}: initiator only bindings (client-aware-flag)" ; > messages.log + ${context} -v --i-channel-bound \ + --i-channel-bindings=abc \ + --mech-type=$mech host@lucid.test.h5l.se > cbinding.log || \ + { eval "$testfailed"; } + grep "sflags:" cbinding.log | grep "channel-bound" > /dev/null && \ + { echo "channel-bound flag unexpected"; eval "$testfailed"; } + + echo "${mech}: acceptor only bindings (client-aware-flag)" ; > messages.log + ${context} -v --i-channel-bound \ + --a-channel-bindings=abc \ + --mech-type=$mech host@lucid.test.h5l.se 2>/dev/null && \ + { eval "$testfailed"; } + + echo "${mech}: matching bindings (client-aware-flag)" ; > messages.log + ${context} -v --i-channel-bound \ + --i-channel-bindings=abc --a-channel-bindings=abc \ + --mech-type=$mech host@lucid.test.h5l.se > cbinding.log || \ + { eval "$testfailed"; } + grep "sflags:" cbinding.log | grep "channel-bound" > /dev/null || \ + { echo "no channel-bound flag"; eval "$testfailed"; } + + echo "${mech}: non matching bindings (client-aware-flag)" ; > messages.log + ${context} -v --i-channel-bound \ + --i-channel-bindings=abc --a-channel-bindings=xyz \ + --mech-type=$mech host@lucid.test.h5l.se 2>/dev/null && \ + { eval "$testfailed"; } + done #echo "sasl-digest-md5" -- 2.34.1 From a5696d41948d4c0c03d81c16f247082955e089e8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 5 Apr 2024 16:07:50 +0200 Subject: [PATCH 20/29] wscript_configure_embedded_heimdal: define HAVE_CLIENT_GSS_C_CHANNEL_BOUND_FLAG See https://github.com/heimdal/heimdal/pull/1234 and https://github.com/krb5/krb5/pull/1329. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 546e39a6fa122e6a40d1e62724e1712882ce3bce) --- wscript_configure_embedded_heimdal | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/wscript_configure_embedded_heimdal b/wscript_configure_embedded_heimdal index 6066f2b39d79..45f47721dec4 100644 --- a/wscript_configure_embedded_heimdal +++ b/wscript_configure_embedded_heimdal @@ -6,3 +6,10 @@ if not conf.env['BISON']: conf.define('USING_EMBEDDED_HEIMDAL', 1) conf.RECURSE('third_party/heimdal_build') + +# +# See https://github.com/heimdal/heimdal/pull/1234 +# and https://github.com/krb5/krb5/pull/1329 +# when this will be available also in +# system libraries... +conf.define('HAVE_CLIENT_GSS_C_CHANNEL_BOUND_FLAG', 1) -- 2.34.1 From 18dcc2577c6165ab6ceea76c59c573f58c5027bd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 11 Feb 2020 15:26:07 +0100 Subject: [PATCH 21/29] auth/gensec: add gensec_set_channel_bindings() function BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit e912ba579b1469c78ca65345ec1fe8376c74272c) --- auth/gensec/gensec.c | 63 +++++++++++++++++++++++++++++++++++ auth/gensec/gensec.h | 8 +++++ auth/gensec/gensec_internal.h | 18 ++++++++++ auth/gensec/gensec_start.c | 1 + 4 files changed, 90 insertions(+) diff --git a/auth/gensec/gensec.c b/auth/gensec/gensec.c index 26b5865bff5d..8785e69be635 100644 --- a/auth/gensec/gensec.c +++ b/auth/gensec/gensec.c @@ -854,3 +854,66 @@ _PUBLIC_ const char *gensec_get_target_principal(struct gensec_security *gensec_ return NULL; } + +static int gensec_channel_bindings_destructor(struct gensec_channel_bindings *cb) +{ + data_blob_clear_free(&cb->initiator_address); + data_blob_clear_free(&cb->acceptor_address); + data_blob_clear_free(&cb->application_data); + *cb = (struct gensec_channel_bindings) { .initiator_addrtype = 0, }; + return 0; +} + +_PUBLIC_ NTSTATUS gensec_set_channel_bindings(struct gensec_security *gensec_security, + uint32_t initiator_addrtype, + const DATA_BLOB *initiator_address, + uint32_t acceptor_addrtype, + const DATA_BLOB *acceptor_address, + const DATA_BLOB *application_data) +{ + struct gensec_channel_bindings *cb = NULL; + + if (gensec_security->subcontext) { + return NT_STATUS_INTERNAL_ERROR; + } + + if (gensec_security->channel_bindings != NULL) { + return NT_STATUS_ALREADY_REGISTERED; + } + + cb = talloc_zero(gensec_security, struct gensec_channel_bindings); + if (cb == NULL) { + return NT_STATUS_NO_MEMORY; + } + talloc_set_destructor(cb, gensec_channel_bindings_destructor); + + cb->initiator_addrtype = initiator_addrtype; + if (initiator_address != NULL) { + cb->initiator_address = data_blob_dup_talloc(cb, + *initiator_address); + if (cb->initiator_address.length != initiator_address->length) { + TALLOC_FREE(cb); + return NT_STATUS_NO_MEMORY; + } + } + cb->acceptor_addrtype = acceptor_addrtype; + if (acceptor_address != NULL) { + cb->acceptor_address = data_blob_dup_talloc(cb, + *acceptor_address); + if (cb->acceptor_address.length != acceptor_address->length) { + TALLOC_FREE(cb); + return NT_STATUS_NO_MEMORY; + } + } + if (application_data != NULL) { + cb->application_data = data_blob_dup_talloc(cb, + *application_data); + if (cb->application_data.length != application_data->length) { + TALLOC_FREE(cb); + return NT_STATUS_NO_MEMORY; + } + } + + gensec_security->channel_bindings = cb; + return NT_STATUS_OK; +} diff --git a/auth/gensec/gensec.h b/auth/gensec/gensec.h index 29d5e92c130f..25242384f555 100644 --- a/auth/gensec/gensec.h +++ b/auth/gensec/gensec.h @@ -70,6 +70,7 @@ struct gensec_target { #define GENSEC_FEATURE_NO_AUTHZ_LOG 0x00000800 #define GENSEC_FEATURE_SMB_TRANSPORT 0x00001000 #define GENSEC_FEATURE_LDAPS_TRANSPORT 0x00002000 +#define GENSEC_FEATURE_CB_OPTIONAL 0x00004000 #define GENSEC_EXPIRE_TIME_INFINITY (NTTIME)0x8000000000000000LL @@ -313,6 +314,13 @@ bool gensec_setting_bool(struct gensec_settings *settings, const char *mechanism NTSTATUS gensec_set_target_principal(struct gensec_security *gensec_security, const char *principal); const char *gensec_get_target_principal(struct gensec_security *gensec_security); +NTSTATUS gensec_set_channel_bindings(struct gensec_security *gensec_security, + uint32_t initiator_addrtype, + const DATA_BLOB *initiator_address, + uint32_t acceptor_addrtype, + const DATA_BLOB *acceptor_address, + const DATA_BLOB *application_data); + NTSTATUS gensec_generate_session_info_pac(TALLOC_CTX *mem_ctx, struct gensec_security *gensec_security, struct smb_krb5_context *smb_krb5_context, diff --git a/auth/gensec/gensec_internal.h b/auth/gensec/gensec_internal.h index 8efb1bdff0fb..4d8eca998816 100644 --- a/auth/gensec/gensec_internal.h +++ b/auth/gensec/gensec_internal.h @@ -95,6 +95,23 @@ struct gensec_security_ops_wrapper { const char *oid; }; +/* + * typedef struct gss_channel_bindings_struct { + * OM_uint32 initiator_addrtype; + * gss_buffer_desc initiator_address; + * OM_uint32 acceptor_addrtype; + * gss_buffer_desc acceptor_address; + * gss_buffer_desc application_data; + * } *gss_channel_bindings_t; + */ +struct gensec_channel_bindings { + uint32_t initiator_addrtype; + DATA_BLOB initiator_address; + uint32_t acceptor_addrtype; + DATA_BLOB acceptor_address; + DATA_BLOB application_data; +}; + struct gensec_security { const struct gensec_security_ops *ops; void *private_data; @@ -106,6 +123,7 @@ struct gensec_security { uint32_t max_update_size; uint8_t dcerpc_auth_level; struct tsocket_address *local_addr, *remote_addr; + struct gensec_channel_bindings *channel_bindings; struct gensec_settings *settings; /* When we are a server, this may be filled in to provide an diff --git a/auth/gensec/gensec_start.c b/auth/gensec/gensec_start.c index bcf98bd59687..4405aca278df 100644 --- a/auth/gensec/gensec_start.c +++ b/auth/gensec/gensec_start.c @@ -732,6 +732,7 @@ _PUBLIC_ NTSTATUS gensec_subcontext_start(TALLOC_CTX *mem_ctx, (*gensec_security)->auth_context = talloc_reference(*gensec_security, parent->auth_context); (*gensec_security)->settings = talloc_reference(*gensec_security, parent->settings); (*gensec_security)->auth_context = talloc_reference(*gensec_security, parent->auth_context); + (*gensec_security)->channel_bindings = talloc_reference(*gensec_security, parent->channel_bindings); talloc_set_destructor((*gensec_security), gensec_security_destructor); return NT_STATUS_OK; -- 2.34.1 From 548aa6e6dea7fc0e048c3d34c93a2ea27fe526b1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 11 Feb 2020 16:07:05 +0100 Subject: [PATCH 22/29] auth/ntlmssp: implement channel binding support BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit f1d34a430d227e685e2fe983b14c74136d9c8a8e) --- auth/ntlmssp/ntlmssp_client.c | 13 ++--- auth/ntlmssp/ntlmssp_private.h | 2 + auth/ntlmssp/ntlmssp_server.c | 47 ++++++++++++++++ auth/ntlmssp/ntlmssp_util.c | 98 ++++++++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 6 deletions(-) diff --git a/auth/ntlmssp/ntlmssp_client.c b/auth/ntlmssp/ntlmssp_client.c index 337aeed92299..d8dc1d2940bf 100644 --- a/auth/ntlmssp/ntlmssp_client.c +++ b/auth/ntlmssp/ntlmssp_client.c @@ -599,6 +599,8 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *gensec_security, SingleHost->Value.AvSingleHost.remaining = data_blob_null; } + if (!(gensec_security->want_features & GENSEC_FEATURE_CB_OPTIONAL) + || gensec_security->channel_bindings != NULL) { struct AV_PAIR *ChannelBindings = NULL; @@ -607,13 +609,12 @@ NTSTATUS ntlmssp_client_challenge(struct gensec_security *gensec_security, count++; *eol = *ChannelBindings; - /* - * gensec doesn't support channel bindings yet, - * but we want to match Windows on the wire - */ ChannelBindings->AvId = MsvChannelBindings; - memset(ChannelBindings->Value.ChannelBindings, 0, - sizeof(ChannelBindings->Value.ChannelBindings)); + nt_status = ntlmssp_hash_channel_bindings(gensec_security, + ChannelBindings->Value.ChannelBindings); + if (!NT_STATUS_IS_OK(nt_status)) { + return nt_status; + } } service = gensec_get_target_service(gensec_security); diff --git a/auth/ntlmssp/ntlmssp_private.h b/auth/ntlmssp/ntlmssp_private.h index 4d84e3347b6c..7b939b80ae21 100644 --- a/auth/ntlmssp/ntlmssp_private.h +++ b/auth/ntlmssp/ntlmssp_private.h @@ -56,6 +56,8 @@ void debug_ntlmssp_flags(uint32_t neg_flags); NTSTATUS ntlmssp_handle_neg_flags(struct ntlmssp_state *ntlmssp_state, uint32_t neg_flags, const char *name); const DATA_BLOB ntlmssp_version_blob(void); +NTSTATUS ntlmssp_hash_channel_bindings(struct gensec_security *gensec_security, + uint8_t cb_hash[16]); /* The following definitions come from auth/ntlmssp_server.c */ diff --git a/auth/ntlmssp/ntlmssp_server.c b/auth/ntlmssp/ntlmssp_server.c index 64b96283eb2f..1e49379a8ed6 100644 --- a/auth/ntlmssp/ntlmssp_server.c +++ b/auth/ntlmssp/ntlmssp_server.c @@ -386,6 +386,9 @@ static NTSTATUS ntlmssp_server_preauth(struct gensec_security *gensec_security, DATA_BLOB version_blob = data_blob_null; const unsigned int mic_len = NTLMSSP_MIC_SIZE; DATA_BLOB mic_blob = data_blob_null; + const uint8_t zero_channel_bindings[16] = { 0, }; + const uint8_t *client_channel_bindings = zero_channel_bindings; + uint8_t server_channel_bindings[16] = { 0, }; const char *parse_string; bool ok; struct timeval endtime; @@ -523,6 +526,7 @@ static NTSTATUS ntlmssp_server_preauth(struct gensec_security *gensec_security, uint32_t i = 0; uint32_t count = 0; const struct AV_PAIR *flags = NULL; + const struct AV_PAIR *cb = NULL; const struct AV_PAIR *eol = NULL; uint32_t av_flags = 0; @@ -598,6 +602,12 @@ static NTSTATUS ntlmssp_server_preauth(struct gensec_security *gensec_security, ntlmssp_state->new_spnego = true; } + cb = ndr_ntlmssp_find_av(&v2_resp.Challenge.AvPairs, + MsvChannelBindings); + if (cb != NULL) { + client_channel_bindings = cb->Value.ChannelBindings; + } + count = ntlmssp_state->server.av_pair_list.count; if (v2_resp.Challenge.AvPairs.count < count) { return NT_STATUS_INVALID_PARAMETER; @@ -700,6 +710,43 @@ static NTSTATUS ntlmssp_server_preauth(struct gensec_security *gensec_security, } } + if (gensec_security->channel_bindings != NULL) { + nt_status = ntlmssp_hash_channel_bindings(gensec_security, + server_channel_bindings); + if (!NT_STATUS_IS_OK(nt_status)) { + return nt_status; + } + + ok = mem_equal_const_time(client_channel_bindings, + server_channel_bindings, + 16); + if (!ok && gensec_security->want_features & GENSEC_FEATURE_CB_OPTIONAL) { + /* + * Unlike kerberos, explicit 16 zeros in + * MsvChannelBindings are not enough to + * pass the optional check. + * + * So we only let it through without explicit + * MsvChannelBindings. + */ + ok = (client_channel_bindings == zero_channel_bindings); + } + if (!ok) { + DBG_WARNING("Invalid channel bindings for " + "user=[%s] domain=[%s] workstation=[%s]\n", + ntlmssp_state->user, + ntlmssp_state->domain, + ntlmssp_state->client.netbios_name); + dump_data(DBGLVL_WARNING, + client_channel_bindings, + 16); + dump_data(DBGLVL_WARNING, + server_channel_bindings, + 16); + return NT_STATUS_BAD_BINDINGS; + } + } + nttime_to_timeval(&endtime, ntlmssp_state->server.challenge_endtime); expired = timeval_expired(&endtime); if (expired) { diff --git a/auth/ntlmssp/ntlmssp_util.c b/auth/ntlmssp/ntlmssp_util.c index 6f3b474fd713..b8dc84e1652b 100644 --- a/auth/ntlmssp/ntlmssp_util.c +++ b/auth/ntlmssp/ntlmssp_util.c @@ -22,9 +22,15 @@ */ #include "includes.h" +#include "auth/gensec/gensec.h" +#include "auth/gensec/gensec_internal.h" #include "../auth/ntlmssp/ntlmssp.h" #include "../auth/ntlmssp/ntlmssp_private.h" +#include "lib/crypto/gnutls_helpers.h" +#include +#include + #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -218,3 +224,95 @@ const DATA_BLOB ntlmssp_version_blob(void) return data_blob_const(version_buffer, ARRAY_SIZE(version_buffer)); } + +NTSTATUS ntlmssp_hash_channel_bindings(struct gensec_security *gensec_security, + uint8_t cb_hash[16]) +{ + const struct gensec_channel_bindings *cb = + gensec_security->channel_bindings; + gnutls_hash_hd_t hash_hnd = NULL; + uint8_t uint32buf[4]; + int rc; + + if (cb == NULL) { + memset(cb_hash, 0, 16); + return NT_STATUS_OK; + } + + GNUTLS_FIPS140_SET_LAX_MODE(); + rc = gnutls_hash_init(&hash_hnd, GNUTLS_DIG_MD5); + if (rc < 0) { + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + + SIVAL(uint32buf, 0, cb->initiator_addrtype); + rc = gnutls_hash(hash_hnd, uint32buf, sizeof(uint32buf)); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + SIVAL(uint32buf, 0, cb->initiator_address.length); + rc = gnutls_hash(hash_hnd, uint32buf, sizeof(uint32buf)); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + if (cb->initiator_address.length > 0) { + rc = gnutls_hash(hash_hnd, + cb->initiator_address.data, + cb->initiator_address.length); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + } + SIVAL(uint32buf, 0, cb->acceptor_addrtype); + rc = gnutls_hash(hash_hnd, uint32buf, sizeof(uint32buf)); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + SIVAL(uint32buf, 0, cb->acceptor_address.length); + rc = gnutls_hash(hash_hnd, uint32buf, sizeof(uint32buf)); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + if (cb->acceptor_address.length > 0) { + rc = gnutls_hash(hash_hnd, + cb->acceptor_address.data, + cb->acceptor_address.length); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + } + SIVAL(uint32buf, 0, cb->application_data.length); + rc = gnutls_hash(hash_hnd, uint32buf, sizeof(uint32buf)); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + if (cb->application_data.length > 0) { + rc = gnutls_hash(hash_hnd, + cb->application_data.data, + cb->application_data.length); + if (rc < 0) { + gnutls_hash_deinit(hash_hnd, NULL); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return gnutls_error_to_ntstatus(rc, NT_STATUS_HMAC_NOT_SUPPORTED); + } + } + + gnutls_hash_deinit(hash_hnd, cb_hash); + GNUTLS_FIPS140_SET_STRICT_MODE(); + return NT_STATUS_OK; +} -- 2.34.1 From 5ec242dcceba94125f8f7ee8b5744144542e1293 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 28 Sep 2023 17:09:37 +0200 Subject: [PATCH 23/29] s4:gensec_gssapi: implement channel binding support BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 1831006b77749dda902ae4ced0a96e5f14d89adb) --- source4/auth/gensec/gensec_gssapi.c | 77 ++++++++++++++++++++++++++++- source4/auth/gensec/gensec_gssapi.h | 1 + 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/source4/auth/gensec/gensec_gssapi.c b/source4/auth/gensec/gensec_gssapi.c index cc0491f0532b..e96ee4d79835 100644 --- a/source4/auth/gensec/gensec_gssapi.c +++ b/source4/auth/gensec/gensec_gssapi.c @@ -153,8 +153,31 @@ static NTSTATUS gensec_gssapi_start(struct gensec_security *gensec_security) gensec_gssapi_state->gssapi_context = GSS_C_NO_CONTEXT; - /* TODO: Fill in channel bindings */ - gensec_gssapi_state->input_chan_bindings = GSS_C_NO_CHANNEL_BINDINGS; + if (gensec_security->channel_bindings != NULL) { + gensec_gssapi_state->_input_chan_bindings.initiator_addrtype = + gensec_security->channel_bindings->initiator_addrtype; + gensec_gssapi_state->_input_chan_bindings.initiator_address.value = + gensec_security->channel_bindings->initiator_address.data; + gensec_gssapi_state->_input_chan_bindings.initiator_address.length = + gensec_security->channel_bindings->initiator_address.length; + + gensec_gssapi_state->_input_chan_bindings.acceptor_addrtype = + gensec_security->channel_bindings->acceptor_addrtype; + gensec_gssapi_state->_input_chan_bindings.acceptor_address.value = + gensec_security->channel_bindings->acceptor_address.data; + gensec_gssapi_state->_input_chan_bindings.acceptor_address.length = + gensec_security->channel_bindings->acceptor_address.length; + + gensec_gssapi_state->_input_chan_bindings.application_data.value = + gensec_security->channel_bindings->application_data.data; + gensec_gssapi_state->_input_chan_bindings.application_data.length = + gensec_security->channel_bindings->application_data.length; + + gensec_gssapi_state->input_chan_bindings = + &gensec_gssapi_state->_input_chan_bindings; + } else { + gensec_gssapi_state->input_chan_bindings = GSS_C_NO_CHANNEL_BINDINGS; + } gensec_gssapi_state->server_name = GSS_C_NO_NAME; gensec_gssapi_state->client_name = GSS_C_NO_NAME; @@ -402,6 +425,20 @@ do_start: gensec_gssapi_state = talloc_get_type(gensec_security->private_data, struct gensec_gssapi_state); +#ifdef HAVE_CLIENT_GSS_C_CHANNEL_BOUND_FLAG + /* + * We can only use GSS_C_CHANNEL_BOUND_FLAG if the kerberos library + * supports that in order to add KERB_AP_OPTIONS_CBT. + * + * See: + * https://github.com/heimdal/heimdal/pull/1234 + * https://github.com/krb5/krb5/pull/1329 + */ + if (!(gensec_security->want_features & GENSEC_FEATURE_CB_OPTIONAL)) { + gensec_gssapi_state->gss_want_flags |= GSS_C_CHANNEL_BOUND_FLAG; + } +#endif + if (cli_credentials_get_impersonate_principal(creds)) { gensec_gssapi_state->gss_want_flags &= ~(GSS_C_DELEG_FLAG|GSS_C_DELEG_POLICY_FLAG); } @@ -653,6 +690,39 @@ init_sec_context_done: if (gss_oid_p) { gensec_gssapi_state->gss_oid = gss_oid_p; } +#ifdef GSS_C_CHANNEL_BOUND_FLAG + if (maj_stat == GSS_S_COMPLETE && + gensec_security->channel_bindings != NULL && + !(gensec_security->want_features & GENSEC_FEATURE_CB_OPTIONAL) && + !(gensec_gssapi_state->gss_got_flags & GSS_C_CHANNEL_BOUND_FLAG)) + { + /* + * If we require valid channel bindings + * we need to check the client provided + * them. + * + * We detect this if + * GSS_C_CHANNEL_BOUND_FLAG is given. + * + * Recent heimdal and MIT releases support this + * with older releases (e.g. MIT > 1.19). + * + * It means client with zero channel bindings + * on a server with non-zero channel bindings + * won't generate GSS_S_BAD_BINDINGS directly + * unless KERB_AP_OPTIONS_CBT was also + * provides by the client. + * + * So we need to convert a missing + * GSS_C_CHANNEL_BOUND_FLAG into + * GSS_S_BAD_BINDINGS by default + * (unless GENSEC_FEATURE_CB_OPTIONAL is given). + */ + maj_stat = GSS_S_BAD_BINDINGS; + min_stat = 0; + } +#endif /* GSS_C_CHANNEL_BOUND_FLAG */ + break; } default: @@ -699,6 +769,9 @@ init_sec_context_done: gss_release_buffer(&min_stat2, &output_token); return NT_STATUS_MORE_PROCESSING_REQUIRED; + } else if (maj_stat == GSS_S_BAD_BINDINGS) { + DBG_WARNING("Got GSS_S_BAD_BINDINGS\n"); + return NT_STATUS_BAD_BINDINGS; } else if (maj_stat == GSS_S_CONTEXT_EXPIRED) { gss_cred_id_t creds = NULL; gss_name_t name; diff --git a/source4/auth/gensec/gensec_gssapi.h b/source4/auth/gensec/gensec_gssapi.h index d788b5ebc38d..b729ed109587 100644 --- a/source4/auth/gensec/gensec_gssapi.h +++ b/source4/auth/gensec/gensec_gssapi.h @@ -48,6 +48,7 @@ struct gensec_gssapi_state { /* gensec_gssapi only */ gss_OID gss_oid; + struct gss_channel_bindings_struct _input_chan_bindings; struct gss_channel_bindings_struct *input_chan_bindings; struct smb_krb5_context *smb_krb5_context; struct gssapi_creds_container *client_cred; -- 2.34.1 From 41ab26a8b8d489dc1932fbf92eb523967138ffa9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 29 Sep 2023 11:55:45 +0200 Subject: [PATCH 24/29] s3:crypto/gse: implement channel binding support BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 811d04fea7d329a7f3c8e01ac20bfad48ac9cd4f) --- source3/librpc/crypto/gse.c | 95 ++++++++++++++++++++++++++++++++++--- 1 file changed, 88 insertions(+), 7 deletions(-) diff --git a/source3/librpc/crypto/gse.c b/source3/librpc/crypto/gse.c index b6a2f7842922..623d5d42831f 100644 --- a/source3/librpc/crypto/gse.c +++ b/source3/librpc/crypto/gse.c @@ -64,6 +64,9 @@ struct gse_context { gss_cred_id_t creds; gss_OID ret_mech; + + struct gss_channel_bindings_struct _channel_bindings; + struct gss_channel_bindings_struct *channel_bindings; }; /* free non talloc dependent contexts */ @@ -171,7 +174,7 @@ static NTSTATUS gse_setup_server_principal(TALLOC_CTX *mem_ctx, return NT_STATUS_OK; } -static NTSTATUS gse_context_init(TALLOC_CTX *mem_ctx, +static NTSTATUS gse_context_init(struct gensec_security *gensec_security, bool do_sign, bool do_seal, const char *ccache_name, uint32_t add_gss_c_flags, @@ -181,7 +184,7 @@ static NTSTATUS gse_context_init(TALLOC_CTX *mem_ctx, krb5_error_code k5ret; NTSTATUS status; - gse_ctx = talloc_zero(mem_ctx, struct gse_context); + gse_ctx = talloc_zero(gensec_security, struct gse_context); if (!gse_ctx) { return NT_STATUS_NO_MEMORY; } @@ -206,6 +209,32 @@ static NTSTATUS gse_context_init(TALLOC_CTX *mem_ctx, gse_ctx->gss_want_flags |= add_gss_c_flags; + if (gensec_security->channel_bindings != NULL) { + gse_ctx->_channel_bindings.initiator_addrtype = + gensec_security->channel_bindings->initiator_addrtype; + gse_ctx->_channel_bindings.initiator_address.value = + gensec_security->channel_bindings->initiator_address.data; + gse_ctx->_channel_bindings.initiator_address.length = + gensec_security->channel_bindings->initiator_address.length; + + gse_ctx->_channel_bindings.acceptor_addrtype = + gensec_security->channel_bindings->acceptor_addrtype; + gse_ctx->_channel_bindings.acceptor_address.value = + gensec_security->channel_bindings->acceptor_address.data; + gse_ctx->_channel_bindings.acceptor_address.length = + gensec_security->channel_bindings->acceptor_address.length; + + gse_ctx->_channel_bindings.application_data.value = + gensec_security->channel_bindings->application_data.data; + gse_ctx->_channel_bindings.application_data.length = + gensec_security->channel_bindings->application_data.length; + + gse_ctx->channel_bindings = + &gse_ctx->_channel_bindings; + } else { + gse_ctx->channel_bindings = GSS_C_NO_CHANNEL_BINDINGS; + } + /* Initialize Kerberos Context */ k5ret = smb_krb5_init_context_common(&gse_ctx->k5ctx); if (k5ret) { @@ -464,7 +493,7 @@ static NTSTATUS gse_get_client_auth_token(TALLOC_CTX *mem_ctx, &gse_ctx->gss_mech, gse_ctx->gss_want_flags, time_req, - GSS_C_NO_CHANNEL_BINDINGS, + gse_ctx->channel_bindings, &in_data, NULL, &out_data, @@ -520,7 +549,8 @@ static NTSTATUS gse_get_client_auth_token(TALLOC_CTX *mem_ctx, gse_ctx->server_name, &gse_ctx->gss_mech, gse_ctx->gss_want_flags, - time_req, GSS_C_NO_CHANNEL_BINDINGS, + time_req, + gse_ctx->channel_bindings, &in_data, NULL, &out_data, &gse_ctx->gss_got_flags, &time_rec); goto init_sec_context_done; @@ -620,7 +650,7 @@ done: return status; } -static NTSTATUS gse_init_server(TALLOC_CTX *mem_ctx, +static NTSTATUS gse_init_server(struct gensec_security *gensec_security, bool do_sign, bool do_seal, uint32_t add_gss_c_flags, struct gse_context **_gse_ctx) @@ -636,7 +666,7 @@ static NTSTATUS gse_init_server(TALLOC_CTX *mem_ctx, */ bool skip_transited_check = (server_role != ROLE_STANDALONE); - status = gse_context_init(mem_ctx, do_sign, do_seal, + status = gse_context_init(gensec_security, do_sign, do_seal, NULL, add_gss_c_flags, &gse_ctx); if (!NT_STATUS_IS_OK(status)) { return NT_STATUS_NO_MEMORY; @@ -706,13 +736,46 @@ static NTSTATUS gse_get_server_auth_token(TALLOC_CTX *mem_ctx, &gse_ctx->gssapi_context, gse_ctx->creds, &in_data, - GSS_C_NO_CHANNEL_BINDINGS, + gse_ctx->channel_bindings, &gse_ctx->client_name, &gse_ctx->ret_mech, &out_data, &gse_ctx->gss_got_flags, &time_rec, &gse_ctx->delegated_cred_handle); +#ifdef GSS_C_CHANNEL_BOUND_FLAG + if (gss_maj == GSS_S_COMPLETE && + gensec_security->channel_bindings != NULL && + !(gensec_security->want_features & GENSEC_FEATURE_CB_OPTIONAL) && + !(gse_ctx->gss_got_flags & GSS_C_CHANNEL_BOUND_FLAG)) + { + /* + * If we require valid channel bindings + * we need to check the client provided + * them. + * + * We detect this if + * GSS_C_CHANNEL_BOUND_FLAG is given. + * + * Recent heimdal and MIT releases support this + * with older releases (e.g. MIT > 1.19). + * + * It means client with zero channel bindings + * on a server with non-zero channel bindings + * won't generate GSS_S_BAD_BINDINGS directly + * unless KERB_AP_OPTIONS_CBT was also + * provides by the client. + * + * So we need to convert a missing + * GSS_C_CHANNEL_BOUND_FLAG into + * GSS_S_BAD_BINDINGS by default + * (unless GENSEC_FEATURE_CB_OPTIONAL is given). + */ + gss_maj = GSS_S_BAD_BINDINGS; + gss_min = 0; + } +#endif /* GSS_C_CHANNEL_BOUND_FLAG */ + switch (gss_maj) { case GSS_S_COMPLETE: /* we are done with it */ @@ -725,6 +788,10 @@ static NTSTATUS gse_get_server_auth_token(TALLOC_CTX *mem_ctx, /* we will need a third leg */ status = NT_STATUS_MORE_PROCESSING_REQUIRED; break; + case GSS_S_BAD_BINDINGS: + DBG_WARNING("Got GSS_S_BAD_BINDINGS\n"); + status = NT_STATUS_BAD_BINDINGS; + goto done; default: DEBUG(1, ("gss_accept_sec_context failed with [%s]\n", gse_errstr(talloc_tos(), gss_maj, gss_min))); @@ -853,6 +920,20 @@ static NTSTATUS gensec_gse_client_start(struct gensec_security *gensec_security) want_flags |= GSS_C_DCE_STYLE; } +#ifdef HAVE_CLIENT_GSS_C_CHANNEL_BOUND_FLAG + /* + * We can only use GSS_C_CHANNEL_BOUND_FLAG if the kerberos library + * supports that in order to add KERB_AP_OPTIONS_CBT. + * + * See: + * https://github.com/heimdal/heimdal/pull/1234 + * https://github.com/krb5/krb5/pull/1329 + */ + if (!(gensec_security->want_features & GENSEC_FEATURE_CB_OPTIONAL)) { + want_flags |= GSS_C_CHANNEL_BOUND_FLAG; + } +#endif + nt_status = gse_init_client(gensec_security, do_sign, do_seal, NULL, hostname, service, realm, username, password, want_flags, -- 2.34.1 From b6b8891fb738466dfbe41cf972be925c0d022854 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 23 Jan 2024 14:20:24 +0100 Subject: [PATCH 25/29] s4:ldap_server: add support for tls channel bindings ldap server require strong auth = allow_sasl_over_tls is now an alias for 'allow_sasl_without_tls_channel_bindings' and should be avoided and changed to 'yes' or 'allow_sasl_without_tls_channel_bindings'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 6c17e3d2800723bafebd1986ab59a9422c881f0b) --- .../ldap/ldapserverrequirestrongauth.xml | 38 +++++++++--- lib/param/loadparm.h | 1 + lib/param/param_table.c | 2 + python/samba/netcmd/testparm.py | 10 +++ selftest/target/Samba4.pm | 2 +- source3/utils/testparm.c | 12 ++++ source4/ldap_server/ldap_bind.c | 62 ++++++++++++++++--- source4/ldap_server/ldap_server.c | 11 ++++ 8 files changed, 122 insertions(+), 16 deletions(-) diff --git a/docs-xml/smbdotconf/ldap/ldapserverrequirestrongauth.xml b/docs-xml/smbdotconf/ldap/ldapserverrequirestrongauth.xml index 02bdd8114919..18f8903dcaab 100644 --- a/docs-xml/smbdotconf/ldap/ldapserverrequirestrongauth.xml +++ b/docs-xml/smbdotconf/ldap/ldapserverrequirestrongauth.xml @@ -7,20 +7,44 @@ The defines whether the ldap server requires ldap traffic to be signed or signed and encrypted (sealed). - Possible values are no, allow_sasl_over_tls + Possible values are no, + allow_sasl_without_tls_channel_bindings and yes. + Windows has LdapEnforceChannelBinding under + HKLM\SYSTEM\CurrentControlSet\Services\NTDS\Parameters\. + + A value of no allows simple and sasl binds over - all transports. + all transports. This matches LdapEnforceChannelBinding=0. + + A value of allow_sasl_without_tls_channel_bindings + allows simple and sasl binds (without sign or seal) over TLS encrypted connections. + Missing tls channel bindings are ignored, so only use this if a value of + yes is not possible. + Unencrypted connections only allow sasl binds with sign or seal. + This matches LdapEnforceChannelBinding=1. + - A value of allow_sasl_over_tls allows simple and sasl binds - (without sign or seal) over TLS encrypted connections. Unencrypted connections only - allow sasl binds with sign or seal. + Before support for tls channel bindings existed in Samba, + a value of allow_sasl_over_tls was possible in order + to allow sasl binds without tls channel bindings. This now misleading + as a value of yes will now allow sasl binds + with tls channel bindings. Configurations should be changed to + yes instead or + allow_sasl_without_tls_channel_bindings + if really required. Currently allow_sasl_over_tls + is just an alias of allow_sasl_without_tls_channel_bindings, + but it will be removed in future versions. + A value of yes allows only simple binds - over TLS encrypted connections. Unencrypted connections only - allow sasl binds with sign or seal. + and sasl binds with correct tls channel bindings + over TLS encrypted connections. sasl binds without tls channel bindings + are not allowed. Unencrypted connections only + allow sasl binds with sign or seal. This matches LdapEnforceChannelBinding=2. + yes diff --git a/lib/param/loadparm.h b/lib/param/loadparm.h index 0bf4c173652c..7e9e5d2da3f8 100644 --- a/lib/param/loadparm.h +++ b/lib/param/loadparm.h @@ -206,6 +206,7 @@ enum printing_types {PRINT_BSD,PRINT_SYSV,PRINT_AIX,PRINT_HPUX, enum ldap_server_require_strong_auth { LDAP_SERVER_REQUIRE_STRONG_AUTH_NO, LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_OVER_TLS, + LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_WITHOUT_TLS_CB, LDAP_SERVER_REQUIRE_STRONG_AUTH_YES, }; diff --git a/lib/param/param_table.c b/lib/param/param_table.c index ce591560ba83..8db4c381e412 100644 --- a/lib/param/param_table.c +++ b/lib/param/param_table.c @@ -318,6 +318,8 @@ static const struct enum_list enum_ldap_server_require_strong_auth_vals[] = { { LDAP_SERVER_REQUIRE_STRONG_AUTH_NO, "0" }, { LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_OVER_TLS, "allow_sasl_over_tls" }, + { LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_WITHOUT_TLS_CB, + "allow_sasl_without_tls_channel_bindings" }, { LDAP_SERVER_REQUIRE_STRONG_AUTH_YES, "Yes" }, { LDAP_SERVER_REQUIRE_STRONG_AUTH_YES, "True" }, { LDAP_SERVER_REQUIRE_STRONG_AUTH_YES, "1" }, diff --git a/python/samba/netcmd/testparm.py b/python/samba/netcmd/testparm.py index 41dbb4bd6236..a419ddf12606 100644 --- a/python/samba/netcmd/testparm.py +++ b/python/samba/netcmd/testparm.py @@ -183,6 +183,16 @@ class cmd_testparm(Command): "When acting as Active Directory domain controller, " + entry + " should be in vfs objects.") + strong_auth = lp.get("ldap server require strong auth") + if strong_auth == "allow_sasl_over_tls": + logger.warning( + "WARNING: You have not configured " + "'ldap server require strong auth = " + "allow_sasl_over_tls'.\n" + "Please change to 'yes' (preferred) or " + "'allow_sasl_without_tls_channel_bindings' " + "(if really needed).") + return valid def allow_access(self, deny_list, allow_list, cname, caddr): diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index f2b84b4f9b7b..c312e818daf2 100755 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -1625,7 +1625,7 @@ sub provision_ad_dc_ntvfs($$$) print "PROVISIONING AD DC (NTVFS)...\n"; my $extra_conf_options = "netbios aliases = localDC1-a server services = +winbind -winbindd - ldap server require strong auth = allow_sasl_over_tls + ldap server require strong auth = allow_sasl_without_tls_channel_bindings raw NTLMv2 auth = yes lsa over netlogon = yes rpc server port = 1027 diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c index fd90e8d734a8..34bce413f82f 100644 --- a/source3/utils/testparm.c +++ b/source3/utils/testparm.c @@ -615,6 +615,18 @@ static int do_global_checks(void) ret = 1; } + if (lp_ldap_server_require_strong_auth() == + LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_OVER_TLS) + { + fprintf(stderr, + "WARNING: You have not configured " + "'ldap server require strong auth = " + "allow_sasl_over_tls'.\n" + "Please change to 'yes' (preferred) or " + "'allow_sasl_without_tls_channel_bindings' " + "(if really needed)\n\n"); + } + if (lp_server_schannel() != true) { /* can be 'auto' */ fprintf(stderr, "WARNING: You have not configured " diff --git a/source4/ldap_server/ldap_bind.c b/source4/ldap_server/ldap_bind.c index d592d472c06d..65e252edb702 100644 --- a/source4/ldap_server/ldap_bind.c +++ b/source4/ldap_server/ldap_bind.c @@ -27,6 +27,7 @@ #include "dsdb/samdb/samdb.h" #include "auth/gensec/gensec.h" #include "auth/gensec/gensec_tstream.h" +#include "lib/tls/tls.h" #include "param/param.h" #include "../lib/util/tevent_ntstatus.h" #include "lib/util/time_basic.h" @@ -359,6 +360,49 @@ static NTSTATUS ldapsrv_setup_gensec(struct ldapsrv_connection *conn, gensec_want_feature(gensec_security, GENSEC_FEATURE_LDAP_STYLE); if (conn->sockets.active == conn->sockets.tls) { + uint32_t initiator_addrtype = 0; + const DATA_BLOB *initiator_address = NULL; + uint32_t acceptor_addrtype = 0; + const DATA_BLOB *acceptor_address = NULL; + const DATA_BLOB *application_data = + tstream_tls_channel_bindings(conn->sockets.tls); + + status = gensec_set_channel_bindings(gensec_security, + initiator_addrtype, + initiator_address, + acceptor_addrtype, + acceptor_address, + application_data); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + /* + * By default channel bindings are required, + * so we only set GENSEC_FEATURE_CB_OPTIONAL + * for the legacy option: + * + * ldap server require strong auth = no + * or + * ldap server require strong auth = + * allow_sasl_without_tls_channel_bindings + * + * And this as an alias to cope with existing smb.conf + * files: + * + * ldap server require strong auth = allow_sasl_over_tls + */ + switch (conn->require_strong_auth) { + case LDAP_SERVER_REQUIRE_STRONG_AUTH_NO: + case LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_OVER_TLS: + case LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_WITHOUT_TLS_CB: + gensec_want_feature(gensec_security, + GENSEC_FEATURE_CB_OPTIONAL); + break; + default: + break; + } + gensec_want_feature(gensec_security, GENSEC_FEATURE_LDAPS_TRANSPORT); } @@ -496,6 +540,14 @@ static void ldapsrv_BindSASL_done(struct tevent_req *subreq) goto do_reply; } + if (NT_STATUS_EQUAL(status, NT_STATUS_BAD_BINDINGS)) { + result = LDAP_INVALID_CREDENTIALS; + errstr = ldapsrv_bind_error_msg(reply, + HRES_SEC_E_BAD_BINDINGS, + 0x0C090711, + status); + goto do_reply; + } if (!NT_STATUS_IS_OK(status)) { status = nt_status_squash(status); result = LDAP_INVALID_CREDENTIALS; @@ -539,17 +591,11 @@ static void ldapsrv_BindSASL_done(struct tevent_req *subreq) case LDAP_SERVER_REQUIRE_STRONG_AUTH_NO: break; case LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_OVER_TLS: + case LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_WITHOUT_TLS_CB: + case LDAP_SERVER_REQUIRE_STRONG_AUTH_YES: if (call->conn->sockets.active == call->conn->sockets.tls) { break; } - status = NT_STATUS_NETWORK_ACCESS_DENIED; - result = LDAP_STRONG_AUTH_REQUIRED; - errstr = talloc_asprintf(reply, - "SASL:[%s]: not allowed if TLS is used.", - req->creds.SASL.mechanism); - goto do_reply; - - case LDAP_SERVER_REQUIRE_STRONG_AUTH_YES: status = NT_STATUS_NETWORK_ACCESS_DENIED; result = LDAP_STRONG_AUTH_REQUIRED; errstr = talloc_asprintf(reply, diff --git a/source4/ldap_server/ldap_server.c b/source4/ldap_server/ldap_server.c index 9c34c3e57122..fe75093d77c7 100644 --- a/source4/ldap_server/ldap_server.c +++ b/source4/ldap_server/ldap_server.c @@ -376,6 +376,17 @@ static void ldapsrv_accept(struct stream_connection *c, conn->require_strong_auth = lpcfg_ldap_server_require_strong_auth(conn->lp_ctx); } + if (conn->require_strong_auth == + LDAP_SERVER_REQUIRE_STRONG_AUTH_ALLOW_SASL_OVER_TLS) + { + D_ERR("WARNING: You have not configured " + "'ldap server require strong auth = " + "allow_sasl_over_tls'.\n" + "Please change to 'yes' (preferred and default) or " + "'allow_sasl_without_tls_channel_bindings' " + "(if really needed)\n\n"); + } + ret = ldapsrv_backend_Init(conn, &errstring); if (ret != LDB_SUCCESS) { char *reason = talloc_asprintf(conn, -- 2.34.1 From 90e88309b2a0358ebecfc7ca566485c9e16aa9b1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 28 Sep 2023 17:11:03 +0200 Subject: [PATCH 26/29] s4:libcli/ldap: add tls channel binding support for ldap_bind_sasl() We still allow 'ldap_testing:tls_channel_bindings = no' and 'ldap_testing:channel_bound = no' for testing the old behavior in order to have expected failures in our tests. And we have 'ldap_testing:forced_channel_binding = somestring' in order to force invalid bindings. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 7acb15a53c061344ffdbd58f9b2f01f8b0233f4e) --- source4/libcli/ldap/ldap_bind.c | 53 +++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c index e1a37a391c5b..b00329c9fa43 100644 --- a/source4/libcli/ldap/ldap_bind.c +++ b/source4/libcli/ldap/ldap_bind.c @@ -217,6 +217,17 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, uint32_t old_gensec_features; unsigned int logon_retries = 0; size_t queue_length; + const DATA_BLOB *tls_cb = NULL; + bool use_channel_bound = lpcfg_parm_bool(lp_ctx, + NULL, + "ldap_testing", + "channel_bound", + true); + const char *forced_channel_binding = lpcfg_parm_string(lp_ctx, + NULL, + "ldap_testing", + "forced_channel_binding"); + DATA_BLOB forced_cb = data_blob_string_const(forced_channel_binding); if (conn->sockets.active == NULL) { status = NT_STATUS_CONNECTION_DISCONNECTED; @@ -247,11 +258,25 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, gensec_init(); if (conn->sockets.active == conn->sockets.tls) { + /* + * allow this for testing the old code: + * ldap_testing:no_tls_channel_bindings = no + */ + bool use_tls_cb = lpcfg_parm_bool(lp_ctx, + NULL, + "ldap_testing", + "tls_channel_bindings", + true); + /* * require Kerberos SIGN/SEAL only if we don't use SSL * Windows seem not to like double encryption */ wrap_flags = 0; + + if (use_tls_cb) { + tls_cb = tstream_tls_channel_bindings(conn->sockets.tls); + } } else if (cli_credentials_is_anonymous(creds)) { /* * anonymous isn't protected @@ -261,6 +286,10 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn, wrap_flags = lpcfg_client_ldap_sasl_wrapping(lp_ctx); } + if (forced_cb.length != 0) { + tls_cb = &forced_cb; + } + try_logon_again: /* we loop back here on a logon failure, and re-create the @@ -306,6 +335,10 @@ try_logon_again: gensec_want_feature(conn->gensec, GENSEC_FEATURE_SIGN); } + if (!use_channel_bound) { + gensec_want_feature(conn->gensec, GENSEC_FEATURE_CB_OPTIONAL); + } + /* * This is an indication for the NTLMSSP backend to * also encrypt when only GENSEC_FEATURE_SIGN is requested @@ -329,6 +362,26 @@ try_logon_again: goto failed; } + if (tls_cb != NULL) { + uint32_t initiator_addrtype = 0; + const DATA_BLOB *initiator_address = NULL; + uint32_t acceptor_addrtype = 0; + const DATA_BLOB *acceptor_address = NULL; + const DATA_BLOB *application_data = tls_cb; + + status = gensec_set_channel_bindings(conn->gensec, + initiator_addrtype, + initiator_address, + acceptor_addrtype, + acceptor_address, + application_data); + if (!NT_STATUS_IS_OK(status)) { + DBG_WARNING("Failed to set GENSEC channel bindings: %s\n", + nt_errstr(status)); + goto failed; + } + } + status = gensec_start_mech_by_sasl_name(conn->gensec, sasl_mech); if (!NT_STATUS_IS_OK(status)) { DBG_WARNING("gensec_start_mech_by_sasl_name(%s): %s\n", -- 2.34.1 From 884d5b31307ee1fad24718741711cb7975a67f52 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Feb 2024 15:50:14 +0100 Subject: [PATCH 27/29] selftest: split out selftest/expectedfail.d/samba4.ldb.simple.ldap-tls BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 6794cc476249452c415881396bce4df663fc4fba) --- selftest/expectedfail.d/samba4.ldb.simple.ldap-tls | 6 ++++++ selftest/knownfail | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 selftest/expectedfail.d/samba4.ldb.simple.ldap-tls diff --git a/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls b/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls new file mode 100644 index 000000000000..963076d5d338 --- /dev/null +++ b/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls @@ -0,0 +1,6 @@ +# +## We assert all "ldap server require strong auth" combinations +# +^samba4.ldb.simple.ldap with SIMPLE-BIND.*ad_dc_ntvfs # ldap server require strong auth = allow_sasl_over_tls +^samba4.ldb.simple.ldap with SIMPLE-BIND.*fl2003dc # ldap server require strong auth = yes +^samba4.ldb.simple.ldaps with SASL-BIND.*fl2003dc # ldap server require strong auth = yes diff --git a/selftest/knownfail b/selftest/knownfail index 77f5d5d5be62..e0db191e2f4d 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -312,12 +312,6 @@ # ^samba4.ldap.sort.python.+UnicodeSortTests # -## We assert all "ldap server require strong auth" combinations -# -^samba4.ldb.simple.ldap with SIMPLE-BIND.*ad_dc_ntvfs # ldap server require strong auth = allow_sasl_over_tls -^samba4.ldb.simple.ldap with SIMPLE-BIND.*fl2003dc # ldap server require strong auth = yes -^samba4.ldb.simple.ldaps with SASL-BIND.*fl2003dc # ldap server require strong auth = yes -# # we don't allow auth_level_connect anymore... # ^samba3.blackbox.rpcclient.*ncacn_np.*with.*connect.*rpcclient # we don't allow auth_level_connect anymore -- 2.34.1 From 9694ae9b08db15639711ec03c0f98efc419d0fe6 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 13 Feb 2024 15:50:14 +0100 Subject: [PATCH 28/29] s4:selftest: also test samba4.ldb.simple.ldap*SASL-BIND with ldap_testing:{channel_bound,tls_channel_bindings,forced_channel_binding} BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 065da873296c23ef3b9051fba39be097cfff60fa) --- .../expectedfail.d/samba4.ldb.simple.ldap-tls | 19 ++++++++++-- selftest/expectedfail_heimdal | 12 +++++++ selftest/wscript | 4 +++ source4/selftest/tests.py | 31 +++++++++++++++++-- 4 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 selftest/expectedfail_heimdal diff --git a/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls b/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls index 963076d5d338..24b9b94a4284 100644 --- a/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls +++ b/selftest/expectedfail.d/samba4.ldb.simple.ldap-tls @@ -1,6 +1,21 @@ # ## We assert all "ldap server require strong auth" combinations # -^samba4.ldb.simple.ldap with SIMPLE-BIND.*ad_dc_ntvfs # ldap server require strong auth = allow_sasl_over_tls +^samba4.ldb.simple.ldap with SIMPLE-BIND.*ad_dc_ntvfs # ldap server require strong auth = allow_sasl_without_tls_channel_bindings ^samba4.ldb.simple.ldap with SIMPLE-BIND.*fl2003dc # ldap server require strong auth = yes -^samba4.ldb.simple.ldaps with SASL-BIND.*fl2003dc # ldap server require strong auth = yes +# fl2003dc has ldap server require strong auth = yes +# and correct channel bindings are required for TLS +^samba4.ldb.simple.ldaps.*SASL-BIND.*ldap_testing:tls_channel_bindings=no.*fl2003dc +# ad_dc_ntvfs and fl2008r2dc have +# ldap server require strong auth = allow_sasl_without_tls_channel_bindings +# it means correct channel bindings are required, if the client indicated +# explicit (even null) channel bindings are provided +# +# The following are in expectedfail_heimdal for now, as MIT +# behaves differently: +#^samba4.ldb.simple.ldaps.with.SASL-BIND.*use-kerberos=required.*ldap_testing:channel_bound=yes.*ldap_testing:tls_channel_bindings=no.*ad_dc_ntvfs +#^samba4.ldb.simple.ldaps.with.SASL-BIND.*use-kerberos=required.*ldap_testing:channel_bound=yes.*ldap_testing:tls_channel_bindings=no.*fl2008r2dc +^samba4.ldb.simple.ldaps.with.SASL-BIND.*ldap_testing:channel_bound=yes.*ldap_testing:forced_channel_binding=wRoNg +^samba4.ldb.simple.ldaps.with.SASL-BIND.*ldap_testing:channel_bound=no.*ldap_testing:forced_channel_binding=wRoNg +^samba4.ldb.simple.ldaps.with.SASL-BIND.*use-kerberos=disabled.*ldap_testing:channel_bound=yes.*ldap_testing:tls_channel_bindings=no.*ad_dc_ntvfs +^samba4.ldb.simple.ldaps.with.SASL-BIND.*use-kerberos=disabled.*ldap_testing:channel_bound=yes.*ldap_testing:tls_channel_bindings=no.*fl2008r2dc diff --git a/selftest/expectedfail_heimdal b/selftest/expectedfail_heimdal new file mode 100644 index 000000000000..6415a6ebb229 --- /dev/null +++ b/selftest/expectedfail_heimdal @@ -0,0 +1,12 @@ +# ad_dc_ntvfs and fl2008r2dc have +# ldap server require strong auth = allow_sasl_without_tls_channel_bindings +# it means correct channel bindings are required, if the client indicated +# explicit (even null) channel bindings are provided +# +# Note currently only embedded_heimdal supports +# GSS_C_CHANNEL_BOUND_FLAG as client. +# See also: +# https://github.com/heimdal/heimdal/pull/1234 +# https://github.com/krb5/krb5/pull/1329 +^samba4.ldb.simple.ldaps.with.SASL-BIND.*use-kerberos=required.*ldap_testing:channel_bound=yes.*ldap_testing:tls_channel_bindings=no.*ad_dc_ntvfs +^samba4.ldb.simple.ldaps.with.SASL-BIND.*use-kerberos=required.*ldap_testing:channel_bound=yes.*ldap_testing:tls_channel_bindings=no.*fl2008r2dc diff --git a/selftest/wscript b/selftest/wscript index daf497d5e62d..b8faf6dbc84f 100644 --- a/selftest/wscript +++ b/selftest/wscript @@ -274,6 +274,10 @@ def cmd_testonly(opt): env.FILTER_XFAIL += " --expected-failures=${srcdir}/selftest/"\ "knownfail_heimdal_kdc" + if CONFIG_SET(opt, 'USING_EMBEDDED_HEIMDAL'): + env.FILTER_XFAIL += " --expected-failures=${srcdir}/selftest/"\ + "expectedfail_heimdal" + if CONFIG_GET(opt, 'SIZEOF_VOID_P') == 4: env.FILTER_XFAIL += " --expected-failures=${srcdir}/selftest/knownfail-32bit" env.OPTIONS += " --default-ldb-backend=tdb --exclude=${srcdir}/selftest/skip-32bit" diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index e47eb5766da4..363a1a24fa74 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -163,19 +163,44 @@ for env in ["ad_dc_ntvfs", "fl2008r2dc", "fl2003dc"]: '--use-kerberos=required --option=clientldapsaslwrapping=plain', '--use-kerberos=required --client-protection=sign', '--use-kerberos=required --client-protection=encrypt', + '--use-kerberos=required --client-protection=sign --option="ldap_testing:channel_bound=yes"', + '--use-kerberos=required --client-protection=sign --option="ldap_testing:channel_bound=no"', + '--use-kerberos=required --client-protection=sign --option="ldap_testing:channel_bound=yes" --option="ldap_testing:forced_channel_binding=wRoNg"', + '--use-kerberos=required --client-protection=sign --option="ldap_testing:channel_bound=no" --option="ldap_testing:forced_channel_binding=wRoNg"', '--use-kerberos=disabled --option=clientldapsaslwrapping=plain', '--use-kerberos=disabled --client-protection=sign --option=ntlmssp_client:ldap_style_send_seal=no', '--use-kerberos=disabled --client-protection=sign', '--use-kerberos=disabled --client-protection=encrypt', + '--use-kerberos=disabled --client-protection=sign --option="ldap_testing:channel_bound=yes"', + '--use-kerberos=disabled --client-protection=sign --option="ldap_testing:channel_bound=no"', + '--use-kerberos=disabled --client-protection=sign --option="ldap_testing:channel_bound=yes" --option="ldap_testing:forced_channel_binding=wRoNg"', + '--use-kerberos=disabled --client-protection=sign --option="ldap_testing:channel_bound=no" --option="ldap_testing:forced_channel_binding=wRoNg"', ] for auth_option in auth_options: options = '-U"$USERNAME%$PASSWORD"' + ' ' + auth_option plantestsuite("samba4.ldb.simple.ldap with SASL-BIND %s(%s)" % (options, env), env, "%s/test_ldb_simple.sh ldap $SERVER %s" % (bbdir, options)) - options = '-U"$USERNAME%$PASSWORD" --option="tlsverifypeer=no_check"' - plantestsuite("samba4.ldb.simple.ldaps with SASL-BIND %s(%s)" % (options, env), - env, "%s/test_ldb_simple.sh ldaps $SERVER %s" % (bbdir, options)) + + auth_options = [ + '--use-kerberos=required --option="ldap_testing:channel_bound=yes" --option="ldap_testing:tls_channel_bindings=yes"', + '--use-kerberos=required --option="ldap_testing:channel_bound=yes" --option="ldap_testing:tls_channel_bindings=no"', + '--use-kerberos=required --option="ldap_testing:channel_bound=yes" --option="ldap_testing:forced_channel_binding=wRoNg"', + '--use-kerberos=required --option="ldap_testing:channel_bound=no" --option="ldap_testing:tls_channel_bindings=no"', + '--use-kerberos=required --option="ldap_testing:channel_bound=no" --option="ldap_testing:tls_channel_bindings=yes"', + '--use-kerberos=required --option="ldap_testing:channel_bound=no" --option="ldap_testing:forced_channel_binding=wRoNg"', + '--use-kerberos=disabled --option="ldap_testing:channel_bound=yes" --option="ldap_testing:tls_channel_bindings=yes"', + '--use-kerberos=disabled --option="ldap_testing:channel_bound=yes" --option="ldap_testing:tls_channel_bindings=no"', + '--use-kerberos=disabled --option="ldap_testing:channel_bound=yes" --option="ldap_testing:forced_channel_binding=wRoNg"', + '--use-kerberos=disabled --option="ldap_testing:channel_bound=no" --option="ldap_testing:tls_channel_bindings=no"', + '--use-kerberos=disabled --option="ldap_testing:channel_bound=no" --option="ldap_testing:tls_channel_bindings=yes"', + '--use-kerberos=disabled --option="ldap_testing:channel_bound=no" --option="ldap_testing:forced_channel_binding=wRoNg"', + ] + for auth_option in auth_options: + options = '-U"$USERNAME%$PASSWORD" --option="tlsverifypeer=no_check" ' + auth_option + plantestsuite("samba4.ldb.simple.ldaps with SASL-BIND %s(%s)" % (options, env), + env, "%s/test_ldb_simple.sh ldaps $SERVER %s" % (bbdir, options)) + envraw = "fl2008r2dc" env = "%s:local" % envraw -- 2.34.1 From 6ead9db5bdc42cde46d6844bb5884d3953597dce Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Mar 2024 19:34:22 +0100 Subject: [PATCH 29/29] TODO JULE-PLEASE-ADJUST-FOR-4-20 WHATSNEW: document ldap_server ldaps/tls channel binding support BUG: https://bugzilla.samba.org/show_bug.cgi?id=15621 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit e1c4caed10d775e23cd7dc294f2cccce76866894) --- WHATSNEW.txt | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/WHATSNEW.txt b/WHATSNEW.txt index fb964d7a6f4f..5a14c496b2c7 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -89,6 +89,29 @@ Release notes for older releases follow: This is the latest stable release of the Samba 4.20 release series. +LDAP TLS/SASL channel binding support +------------------------------------- + +The ldap server supports SASL binds with +kerberos or NTLMSSP over TLS connections +now (either ldaps or starttls). + +Setups where 'ldap server require strong auth = allow_sasl_over_tls' +was required before, can now most likely move to the +default of 'ldap server require strong auth = yes'. + +If SASL binds without correct tls channel bindings are required +'ldap server require strong auth = allow_sasl_without_tls_channel_bindings' +should be used now, as 'allow_sasl_over_tls' will generate a +warning in every start of 'samba', as well as '[samba-tool ]testparm'. + +This is similar to LdapEnforceChannelBinding under +HKLM\SYSTEM\CurrentControlSet\Services\NTDS\Parameters +on Windows. + +All client tools using ldaps also include the correct +channel bindings now. + Changes since 4.20.0 -------------------- @@ -404,6 +427,7 @@ smb.conf changes Parameter Name Description Default -------------- ----------- ------- + ldap server require strong auth new values acl claims evaluation new AD DC only smb3 unix extensions Per share - smb3 share cap:ASYMMETRIC new no -- 2.34.1