The Samba-Bugzilla – Attachment 17302 Details for
Bug 15047
[SECURITY] CVE-2022-2031 kadmin/changew gets a krbtgt key as AS-REP
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch proposal #3
kadmin_changepw.patch (text/plain), 16.25 KB, created by
Andreas Schneider
on 2022-05-24 11:35:14 UTC
(
hide
)
Description:
Patch proposal #3
Filename:
MIME Type:
Creator:
Andreas Schneider
Created:
2022-05-24 11:35:14 UTC
Size:
16.25 KB
patch
obsolete
>From bb643664a28e4f371629e358bd99c2d6f6444446 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Tue, 24 May 2022 10:17:00 +0200 >Subject: [PATCH 1/6] testprogs: Fix auth with smbclient and krb5 ccache > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 > >Signed-off-by: Andreas Schneider <asn@samba.org> >--- > testprogs/blackbox/test_kpasswd_heimdal.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/testprogs/blackbox/test_kpasswd_heimdal.sh b/testprogs/blackbox/test_kpasswd_heimdal.sh >index 43f38b09de2..a73c6665a18 100755 >--- a/testprogs/blackbox/test_kpasswd_heimdal.sh >+++ b/testprogs/blackbox/test_kpasswd_heimdal.sh >@@ -71,7 +71,7 @@ testit "kinit with user password" \ > do_kinit $TEST_PRINCIPAL $TEST_PASSWORD || failed=`expr $failed + 1` > > test_smbclient "Test login with user kerberos ccache" \ >- "ls" "$SMB_UNC" --use-kerberos=required || failed=`expr $failed + 1` >+ "ls" "$SMB_UNC" --use-krb5-ccache=${KRB5CCNAME} || failed=`expr $failed + 1` > > testit "change user password with 'samba-tool user password' (unforced)" \ > $VALGRIND $PYTHON $samba_tool user password -W$DOMAIN -U$TEST_USERNAME%$TEST_PASSWORD --use-kerberos=off --newpassword=$TEST_PASSWORD_NEW || failed=`expr $failed + 1` >@@ -84,7 +84,7 @@ testit "kinit with user password" \ > do_kinit $TEST_PRINCIPAL $TEST_PASSWORD || failed=`expr $failed + 1` > > test_smbclient "Test login with user kerberos ccache" \ >- "ls" "$SMB_UNC" --use-kerberos=required || failed=`expr $failed + 1` >+ "ls" "$SMB_UNC" --use-krb5-ccache=${KRB5CCNAME} || failed=`expr $failed + 1` > > ########################################################### > ### check that a short password is rejected >-- >2.36.1 > > >From 24919c0c10551c6f151b8cf59882c886239313ca Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 19 May 2022 16:35:28 +0200 >Subject: [PATCH 2/6] testprogs: Add test with MIT kpasswd for CVE-2022-XXXXX > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 > >Signed-off-by: Andreas Schneider <asn@samba.org> >--- > selftest/knownfail.d/kadmin_changew | 1 + > testprogs/blackbox/test_kpasswd_heimdal.sh | 35 +++++++++++++++++++++- > 2 files changed, 35 insertions(+), 1 deletion(-) > create mode 100644 selftest/knownfail.d/kadmin_changew > >diff --git a/selftest/knownfail.d/kadmin_changew b/selftest/knownfail.d/kadmin_changew >new file mode 100644 >index 00000000000..97c14793ea5 >--- /dev/null >+++ b/selftest/knownfail.d/kadmin_changew >@@ -0,0 +1 @@ >+^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password >diff --git a/testprogs/blackbox/test_kpasswd_heimdal.sh b/testprogs/blackbox/test_kpasswd_heimdal.sh >index a73c6665a18..698044a3fd3 100755 >--- a/testprogs/blackbox/test_kpasswd_heimdal.sh >+++ b/testprogs/blackbox/test_kpasswd_heimdal.sh >@@ -7,7 +7,7 @@ > > if [ $# -lt 6 ]; then > cat <<EOF >-Usage: test_passwords.sh SERVER USERNAME PASSWORD REALM DOMAIN PREFIX SMBCLIENT >+Usage: test_kpasswd_heimdal.sh SERVER USERNAME PASSWORD REALM DOMAIN PREFIX SMBCLIENT > EOF > exit 1; > fi >@@ -27,6 +27,8 @@ smbclient="$samba_bindir/smbclient" > samba_kinit=$samba_bindir/samba4kinit > samba_kpasswd=$samba_bindir/samba4kpasswd > >+mit_kpasswd="$(command -v kpasswd)" >+ > samba_tool="$samba_bindir/samba-tool" > net_tool="$samba_bindir/net" > texpect="$samba_bindir/texpect" >@@ -141,6 +143,37 @@ testit "kpasswd change user password" \ > TEST_PASSWORD=$TEST_PASSWORD_NEW > TEST_PASSWORD_NEW="testPaSS@03%" > >+########################################################### >+### CVE-2022-XXXXX >+########################################################### >+ >+if [ -n "${mit_kpasswd}" ]; then >+ cat > "${PREFIX}/tmpkpasswdscript" <<EOF >+expect Password for ${TEST_PRINCIPAL} >+password ${TEST_PASSWORD}\n >+expect Enter new password >+send ${TEST_PASSWORD_NEW}\n >+expect Enter it again >+send ${TEST_PASSWORD_NEW}\n >+expect Password changed. >+EOF >+ >+ SAVE_KRB5_CONFIG="${KRB5_CONFIG}" >+ KRB5_CONFIG="${PREFIX}/tmpkrb5.conf" >+ export KRB5_CONFIG >+ sed -e 's/\[libdefaults\]/[libdefaults]\n canonicalize = yes/' \ >+ "${SAVE_KRB5_CONFIG}" > "${KRB5_CONFIG}" >+ testit "MIT kpasswd change user password" \ >+ "${texpect}" "${PREFIX}/tmpkpasswdscript" "${mit_kpasswd}" \ >+ "${TEST_PRINCIPAL}" || >+ failed=$((failed + 1)) >+ KRB5_CONFIG="${SAVE_KRB5_CONFIG}" >+ export KRB5_CONFIG >+fi >+ >+TEST_PASSWORD="${TEST_PASSWORD_NEW}" >+TEST_PASSWORD_NEW="testPaSS@03force%" >+ > ########################################################### > ### Force password change at login > ########################################################### >-- >2.36.1 > > >From 96c4e4bcc3a1f249642b89637241d116a6a7958a Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Tue, 24 May 2022 09:54:18 +0200 >Subject: [PATCH 3/6] s4:kdc: Implement is_kadmin_changepw() helper function > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 > >Signed-off-by: Andreas Schneider <asn@samba.org> >--- > source4/kdc/db-glue.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > >diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c >index ea329b7edab..b03a32b67f5 100644 >--- a/source4/kdc/db-glue.c >+++ b/source4/kdc/db-glue.c >@@ -917,6 +917,14 @@ static int principal_comp_strcmp(krb5_context context, > component, string, false); > } > >+static bool is_kadmin_changepw(krb5_context context, >+ krb5_const_principal principal) >+{ >+ return krb5_princ_size(context, principal) == 2 && >+ (principal_comp_strcmp(context, principal, 0, "kadmin") == 0) && >+ (principal_comp_strcmp(context, principal, 1, "changepw") == 0); >+} >+ > /* > * Construct an hdb_entry from a directory entry. > */ >@@ -1221,11 +1229,9 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, > * 'change password', as otherwise we could get into > * trouble, and not enforce the password expirty. > * Instead, only do it when request is for the kpasswd service */ >- if (ent_type == SAMBA_KDC_ENT_TYPE_SERVER >- && krb5_princ_size(context, principal) == 2 >- && (principal_comp_strcmp(context, principal, 0, "kadmin") == 0) >- && (principal_comp_strcmp(context, principal, 1, "changepw") == 0) >- && lpcfg_is_my_domain_or_realm(lp_ctx, realm)) { >+ if (ent_type == SAMBA_KDC_ENT_TYPE_SERVER && >+ is_kadmin_changepw(context, principal) && >+ lpcfg_is_my_domain_or_realm(lp_ctx, realm)) { > entry->flags.change_pw = 1; > } > >-- >2.36.1 > > >From dcf4690e8f85dd49fd9a439d776947bd74f2178a Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Tue, 24 May 2022 09:56:28 +0200 >Subject: [PATCH 4/6] CVE-XXXX-XXXXX: s4:kdc: Do not create a krbtgt for > kadmin/changepw > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 > >Signed-off-by: Andreas Schneider <asn@samba.org> >--- > selftest/knownfail.d/kadmin_changew | 1 - > source4/kdc/db-glue.c | 4 +++- > 2 files changed, 3 insertions(+), 2 deletions(-) > delete mode 100644 selftest/knownfail.d/kadmin_changew > >diff --git a/selftest/knownfail.d/kadmin_changew b/selftest/knownfail.d/kadmin_changew >deleted file mode 100644 >index 97c14793ea5..00000000000 >--- a/selftest/knownfail.d/kadmin_changew >+++ /dev/null >@@ -1 +0,0 @@ >-^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password >diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c >index b03a32b67f5..059d14f9fed 100644 >--- a/source4/kdc/db-glue.c >+++ b/source4/kdc/db-glue.c >@@ -1070,7 +1070,9 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, > goto out; > } > } else if ((flags & SDB_F_FORCE_CANON) || >- ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { >+ ((flags & SDB_F_CANON) && >+ (flags & SDB_F_FOR_AS_REQ) && >+ !is_kadmin_changepw(context, principal))) { > /* > * SDB_F_CANON maps from the canonicalize flag in the > * packet, and has a different meaning between AS-REQ >-- >2.36.1 > > >From 81f62aea861eb3e0e418b9947bad8ab2808bce18 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Tue, 24 May 2022 10:33:38 +0200 >Subject: [PATCH 5/6] Revert "CVE-XXXX-XXXXX: s4:kdc: Do not create a krbtgt > for kadmin/changepw" > >This reverts commit 3ac243ac762795a6262633890b66900f8caa12dc. >--- > selftest/knownfail.d/kadmin_changew | 1 + > source4/kdc/db-glue.c | 4 +--- > 2 files changed, 2 insertions(+), 3 deletions(-) > create mode 100644 selftest/knownfail.d/kadmin_changew > >diff --git a/selftest/knownfail.d/kadmin_changew b/selftest/knownfail.d/kadmin_changew >new file mode 100644 >index 00000000000..97c14793ea5 >--- /dev/null >+++ b/selftest/knownfail.d/kadmin_changew >@@ -0,0 +1 @@ >+^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password >diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c >index 059d14f9fed..b03a32b67f5 100644 >--- a/source4/kdc/db-glue.c >+++ b/source4/kdc/db-glue.c >@@ -1070,9 +1070,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, > goto out; > } > } else if ((flags & SDB_F_FORCE_CANON) || >- ((flags & SDB_F_CANON) && >- (flags & SDB_F_FOR_AS_REQ) && >- !is_kadmin_changepw(context, principal))) { >+ ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { > /* > * SDB_F_CANON maps from the canonicalize flag in the > * packet, and has a different meaning between AS-REQ >-- >2.36.1 > > >From 94f2824d1fe0bc672dd4993a57d52b2f2e89d535 Mon Sep 17 00:00:00 2001 >From: Joseph Sutton <josephsutton@catalyst.net.nz> >Date: Wed, 18 May 2022 16:56:01 +1200 >Subject: [PATCH 6/6] CVE-2022-XXXXXX: s4:kdc: Fix canonicalisation of > kadmin/changepw principal > >Since this principal goes through the samba_kdc_fetch_server() path, >setting the canonicalisation flag would cause the principal to be >replaced with the sAMAccountName; this meant requests to >kadmin/changepw@REALM would result in a ticket to krbtgt@REALM. Now we >properly handle canonicalisation for the kadmin/changepw principal. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 > >Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> >Reviewed-by: Andreas Schneider <asn@samba.org> >--- > selftest/knownfail.d/kadmin_changew | 1 - > source4/kdc/db-glue.c | 176 ++++++++++++++++------------ > 2 files changed, 103 insertions(+), 74 deletions(-) > delete mode 100644 selftest/knownfail.d/kadmin_changew > >diff --git a/selftest/knownfail.d/kadmin_changew b/selftest/knownfail.d/kadmin_changew >deleted file mode 100644 >index 97c14793ea5..00000000000 >--- a/selftest/knownfail.d/kadmin_changew >+++ /dev/null >@@ -1 +0,0 @@ >-^samba4.blackbox.kpasswd.MIT kpasswd.change.user.password >diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c >index b03a32b67f5..838cc06061e 100644 >--- a/source4/kdc/db-glue.c >+++ b/source4/kdc/db-glue.c >@@ -925,6 +925,96 @@ static bool is_kadmin_changepw(krb5_context context, > (principal_comp_strcmp(context, principal, 1, "changepw") == 0); > } > >+static krb5_error_code samba_kdc_get_entry_principal( >+ krb5_context context, >+ struct samba_kdc_db_context *kdc_db_ctx, >+ const char *samAccountName, >+ enum samba_kdc_ent_type ent_type, >+ unsigned flags, >+ bool is_kadmin_changepw, >+ krb5_const_principal in_princ, >+ krb5_principal *out_princ) >+{ >+ struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx; >+ krb5_error_code code = 0; >+ >+ if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && >+ in_princ == NULL && >+ !is_kadmin_changepw) { >+ code = smb_krb5_make_principal(context, >+ out_princ, >+ lpcfg_realm(lp_ctx), >+ samAccountName, NULL); >+ return code; >+ } >+ >+ /* >+ * We need to ensure that the kadmin/changepw princial it isn't able to >+ * get krbtgt ticket, even if canonicalization is turned on. >+ */ >+ if ((flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) && !is_kadmin_changepw) { >+ if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { >+ /* >+ * When requested to do so, ensure that the >+ * both realm values in the principal are set >+ * to the upper case, canonical realm >+ */ >+ code = smb_krb5_make_principal(context, >+ out_princ, >+ lpcfg_realm(lp_ctx), >+ "krbtgt", >+ lpcfg_realm(lp_ctx), >+ NULL); >+ if (code != 0) { >+ return code; >+ } >+ smb_krb5_principal_set_type(context, >+ *out_princ, >+ KRB5_NT_SRV_INST); >+ >+ return 0; >+ } >+ >+ if (flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) { >+ /* >+ * SDB_F_CANON maps from the canonicalize flag in the >+ * packet, and has a different meaning between AS-REQ >+ * and TGS-REQ. We only change the principal in the >+ * AS-REQ case. >+ * >+ * The SDB_F_FORCE_CANON if for new MIT KDC code that >+ * wants the canonical name in all lookups, and takes >+ * care to canonicalize only when appropriate. >+ */ >+ code = smb_krb5_make_principal(context, >+ out_princ, >+ lpcfg_realm(lp_ctx), >+ samAccountName, >+ NULL); >+ return code; >+ } >+ } >+ >+ /* >+ * For a krbtgt entry, this appears to be required regardless of the >+ * canonicalize flag from the client. >+ */ >+ code = krb5_copy_principal(context, in_princ, out_princ); >+ if (code != 0) { >+ return code; >+ } >+ >+ /* >+ * While we have copied the client principal, tests show that Win2k3 >+ * returns the 'corrected' realm, not the client-specified realm. This >+ * code attempts to replace the client principal's realm with the one >+ * we determine from our records >+ */ >+ code = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); >+ >+ return code; >+} >+ > /* > * Construct an hdb_entry from a directory entry. > */ >@@ -1031,79 +1121,6 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, > > if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { > p->is_krbtgt = true; >- >- if (flags & (SDB_F_CANON|SDB_F_FORCE_CANON)) { >- /* >- * When requested to do so, ensure that the >- * both realm values in the principal are set >- * to the upper case, canonical realm >- */ >- ret = smb_krb5_make_principal(context, &entry->principal, >- lpcfg_realm(lp_ctx), "krbtgt", >- lpcfg_realm(lp_ctx), NULL); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } >- smb_krb5_principal_set_type(context, entry->principal, KRB5_NT_SRV_INST); >- } else { >- ret = krb5_copy_principal(context, principal, &entry->principal); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } >- /* >- * this appears to be required regardless of >- * the canonicalize flag from the client >- */ >- ret = smb_krb5_principal_set_realm(context, entry->principal, lpcfg_realm(lp_ctx)); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } >- } >- >- } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && principal == NULL) { >- ret = smb_krb5_make_principal(context, &entry->principal, lpcfg_realm(lp_ctx), samAccountName, NULL); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } >- } else if ((flags & SDB_F_FORCE_CANON) || >- ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { >- /* >- * SDB_F_CANON maps from the canonicalize flag in the >- * packet, and has a different meaning between AS-REQ >- * and TGS-REQ. We only change the principal in the AS-REQ case >- * >- * The SDB_F_FORCE_CANON if for new MIT KDC code that wants >- * the canonical name in all lookups, and takes care to >- * canonicalize only when appropriate. >- */ >- ret = smb_krb5_make_principal(context, &entry->principal, lpcfg_realm(lp_ctx), samAccountName, NULL); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } >- } else { >- ret = krb5_copy_principal(context, principal, &entry->principal); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } >- >- /* While we have copied the client principal, tests >- * show that Win2k3 returns the 'corrected' realm, not >- * the client-specified realm. This code attempts to >- * replace the client principal's realm with the one >- * we determine from our records */ >- >- /* this has to be with malloc() */ >- ret = smb_krb5_principal_set_realm(context, entry->principal, lpcfg_realm(lp_ctx)); >- if (ret) { >- krb5_clear_error_message(context); >- goto out; >- } > } > > /* First try and figure out the flags based on the userAccountControl */ >@@ -1296,6 +1313,19 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, > } > } > >+ ret = samba_kdc_get_entry_principal(context, >+ kdc_db_ctx, >+ samAccountName, >+ ent_type, >+ flags, >+ entry->flags.change_pw, >+ principal, >+ &entry->principal); >+ if (ret != 0) { >+ krb5_clear_error_message(context); >+ goto out; >+ } >+ > entry->valid_start = NULL; > > entry->max_life = malloc(sizeof(*entry->max_life)); >-- >2.36.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 15047
:
17299
|
17300
|
17302
|
17303
|
17305
|
17306
|
17307
|
17308
|
17309
|
17310
|
17311
|
17312
|
17313
|
17337
|
17342
|
17346
|
17352
|
17353
|
17357
|
17358
|
17359
|
17360
|
17361
|
17368
|
17372
|
17373
|
17394
|
17395
|
17396
|
17397
|
17398
|
17433
|
17446
|
17450