From 861dc25f14a31f88020ff0ffd74bfe344361f4ea Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:17:02 +1200 Subject: [PATCH 01/46] s4:kpasswd: Account for missing target principal This field is supposed to be optional. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-service-mit.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/source4/kdc/kpasswd-service-mit.c b/source4/kdc/kpasswd-service-mit.c index 1dbe5adad6d..670210819a1 100644 --- a/source4/kdc/kpasswd-service-mit.c +++ b/source4/kdc/kpasswd-service-mit.c @@ -142,16 +142,18 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, return KRB5_KPASSWD_HARDERROR; } - target_realm = smb_krb5_principal_get_realm( - mem_ctx, context, target_principal); - code = krb5_unparse_name_flags(context, - target_principal, - KRB5_PRINCIPAL_UNPARSE_NO_REALM, - &target_name); - if (code != 0) { - DBG_WARNING("Failed to parse principal\n"); - *error_string = "String conversion failed"; - return KRB5_KPASSWD_HARDERROR; + if (target_principal != NULL) { + target_realm = smb_krb5_principal_get_realm( + mem_ctx, context, target_principal); + code = krb5_unparse_name_flags(context, + target_principal, + KRB5_PRINCIPAL_UNPARSE_NO_REALM, + &target_name); + if (code != 0) { + DBG_WARNING("Failed to parse principal\n"); + *error_string = "String conversion failed"; + return KRB5_KPASSWD_HARDERROR; + } } if ((target_name != NULL && target_realm == NULL) || -- 2.35.0 From a2125a75aab737063ce0e359d1d9b3ec90732263 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:32:44 +1200 Subject: [PATCH 02/46] s3:libads: Refactor ads_krb5_chg_password() We simplify the function by having a common cleanup path. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source3/libads/krb5_setpw.c | 42 +++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/source3/libads/krb5_setpw.c b/source3/libads/krb5_setpw.c index 8f638dcdb8e..131a0080188 100644 --- a/source3/libads/krb5_setpw.c +++ b/source3/libads/krb5_setpw.c @@ -181,21 +181,23 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, if (ret) { DBG_ERR("kerberos init context failed (%s)\n", error_message(ret)); - return ADS_ERROR_KRB5(ret); + aret = ADS_ERROR_KRB5(ret); + goto done; } - if ((ret = smb_krb5_parse_name(context, principal, &princ))) { - krb5_free_context(context); + ret = smb_krb5_parse_name(context, principal, &princ); + if (ret) { DEBUG(1,("Failed to parse %s (%s)\n", principal, error_message(ret))); - return ADS_ERROR_KRB5(ret); + aret = ADS_ERROR_KRB5(ret); + goto done; } ret = krb5_get_init_creds_opt_alloc(context, &opts); if (ret != 0) { - krb5_free_context(context); DBG_WARNING("krb5_get_init_creds_opt_alloc failed: %s\n", error_message(ret)); - return ADS_ERROR_KRB5(ret); + aret = ADS_ERROR_KRB5(ret); + goto done; } krb5_get_init_creds_opt_set_tkt_life(opts, 5 * 60); @@ -230,33 +232,26 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, * - avoids this - gd */ ret = smb_krb5_gen_netbios_krb5_address(&addr, lp_netbios_name()); if (ret) { - krb5_free_principal(context, princ); - krb5_get_init_creds_opt_free(context, opts); - krb5_free_context(context); - return ADS_ERROR_KRB5(ret); + aret = ADS_ERROR_KRB5(ret); + goto done; } krb5_get_init_creds_opt_set_address_list(opts, addr->addrs); realm = smb_krb5_principal_get_realm(NULL, context, princ); /* We have to obtain an INITIAL changepw ticket for changing password */ - if (asprintf(&chpw_princ, "kadmin/changepw@%s", realm) == -1) { - krb5_free_principal(context, princ); - krb5_get_init_creds_opt_free(context, opts); - smb_krb5_free_addresses(context, addr); - krb5_free_context(context); - TALLOC_FREE(realm); + ret = asprintf(&chpw_princ, "kadmin/changepw@%s", realm); + TALLOC_FREE(realm); + if (ret == -1) { DEBUG(1, ("ads_krb5_chg_password: asprintf fail\n")); - return ADS_ERROR_NT(NT_STATUS_NO_MEMORY); + aret = ADS_ERROR_NT(NT_STATUS_NO_MEMORY); + goto done; } - TALLOC_FREE(realm); password = SMB_STRDUP(oldpw); ret = krb5_get_init_creds_password(context, &creds, princ, password, kerb_prompter, NULL, 0, chpw_princ, opts); - krb5_get_init_creds_opt_free(context, opts); - smb_krb5_free_addresses(context, addr); SAFE_FREE(chpw_princ); SAFE_FREE(password); @@ -266,9 +261,8 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, } else { DEBUG(1,("krb5_get_init_creds_password failed (%s)\n", error_message(ret))); } - krb5_free_principal(context, princ); - krb5_free_context(context); - return ADS_ERROR_KRB5(ret); + aret = ADS_ERROR_KRB5(ret); + goto done; } ret = krb5_set_password(context, @@ -297,6 +291,8 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, done: smb_krb5_free_data_contents(context, &result_code_string); smb_krb5_free_data_contents(context, &result_string); + smb_krb5_free_addresses(context, addr); + krb5_get_init_creds_opt_free(context, opts); krb5_free_principal(context, princ); krb5_free_context(context); -- 2.35.0 From 19bb8d4a75f3e00862973f5b703f4ab32c4aca3c Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:37:48 +1200 Subject: [PATCH 03/46] s3:libads: Add target_principal to ads_krb5_chg_password() The function can now change the password of a different account specified with a non-NULL target_principal value. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source3/libads/krb5_setpw.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/source3/libads/krb5_setpw.c b/source3/libads/krb5_setpw.c index 131a0080188..5b33a12c03b 100644 --- a/source3/libads/krb5_setpw.c +++ b/source3/libads/krb5_setpw.c @@ -159,15 +159,17 @@ kerb_prompter(krb5_context ctx, void *data, } static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, - const char *principal, - const char *oldpw, + const char *auth_principal, + const char *authpw, + const char *target_principal, const char *newpw, int time_offset) { ADS_STATUS aret; krb5_error_code ret; krb5_context context = NULL; - krb5_principal princ; + krb5_principal auth_princ = NULL; + krb5_principal target_princ = NULL; krb5_get_init_creds_opt *opts = NULL; krb5_creds creds; char *chpw_princ = NULL, *password; @@ -185,13 +187,22 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, goto done; } - ret = smb_krb5_parse_name(context, principal, &princ); + ret = smb_krb5_parse_name(context, auth_principal, &auth_princ); if (ret) { - DEBUG(1,("Failed to parse %s (%s)\n", principal, error_message(ret))); + DEBUG(1,("Failed to parse %s (%s)\n", auth_principal, error_message(ret))); aret = ADS_ERROR_KRB5(ret); goto done; } + if (target_principal != NULL) { + ret = smb_krb5_parse_name(context, target_principal, &target_princ); + if (ret) { + DEBUG(1,("Failed to parse %s (%s)\n", target_principal, error_message(ret))); + aret = ADS_ERROR_KRB5(ret); + goto done; + } + } + ret = krb5_get_init_creds_opt_alloc(context, &opts); if (ret != 0) { DBG_WARNING("krb5_get_init_creds_opt_alloc failed: %s\n", @@ -237,7 +248,7 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, } krb5_get_init_creds_opt_set_address_list(opts, addr->addrs); - realm = smb_krb5_principal_get_realm(NULL, context, princ); + realm = smb_krb5_principal_get_realm(NULL, context, auth_princ); /* We have to obtain an INITIAL changepw ticket for changing password */ ret = asprintf(&chpw_princ, "kadmin/changepw@%s", realm); @@ -248,8 +259,8 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, goto done; } - password = SMB_STRDUP(oldpw); - ret = krb5_get_init_creds_password(context, &creds, princ, password, + password = SMB_STRDUP(authpw); + ret = krb5_get_init_creds_password(context, &creds, auth_princ, password, kerb_prompter, NULL, 0, chpw_princ, opts); SAFE_FREE(chpw_princ); @@ -268,7 +279,7 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, ret = krb5_set_password(context, &creds, discard_const_p(char, newpw), - NULL, + target_princ, &result_code, &result_code_string, &result_string); @@ -293,7 +304,8 @@ static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, smb_krb5_free_data_contents(context, &result_string); smb_krb5_free_addresses(context, addr); krb5_get_init_creds_opt_free(context, opts); - krb5_free_principal(context, princ); + krb5_free_principal(context, target_princ); + krb5_free_principal(context, auth_princ); krb5_free_context(context); return aret; @@ -313,8 +325,9 @@ ADS_STATUS kerberos_set_password(const char *kpasswd_server, } if (!strcmp(auth_principal, target_principal)) { - return ads_krb5_chg_password(kpasswd_server, target_principal, - auth_password, new_password, + return ads_krb5_chg_password(kpasswd_server, + auth_principal, auth_password, + NULL, new_password, time_offset); } else { return ads_krb5_set_password(kpasswd_server, target_principal, -- 2.35.0 From a350774b38c946c8feb67a25509ac02462e3cc07 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:43:08 +1200 Subject: [PATCH 04/46] s3:libads: Have ads_krb5_chg_password() always use kerberos_set_password() ads_krb5_chg_password() now operates in essentially the same way as ads_krb5_set_password() -- they are both wrappers around the same function, krb5_set_password(). The difference is that the former obtains an initial ticket to kadmin/changepw, but the latter attempts to get a ticket matching kadmin/changepw from a credentials cache. We want to move away from the latter behaviour and ensure we always have an initial ticket, so we prefer the former function. Since we are no longer using a credentials cache, this also removes the need for performing a kinit prior to the password set or change. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source3/libads/kerberos_proto.h | 6 ++++++ source3/libads/krb5_setpw.c | 32 +++++++++++++------------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/source3/libads/kerberos_proto.h b/source3/libads/kerberos_proto.h index 807381248c8..3027a7058eb 100644 --- a/source3/libads/kerberos_proto.h +++ b/source3/libads/kerberos_proto.h @@ -86,6 +86,12 @@ NTSTATUS kerberos_return_pac(TALLOC_CTX *mem_ctx, ADS_STATUS ads_krb5_set_password(const char *kdc_host, const char *princ, const char *newpw, int time_offset); +ADS_STATUS ads_krb5_chg_password(const char *kdc_host, + const char *auth_principal, + const char *authpw, + const char *target_principal, + const char *newpw, + int time_offset); ADS_STATUS kerberos_set_password(const char *kpasswd_server, const char *auth_principal, const char *auth_password, const char *target_principal, const char *new_password, diff --git a/source3/libads/krb5_setpw.c b/source3/libads/krb5_setpw.c index 5b33a12c03b..81aec95f808 100644 --- a/source3/libads/krb5_setpw.c +++ b/source3/libads/krb5_setpw.c @@ -158,12 +158,12 @@ kerb_prompter(krb5_context ctx, void *data, return 0; } -static ADS_STATUS ads_krb5_chg_password(const char *kdc_host, - const char *auth_principal, - const char *authpw, - const char *target_principal, - const char *newpw, - int time_offset) +ADS_STATUS ads_krb5_chg_password(const char *kdc_host, + const char *auth_principal, + const char *authpw, + const char *target_principal, + const char *newpw, + int time_offset) { ADS_STATUS aret; krb5_error_code ret; @@ -317,22 +317,16 @@ ADS_STATUS kerberos_set_password(const char *kpasswd_server, const char *target_principal, const char *new_password, int time_offset) { - int ret; - - if ((ret = kerberos_kinit_password(auth_principal, auth_password, time_offset, NULL))) { - DEBUG(1,("Failed kinit for principal %s (%s)\n", auth_principal, error_message(ret))); - return ADS_ERROR_KRB5(ret); - } if (!strcmp(auth_principal, target_principal)) { - return ads_krb5_chg_password(kpasswd_server, - auth_principal, auth_password, - NULL, new_password, - time_offset); - } else { - return ads_krb5_set_password(kpasswd_server, target_principal, - new_password, time_offset); + /* We're changing our own password. */ + target_principal = NULL; } + + return ads_krb5_chg_password(kpasswd_server, + auth_principal, auth_password, + target_principal, new_password, + time_offset); } #endif -- 2.35.0 From a3f3aee3ec785391ab20f6af666af24bd98fa0da Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:43:42 +1200 Subject: [PATCH 05/46] s3:utils: Use ads_krb5_chg_password() in ads_user_add() ads_krb5_chg_password() now operates in essentially the same way as ads_krb5_set_password() -- they are both wrappers around the same function, krb5_set_password(). The difference is that the former obtains an initial ticket to kadmin/changepw, but the latter attempts to get a ticket matching kadmin/changepw from a credentials cache. We want to move away from the latter behaviour and ensure we always have an initial ticket, so we prefer the former function, passing in the administrator credentials. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source3/utils/net_ads.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c index d666f7fc3ec..434da5245f2 100644 --- a/source3/utils/net_ads.c +++ b/source3/utils/net_ads.c @@ -852,6 +852,8 @@ static int net_ads_user_usage(struct net_context *c, int argc, const char **argv static int ads_user_add(struct net_context *c, int argc, const char **argv) { ADS_STRUCT *ads; + const char *auth_principal = cli_credentials_get_username(c->creds); + const char *auth_password = cli_credentials_get_password(c->creds); ADS_STATUS status; char *upn, *userdn; LDAPMessage *res=NULL; @@ -861,6 +863,18 @@ static int ads_user_add(struct net_context *c, int argc, const char **argv) if (argc < 1 || c->display_usage) return net_ads_user_usage(c, argc, argv); + if (argc == 1) { + /* + * If we're changing a password, ensure we have admin + * credentials. + */ + if (auth_principal == NULL || auth_password == NULL) { + d_fprintf(stderr, _("You must supply an administrator " + "username/password\n")); + return -1; + } + } + if (!ADS_ERR_OK(ads_startup(c, false, &ads))) { return -1; } @@ -903,7 +917,9 @@ static int ads_user_add(struct net_context *c, int argc, const char **argv) if (asprintf(&upn, "%s@%s", argv[0], ads->config.realm) == -1) { goto done; } - status = ads_krb5_set_password(ads->auth.kdc_server, upn, argv[1], + status = ads_krb5_chg_password(ads->auth.kdc_server, + auth_principal, auth_password, + upn, argv[1], ads->auth.time_offset); SAFE_FREE(upn); if (ADS_ERR_OK(status)) { -- 2.35.0 From 16180257ae94f1ccb91f21435d0c030ca1d6f409 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:43:58 +1200 Subject: [PATCH 06/46] s3:libads: Remove unused ads_krb5_set_password() This function is no longer used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source3/libads/kerberos_proto.h | 2 - source3/libads/krb5_setpw.c | 74 --------------------------------- 2 files changed, 76 deletions(-) diff --git a/source3/libads/kerberos_proto.h b/source3/libads/kerberos_proto.h index 3027a7058eb..341e0a21966 100644 --- a/source3/libads/kerberos_proto.h +++ b/source3/libads/kerberos_proto.h @@ -84,8 +84,6 @@ NTSTATUS kerberos_return_pac(TALLOC_CTX *mem_ctx, /* The following definitions come from libads/krb5_setpw.c */ -ADS_STATUS ads_krb5_set_password(const char *kdc_host, const char *princ, - const char *newpw, int time_offset); ADS_STATUS ads_krb5_chg_password(const char *kdc_host, const char *auth_principal, const char *authpw, diff --git a/source3/libads/krb5_setpw.c b/source3/libads/krb5_setpw.c index 81aec95f808..5fb5a5ab4ed 100644 --- a/source3/libads/krb5_setpw.c +++ b/source3/libads/krb5_setpw.c @@ -56,80 +56,6 @@ static krb5_error_code kpasswd_err_to_krb5_err(krb5_error_code res_code) } } -ADS_STATUS ads_krb5_set_password(const char *kdc_host, const char *principal, - const char *newpw, int time_offset) -{ - - ADS_STATUS aret; - krb5_error_code ret = 0; - krb5_context context = NULL; - krb5_principal princ = NULL; - krb5_ccache ccache = NULL; - int result_code; - krb5_data result_code_string = { 0 }; - krb5_data result_string = { 0 }; - - ret = smb_krb5_init_context_common(&context); - if (ret) { - DBG_ERR("kerberos init context failed (%s)\n", - error_message(ret)); - return ADS_ERROR_KRB5(ret); - } - - if (principal) { - ret = smb_krb5_parse_name(context, principal, &princ); - if (ret) { - krb5_free_context(context); - DEBUG(1, ("Failed to parse %s (%s)\n", principal, - error_message(ret))); - return ADS_ERROR_KRB5(ret); - } - } - - if (time_offset != 0) { - krb5_set_real_time(context, time(NULL) + time_offset, 0); - } - - ret = krb5_cc_default(context, &ccache); - if (ret) { - krb5_free_principal(context, princ); - krb5_free_context(context); - DEBUG(1,("Failed to get default creds (%s)\n", error_message(ret))); - return ADS_ERROR_KRB5(ret); - } - - ret = krb5_set_password_using_ccache(context, - ccache, - discard_const_p(char, newpw), - princ, - &result_code, - &result_code_string, - &result_string); - if (ret) { - DEBUG(1, ("krb5_set_password failed (%s)\n", error_message(ret))); - aret = ADS_ERROR_KRB5(ret); - goto done; - } - - if (result_code != KRB5_KPASSWD_SUCCESS) { - ret = kpasswd_err_to_krb5_err(result_code); - DEBUG(1, ("krb5_set_password failed (%s)\n", error_message(ret))); - aret = ADS_ERROR_KRB5(ret); - goto done; - } - - aret = ADS_SUCCESS; - - done: - smb_krb5_free_data_contents(context, &result_code_string); - smb_krb5_free_data_contents(context, &result_string); - krb5_free_principal(context, princ); - krb5_cc_close(context, ccache); - krb5_free_context(context); - - return aret; -} - /* we use a prompter to avoid a crash bug in the kerberos libs when dealing with empty passwords -- 2.35.0 From 53930ccf5741acf71f3d1a7866e4d9f76b81c247 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:44:32 +1200 Subject: [PATCH 07/46] testprogs: Always provide a password to kpasswd If we specify the --cache parameter, kpasswd will fetch a service ticket to kadmin/changepw using a TGT from the credentials cache, and then attempt to perform the password change. However, getting a service ticket is wrong, as the kpasswd service expects an initial ticket obtained with an AS-REQ, and will fail the password change. By providing a password to kpasswd rather than a credentials cache path, kpasswd will correctly make an initial request to kadmin/changepw. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- testprogs/blackbox/test_kinit_heimdal.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testprogs/blackbox/test_kinit_heimdal.sh b/testprogs/blackbox/test_kinit_heimdal.sh index 7a3ff684135..4759248ee83 100755 --- a/testprogs/blackbox/test_kinit_heimdal.sh +++ b/testprogs/blackbox/test_kinit_heimdal.sh @@ -169,12 +169,15 @@ testit "change user password with kpasswd" $texpect $PREFIX/tmpkpasswdscript $sa rm -f $KRB5CCNAME_PATH testit "kinit with user password (after kpasswd change)" $samba4kinit $enctype --password-file=$PREFIX/tmpuserpassfile --request-pac nettestuser@$REALM || failed=`expr $failed + 1` +USERPASS=$NEWUSERPASS NEWUSERPASS=testPaSS@78% echo $NEWUSERPASS > $PREFIX/tmpuserpassfile test_smbclient "Test login with user kerberos ccache (after kpasswd change)" 'ls' "$unc" --use-krb5-ccache=$KRB5CCNAME || failed=`expr $failed + 1` cat > $PREFIX/tmpkpasswdscript < Date: Sat, 28 May 2022 17:47:48 +1200 Subject: [PATCH 08/46] testprogs: Remove servicePrincipalName kpasswd test Getting an initial ticket for kpasswd with a servicePrincipalName will fail, as the KDC will only find servers and not clients given a servicePrincipalName. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- testprogs/blackbox/test_kinit_heimdal.sh | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/testprogs/blackbox/test_kinit_heimdal.sh b/testprogs/blackbox/test_kinit_heimdal.sh index 4759248ee83..07162bfc520 100755 --- a/testprogs/blackbox/test_kinit_heimdal.sh +++ b/testprogs/blackbox/test_kinit_heimdal.sh @@ -92,13 +92,11 @@ BASEDN=`$ldbsearch $options --basedn='' -H ldap://$SERVER --scope=base DUMMY=x d cat > $PREFIX/tmpldbmodify < $PREFIX/tmpuserpassfile - -cat > $PREFIX/tmpkpasswdscript < $PREFIX/tmpldbmodify < Date: Thu, 26 May 2022 16:34:01 +1200 Subject: [PATCH 09/46] tests/krb5: Correctly handle specifying account kvno The environment variable is a string, but we expect an integer. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/raw_testcase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 7f9d9d17640..d0c28fb2002 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -759,7 +759,7 @@ class RawKerberosTest(TestCaseInTempDir): fallback_default=False, allow_missing=kvno_allow_missing) if kvno is not None: - c.set_kvno(kvno) + c.set_kvno(int(kvno)) aes256_key = self.env_get_var('AES256_KEY_HEX', prefix, fallback_default=False, allow_missing=aes256_allow_missing) -- 2.35.0 From c7fca668a086cb8a1b5ea77ba872327afbc49cd7 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:09:58 +1200 Subject: [PATCH 10/46] tests/krb5: Use object() rather than auto() to initialise enums This ensures that when an enum value is expected, a magic constant won't be supplied instead. Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 7d180380d13..22db004f879 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -23,7 +23,7 @@ import tempfile import binascii import collections import secrets -from enum import Enum, auto +from enum import Enum from collections import namedtuple import ldb @@ -98,10 +98,10 @@ class KDCBaseTest(RawKerberosTest): """ class AccountType(Enum): - USER = auto() - COMPUTER = auto() - SERVER = auto() - RODC = auto() + USER = object() + COMPUTER = object() + SERVER = object() + RODC = object() @classmethod def setUpClass(cls): -- 2.35.0 From 00263df1b0caff3cd5a3e1cd221b6a22e4036238 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 20:52:04 +1200 Subject: [PATCH 11/46] tests/krb5: Split out _make_tgs_request() This allows us to make use of it in other tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 85 ++++++++++++++++++++++++ python/samba/tests/krb5/kdc_tgs_tests.py | 84 ----------------------- 2 files changed, 85 insertions(+), 84 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 22db004f879..6c9fe6668d6 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -72,6 +72,7 @@ from samba.tests.krb5.rfc4120_constants import ( AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5, KDC_ERR_PREAUTH_REQUIRED, + KDC_ERR_TGT_REVOKED, KRB_AS_REP, KRB_TGS_REP, KRB_ERROR, @@ -1663,6 +1664,90 @@ class KDCBaseTest(RawKerberosTest): return ticket_creds + def _make_tgs_request(self, client_creds, service_creds, tgt, + client_account=None, + client_name_type=NT_PRINCIPAL, + kdc_options=None, + pac_request=None, expect_pac=True, + expect_error=False, + expected_cname=None, + expected_account_name=None, + expected_upn_name=None, + expected_sid=None): + if client_account is None: + client_account = client_creds.get_username() + cname = self.PrincipalName_create(name_type=client_name_type, + names=client_account.split('/')) + + service_account = service_creds.get_username() + sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=[service_account]) + + realm = service_creds.get_realm() + + expected_crealm = realm + if expected_cname is None: + expected_cname = cname + expected_srealm = realm + expected_sname = sname + + expected_supported_etypes = service_creds.tgs_supported_enctypes + + etypes = (AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5) + + if kdc_options is None: + kdc_options = 'canonicalize' + kdc_options = str(krb5_asn1.KDCOptions(kdc_options)) + + target_decryption_key = self.TicketDecryptionKey_from_creds( + service_creds) + + authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) + + if expect_error: + expected_error_mode = KDC_ERR_TGT_REVOKED + check_error_fn = self.generic_check_kdc_error + check_rep_fn = None + else: + expected_error_mode = 0 + check_error_fn = None + check_rep_fn = self.generic_check_kdc_rep + + kdc_exchange_dict = self.tgs_exchange_dict( + expected_crealm=expected_crealm, + expected_cname=expected_cname, + expected_srealm=expected_srealm, + expected_sname=expected_sname, + expected_account_name=expected_account_name, + expected_upn_name=expected_upn_name, + expected_sid=expected_sid, + expected_supported_etypes=expected_supported_etypes, + ticket_decryption_key=target_decryption_key, + check_error_fn=check_error_fn, + check_rep_fn=check_rep_fn, + check_kdc_private_fn=self.generic_check_kdc_private, + expected_error_mode=expected_error_mode, + tgt=tgt, + authenticator_subkey=authenticator_subkey, + kdc_options=kdc_options, + pac_request=pac_request, + expect_pac=expect_pac, + expect_edata=False) + + rep = self._generic_kdc_exchange(kdc_exchange_dict, + cname=cname, + realm=realm, + sname=sname, + etypes=etypes) + if expect_error: + self.check_error_rep(rep, expected_error_mode) + + return None + else: + self.check_reply(rep, KRB_TGS_REP) + + return kdc_exchange_dict['rep_ticket_creds'] + # Named tuple to contain values of interest when the PAC is decoded. PacData = namedtuple( "PacData", diff --git a/python/samba/tests/krb5/kdc_tgs_tests.py b/python/samba/tests/krb5/kdc_tgs_tests.py index 1f16d05e2db..83315f6879f 100755 --- a/python/samba/tests/krb5/kdc_tgs_tests.py +++ b/python/samba/tests/krb5/kdc_tgs_tests.py @@ -231,90 +231,6 @@ class KdcTgsTests(KDCBaseTest): pac_data.account_sid, "rep = {%s},%s" % (rep, pac_data)) - def _make_tgs_request(self, client_creds, service_creds, tgt, - client_account=None, - client_name_type=NT_PRINCIPAL, - kdc_options=None, - pac_request=None, expect_pac=True, - expect_error=False, - expected_cname=None, - expected_account_name=None, - expected_upn_name=None, - expected_sid=None): - if client_account is None: - client_account = client_creds.get_username() - cname = self.PrincipalName_create(name_type=client_name_type, - names=client_account.split('/')) - - service_account = service_creds.get_username() - sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, - names=[service_account]) - - realm = service_creds.get_realm() - - expected_crealm = realm - if expected_cname is None: - expected_cname = cname - expected_srealm = realm - expected_sname = sname - - expected_supported_etypes = service_creds.tgs_supported_enctypes - - etypes = (AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5) - - if kdc_options is None: - kdc_options = 'canonicalize' - kdc_options = str(krb5_asn1.KDCOptions(kdc_options)) - - target_decryption_key = self.TicketDecryptionKey_from_creds( - service_creds) - - authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) - - if expect_error: - expected_error_mode = KDC_ERR_TGT_REVOKED - check_error_fn = self.generic_check_kdc_error - check_rep_fn = None - else: - expected_error_mode = 0 - check_error_fn = None - check_rep_fn = self.generic_check_kdc_rep - - kdc_exchange_dict = self.tgs_exchange_dict( - expected_crealm=expected_crealm, - expected_cname=expected_cname, - expected_srealm=expected_srealm, - expected_sname=expected_sname, - expected_account_name=expected_account_name, - expected_upn_name=expected_upn_name, - expected_sid=expected_sid, - expected_supported_etypes=expected_supported_etypes, - ticket_decryption_key=target_decryption_key, - check_error_fn=check_error_fn, - check_rep_fn=check_rep_fn, - check_kdc_private_fn=self.generic_check_kdc_private, - expected_error_mode=expected_error_mode, - tgt=tgt, - authenticator_subkey=authenticator_subkey, - kdc_options=kdc_options, - pac_request=pac_request, - expect_pac=expect_pac, - expect_edata=False) - - rep = self._generic_kdc_exchange(kdc_exchange_dict, - cname=cname, - realm=realm, - sname=sname, - etypes=etypes) - if expect_error: - self.check_error_rep(rep, expected_error_mode) - - return None - else: - self.check_reply(rep, KRB_TGS_REP) - - return kdc_exchange_dict['rep_ticket_creds'] - def test_request(self): client_creds = self.get_client_creds() service_creds = self.get_service_creds() -- 2.35.0 From f23a793d69810a53f4e93682db8c79103f21cf8e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:06:53 +1200 Subject: [PATCH 12/46] tests/krb5: Correctly calculate salt for pre-existing accounts BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 1 + python/samba/tests/krb5/raw_testcase.py | 1 + 2 files changed, 2 insertions(+) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 6c9fe6668d6..6d7cb2e1f24 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1155,6 +1155,7 @@ class KDCBaseTest(RawKerberosTest): kvno = int(res[0]['msDS-KeyVersionNumber'][0]) creds.set_kvno(kvno) + creds.set_workstation(username[:-1]) creds.set_dn(dn) keys = self.get_keys(samdb, dn) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index d0c28fb2002..8d2b84c9d7f 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -869,6 +869,7 @@ class RawKerberosTest(TestCaseInTempDir): allow_missing_password=allow_missing_password, allow_missing_keys=allow_missing_keys) c.set_gensec_features(c.get_gensec_features() | FEATURE_SEAL) + c.set_workstation('') return c def get_rodc_krbtgt_creds(self, -- 2.35.0 From 93e0778f20043fb298ee78a3544f62618afaf592 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:13:54 +1200 Subject: [PATCH 13/46] tests/krb5: Add new definitions for kpasswd BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/rfc4120.asn1 | 6 ++++++ python/samba/tests/krb5/rfc4120_constants.py | 13 +++++++++++++ python/samba/tests/krb5/rfc4120_pyasn1.py | 13 ++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/python/samba/tests/krb5/rfc4120.asn1 b/python/samba/tests/krb5/rfc4120.asn1 index 7b146015548..e2f96829370 100644 --- a/python/samba/tests/krb5/rfc4120.asn1 +++ b/python/samba/tests/krb5/rfc4120.asn1 @@ -568,6 +568,12 @@ PA-FX-FAST-REPLY ::= CHOICE { ... } +ChangePasswdDataMS ::= SEQUENCE { + newpasswd [0] OCTET STRING, + targname [1] PrincipalName OPTIONAL, + targrealm [2] Realm OPTIONAL +} + -- MS-KILE End -- -- diff --git a/python/samba/tests/krb5/rfc4120_constants.py b/python/samba/tests/krb5/rfc4120_constants.py index 28d83407ac5..7d20093f97d 100644 --- a/python/samba/tests/krb5/rfc4120_constants.py +++ b/python/samba/tests/krb5/rfc4120_constants.py @@ -35,11 +35,13 @@ DES3_CBC_SHA1 = int( # Message types KRB_ERROR = int(krb5_asn1.MessageTypeValues('krb-error')) +KRB_AP_REP = int(krb5_asn1.MessageTypeValues('krb-ap-rep')) KRB_AP_REQ = int(krb5_asn1.MessageTypeValues('krb-ap-req')) KRB_AS_REP = int(krb5_asn1.MessageTypeValues('krb-as-rep')) KRB_AS_REQ = int(krb5_asn1.MessageTypeValues('krb-as-req')) KRB_TGS_REP = int(krb5_asn1.MessageTypeValues('krb-tgs-rep')) KRB_TGS_REQ = int(krb5_asn1.MessageTypeValues('krb-tgs-req')) +KRB_PRIV = int(krb5_asn1.MessageTypeValues('krb-priv')) # PAData types PADATA_ENC_TIMESTAMP = int( @@ -90,6 +92,7 @@ KDC_ERR_TGT_REVOKED = 20 KDC_ERR_PREAUTH_FAILED = 24 KDC_ERR_PREAUTH_REQUIRED = 25 KDC_ERR_BAD_INTEGRITY = 31 +KDC_ERR_TKT_EXPIRED = 32 KRB_ERR_TKT_NYV = 33 KDC_ERR_NOT_US = 35 KDC_ERR_BADMATCH = 36 @@ -101,6 +104,16 @@ KDC_ERR_WRONG_REALM = 68 KDC_ERR_CLIENT_NAME_MISMATCH = 75 KDC_ERR_UNKNOWN_CRITICAL_FAST_OPTIONS = 93 +# Kpasswd error codes +KPASSWD_SUCCESS = 0 +KPASSWD_MALFORMED = 1 +KPASSWD_HARDERROR = 2 +KPASSWD_AUTHERROR = 3 +KPASSWD_SOFTERROR = 4 +KPASSWD_ACCESSDENIED = 5 +KPASSWD_BAD_VERSION = 6 +KPASSWD_INITIAL_FLAG_NEEDED = 7 + # Extended error types KERB_AP_ERR_TYPE_SKEW_RECOVERY = int( krb5_asn1.KerbErrorDataTypeValues('kERB-AP-ERR-TYPE-SKEW-RECOVERY')) diff --git a/python/samba/tests/krb5/rfc4120_pyasn1.py b/python/samba/tests/krb5/rfc4120_pyasn1.py index d789ab96b43..ef77ac19ce3 100644 --- a/python/samba/tests/krb5/rfc4120_pyasn1.py +++ b/python/samba/tests/krb5/rfc4120_pyasn1.py @@ -1,5 +1,5 @@ # Auto-generated by asn1ate v.0.6.1.dev0 from rfc4120.asn1 -# (last modified on 2021-06-25 12:10:34.484667) +# (last modified on 2022-05-13 20:03:06.039817) # KerberosV5Spec2 from pyasn1.type import univ, char, namedtype, namedval, tag, constraint, useful @@ -364,6 +364,17 @@ Authenticator.componentType = namedtype.NamedTypes( ) +class ChangePasswdDataMS(univ.Sequence): + pass + + +ChangePasswdDataMS.componentType = namedtype.NamedTypes( + namedtype.NamedType('newpasswd', univ.OctetString().subtype(explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))), + namedtype.OptionalNamedType('targname', PrincipalName().subtype(explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatConstructed, 1))), + namedtype.OptionalNamedType('targrealm', Realm().subtype(explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 2))) +) + + class ChecksumTypeValues(univ.Integer): pass -- 2.35.0 From 7d9ebdb0ddf3b5bdbacaa7921796dde9d9369fa4 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:17:45 +1200 Subject: [PATCH 14/46] tests/krb5: Add methods to create ASN1 kpasswd structures BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/raw_testcase.py | 95 +++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 8d2b84c9d7f..008efcb6c9b 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -56,6 +56,7 @@ from samba.tests.krb5.rfc4120_constants import ( KRB_AS_REP, KRB_AS_REQ, KRB_ERROR, + KRB_PRIV, KRB_TGS_REP, KRB_TGS_REQ, KU_AP_REQ_AUTH, @@ -66,6 +67,7 @@ from samba.tests.krb5.rfc4120_constants import ( KU_FAST_FINISHED, KU_FAST_REP, KU_FAST_REQ_CHKSUM, + KU_KRB_PRIV, KU_NON_KERB_CKSUM_SALT, KU_TGS_REP_ENC_PART_SESSION, KU_TGS_REP_ENC_PART_SUB_KEY, @@ -1825,6 +1827,99 @@ class RawKerberosTest(TestCaseInTempDir): PA_S4U2Self_obj, asn1Spec=krb5_asn1.PA_S4U2Self()) return self.PA_DATA_create(PADATA_FOR_USER, pa_s4u2self) + def ChangePasswdDataMS_create(self, + new_password, + target_name=None, + target_realm=None): + ChangePasswdDataMS_obj = { + 'newpasswd': new_password, + } + if target_name is not None: + ChangePasswdDataMS_obj['targname'] = target_name + if target_realm is not None: + ChangePasswdDataMS_obj['targrealm'] = target_realm + + change_password_data = self.der_encode( + ChangePasswdDataMS_obj, asn1Spec=krb5_asn1.ChangePasswdDataMS()) + + return change_password_data + + def KRB_PRIV_create(self, + subkey, + user_data, + s_address, + timestamp=None, + usec=None, + seq_number=None, + r_address=None): + EncKrbPrivPart_obj = { + 'user-data': user_data, + 's-address': s_address, + } + if timestamp is not None: + EncKrbPrivPart_obj['timestamp'] = timestamp + if usec is not None: + EncKrbPrivPart_obj['usec'] = usec + if seq_number is not None: + EncKrbPrivPart_obj['seq-number'] = seq_number + if r_address is not None: + EncKrbPrivPart_obj['r-address'] = r_address + + enc_krb_priv_part = self.der_encode( + EncKrbPrivPart_obj, asn1Spec=krb5_asn1.EncKrbPrivPart()) + + enc_data = self.EncryptedData_create(subkey, + KU_KRB_PRIV, + enc_krb_priv_part) + + KRB_PRIV_obj = { + 'pvno': 5, + 'msg-type': KRB_PRIV, + 'enc-part': enc_data, + } + + krb_priv = self.der_encode( + KRB_PRIV_obj, asn1Spec=krb5_asn1.KRB_PRIV()) + + return krb_priv + + def kpasswd_create(self, + subkey, + user_data, + version, + seq_number, + ap_req, + local_address, + remote_address): + self.assertIsNotNone(self.s, 'call self.connect() first') + + timestamp, usec = self.get_KerberosTimeWithUsec() + + krb_priv = self.KRB_PRIV_create(subkey, + user_data, + s_address=local_address, + timestamp=timestamp, + usec=usec, + seq_number=seq_number, + r_address=remote_address) + + size = 6 + len(ap_req) + len(krb_priv) + self.assertLess(size, 0x10000) + + msg = bytearray() + msg.append(size >> 8) + msg.append(size & 0xff) + msg.append(version >> 8) + msg.append(version & 0xff) + msg.append(len(ap_req) >> 8) + msg.append(len(ap_req) & 0xff) + # Note: for sets, there could be a little-endian four-byte length here. + + msg.extend(ap_req) + msg.extend(krb_priv) + + return msg + def _generic_kdc_exchange(self, kdc_exchange_dict, # required cname=None, # optional -- 2.35.0 From a538e2210ddec97d18f33aec4c65a95d7b801c1d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:21:37 +1200 Subject: [PATCH 15/46] tests/krb5: Add 'port' parameter to connect() This allows us to use the kpasswd port, 464. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/raw_testcase.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 008efcb6c9b..604a2269a57 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -673,10 +673,11 @@ class RawKerberosTest(TestCaseInTempDir): if self.do_hexdump: sys.stderr.write("disconnect[%s]\n" % reason) - def _connect_tcp(self, host): - tcp_port = 88 + def _connect_tcp(self, host, port=None): + if port is None: + port = 88 try: - self.a = socket.getaddrinfo(host, tcp_port, socket.AF_UNSPEC, + self.a = socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM, socket.SOL_TCP, 0) self.s = socket.socket(self.a[0][0], self.a[0][1], self.a[0][2]) @@ -689,9 +690,9 @@ class RawKerberosTest(TestCaseInTempDir): self.s.close() raise - def connect(self, host): + def connect(self, host, port=None): self.assertNotConnected() - self._connect_tcp(host) + self._connect_tcp(host, port) if self.do_hexdump: sys.stderr.write("connected[%s]\n" % host) -- 2.35.0 From 8efdc0bdc71df7d0a0ebce09124f59e5bafb458b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:20:28 +1200 Subject: [PATCH 16/46] tests/krb5: Add methods to send and receive generic messages This allows us to send and receive kpasswd messages, while avoiding the existing logic for encoding and decoding other Kerberos message types. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/raw_testcase.py | 44 +++++++++++++++---------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 604a2269a57..a6faacf03e4 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -955,24 +955,28 @@ class RawKerberosTest(TestCaseInTempDir): return blob def send_pdu(self, req, asn1_print=None, hexdump=None): + k5_pdu = self.der_encode( + req, native_decode=False, asn1_print=asn1_print, hexdump=False) + self.send_msg(k5_pdu, hexdump=hexdump) + + def send_msg(self, msg, hexdump=None): + header = struct.pack('>I', len(msg)) + req_pdu = header + req_pdu += msg + self.hex_dump("send_msg", header, hexdump=hexdump) + self.hex_dump("send_msg", msg, hexdump=hexdump) + try: - k5_pdu = self.der_encode( - req, native_decode=False, asn1_print=asn1_print, hexdump=False) - header = struct.pack('>I', len(k5_pdu)) - req_pdu = header - req_pdu += k5_pdu - self.hex_dump("send_pdu", header, hexdump=hexdump) - self.hex_dump("send_pdu", k5_pdu, hexdump=hexdump) while True: sent = self.s.send(req_pdu, 0) if sent == len(req_pdu): - break + return req_pdu = req_pdu[sent:] except socket.error as e: - self._disconnect("send_pdu: %s" % e) + self._disconnect("send_msg: %s" % e) raise except IOError as e: - self._disconnect("send_pdu: %s" % e) + self._disconnect("send_msg: %s" % e) raise def recv_raw(self, num_recv=0xffff, hexdump=None, timeout=None): @@ -998,16 +1002,14 @@ class RawKerberosTest(TestCaseInTempDir): return rep_pdu def recv_pdu_raw(self, asn1_print=None, hexdump=None, timeout=None): - rep_pdu = None - rep = None raw_pdu = self.recv_raw( num_recv=4, hexdump=hexdump, timeout=timeout) if raw_pdu is None: - return (None, None) + return None header = struct.unpack(">I", raw_pdu[0:4]) k5_len = header[0] if k5_len == 0: - return (None, "") + return "" missing = k5_len rep_pdu = b'' while missing > 0: @@ -1016,6 +1018,14 @@ class RawKerberosTest(TestCaseInTempDir): self.assertGreaterEqual(len(raw_pdu), 1) rep_pdu += raw_pdu missing = k5_len - len(rep_pdu) + return rep_pdu + + def recv_reply(self, asn1_print=None, hexdump=None, timeout=None): + rep_pdu = self.recv_pdu_raw(asn1_print=asn1_print, + hexdump=hexdump, + timeout=timeout) + if not rep_pdu: + return None, rep_pdu k5_raw = self.der_decode( rep_pdu, asn1Spec=None, @@ -1037,9 +1047,9 @@ class RawKerberosTest(TestCaseInTempDir): return (rep, rep_pdu) def recv_pdu(self, asn1_print=None, hexdump=None, timeout=None): - (rep, rep_pdu) = self.recv_pdu_raw(asn1_print=asn1_print, - hexdump=hexdump, - timeout=timeout) + (rep, rep_pdu) = self.recv_reply(asn1_print=asn1_print, + hexdump=hexdump, + timeout=timeout) return rep def assertIsConnected(self): -- 2.35.0 From 7dd743f55a32739bf3a5de9e7e5fd1241659549d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:26:56 +1200 Subject: [PATCH 17/46] tests/krb5: Fix enum typo Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 6d7cb2e1f24..8e3460fb246 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -276,9 +276,9 @@ class KDCBaseTest(RawKerberosTest): which is used by tearDownClass to clean up the created accounts. ''' if ou is None: - if account_type is account_type.COMPUTER: + if account_type is self.AccountType.COMPUTER: guid = DS_GUID_COMPUTERS_CONTAINER - elif account_type is account_type.SERVER: + elif account_type is self.AccountType.SERVER: guid = DS_GUID_DOMAIN_CONTROLLERS_CONTAINER else: guid = DS_GUID_USERS_CONTAINER -- 2.35.0 From fe629c03540e167361a3d6a2907578099d3583bc Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:30:12 +1200 Subject: [PATCH 18/46] tests/krb5: Add option for creating accounts with expired passwords Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 8e3460fb246..248fe75e60d 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -270,7 +270,8 @@ class KDCBaseTest(RawKerberosTest): def create_account(self, samdb, name, account_type=AccountType.USER, spn=None, upn=None, additional_details=None, - ou=None, account_control=0, add_dollar=True): + ou=None, account_control=0, add_dollar=True, + expired_password=False): '''Create an account for testing. The dn of the created account is added to self.accounts, which is used by tearDownClass to clean up the created accounts. @@ -324,6 +325,8 @@ class KDCBaseTest(RawKerberosTest): details["servicePrincipalName"] = spn if upn is not None: details["userPrincipalName"] = upn + if expired_password: + details["pwdLastSet"] = "0" if additional_details is not None: details.update(additional_details) # Save the account name so it can be deleted in tearDownClass @@ -730,6 +733,7 @@ class KDCBaseTest(RawKerberosTest): 'revealed_to_rodc': False, 'revealed_to_mock_rodc': False, 'no_auth_data_required': False, + 'expired_password': False, 'supported_enctypes': None, 'not_delegated': False, 'delegation_to_spn': None, @@ -776,6 +780,7 @@ class KDCBaseTest(RawKerberosTest): revealed_to_rodc, revealed_to_mock_rodc, no_auth_data_required, + expired_password, supported_enctypes, not_delegated, delegation_to_spn, @@ -841,7 +846,8 @@ class KDCBaseTest(RawKerberosTest): spn=spn, additional_details=details, account_control=user_account_control, - add_dollar=add_dollar) + add_dollar=add_dollar, + expired_password=expired_password) keys = self.get_keys(samdb, dn) self.creds_set_keys(creds, keys) -- 2.35.0 From 836fff52f085b192315a08704c677b085d295550 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:34:59 +1200 Subject: [PATCH 19/46] tests/krb5: Allow requesting a TGT to a different sname and realm BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index 248fe75e60d..d3445ab9794 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1451,11 +1451,13 @@ class KDCBaseTest(RawKerberosTest): expected_flags=None, unexpected_flags=None, pac_request=True, expect_pac=True, fresh=False): user_name = tgt.cname['name-string'][0] + ticket_sname = tgt.sname if target_name is None: target_name = target_creds.get_username()[:-1] cache_key = (user_name, target_name, service, to_rodc, kdc_options, pac_request, str(expected_flags), str(unexpected_flags), till, rc4_support, + str(ticket_sname), expect_pac) if not fresh: @@ -1526,6 +1528,7 @@ class KDCBaseTest(RawKerberosTest): expected_account_name=None, expected_upn_name=None, expected_cname=None, expected_sid=None, + sname=None, realm=None, pac_request=True, expect_pac=True, expect_pac_attrs=None, expect_pac_attrs_pac_request=None, expect_requester_sid=None, @@ -1540,6 +1543,7 @@ class KDCBaseTest(RawKerberosTest): client_name_type, str(expected_flags), str(unexpected_flags), expected_account_name, expected_upn_name, expected_sid, + str(sname), str(realm), str(expected_cname), rc4_support, expect_pac, expect_pac_attrs, @@ -1551,15 +1555,21 @@ class KDCBaseTest(RawKerberosTest): if tgt is not None: return tgt - realm = creds.get_realm() + if realm is None: + realm = creds.get_realm() salt = creds.get_salt() etype = (AES256_CTS_HMAC_SHA1_96, ARCFOUR_HMAC_MD5) cname = self.PrincipalName_create(name_type=client_name_type, names=user_name.split('/')) - sname = self.PrincipalName_create(name_type=NT_SRV_INST, - names=['krbtgt', realm]) + if sname is None: + sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + expected_sname = self.PrincipalName_create( + name_type=NT_SRV_INST, names=['krbtgt', realm.upper()]) + else: + expected_sname = sname if expected_cname is None: expected_cname = cname @@ -1629,9 +1639,6 @@ class KDCBaseTest(RawKerberosTest): expected_realm = realm.upper() - expected_sname = self.PrincipalName_create( - name_type=NT_SRV_INST, names=['krbtgt', realm.upper()]) - rep, kdc_exchange_dict = self._test_as_exchange( cname=cname, realm=realm, -- 2.35.0 From 5b4612b7a92e5180418ce16249fb83c66e475c70 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:57:57 +1200 Subject: [PATCH 20/46] tests/krb5: Add kpasswd_exchange() method Now we can test the kpasswd service from Python. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/raw_testcase.py | 258 ++++++++++++++++++++++-- 1 file changed, 242 insertions(+), 16 deletions(-) diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index a6faacf03e4..0deb6f0d76d 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -26,6 +26,8 @@ import binascii import itertools import collections +from enum import Enum + from pyasn1.codec.der.decoder import decode as pyasn1_der_decode from pyasn1.codec.der.encoder import encode as pyasn1_der_encode from pyasn1.codec.native.decoder import decode as pyasn1_native_decode @@ -52,6 +54,7 @@ from samba.tests.krb5.rfc4120_constants import ( KDC_ERR_SKEW, KDC_ERR_UNKNOWN_CRITICAL_FAST_OPTIONS, KERB_ERR_TYPE_EXTENDED, + KRB_AP_REP, KRB_AP_REQ, KRB_AS_REP, KRB_AS_REQ, @@ -61,6 +64,7 @@ from samba.tests.krb5.rfc4120_constants import ( KRB_TGS_REQ, KU_AP_REQ_AUTH, KU_AS_REP_ENC_PART, + KU_AP_REQ_ENC_PART, KU_AS_REQ, KU_ENC_CHALLENGE_KDC, KU_FAST_ENC, @@ -76,6 +80,7 @@ from samba.tests.krb5.rfc4120_constants import ( KU_TGS_REQ_AUTH_DAT_SESSION, KU_TGS_REQ_AUTH_DAT_SUBKEY, KU_TICKET, + NT_PRINCIPAL, NT_SRV_INST, NT_WELLKNOWN, PADATA_ENCRYPTED_CHALLENGE, @@ -525,6 +530,10 @@ class KerberosTicketCreds: class RawKerberosTest(TestCaseInTempDir): """A raw Kerberos Test case.""" + class KpasswdMode(Enum): + SET = object() + CHANGE = object() + pac_checksum_types = {krb5pac.PAC_TYPE_SRV_CHECKSUM, krb5pac.PAC_TYPE_KDC_CHECKSUM, krb5pac.PAC_TYPE_TICKET_CHECKSUM} @@ -1840,13 +1849,13 @@ class RawKerberosTest(TestCaseInTempDir): def ChangePasswdDataMS_create(self, new_password, - target_name=None, + target_princ=None, target_realm=None): ChangePasswdDataMS_obj = { 'newpasswd': new_password, } - if target_name is not None: - ChangePasswdDataMS_obj['targname'] = target_name + if target_princ is not None: + ChangePasswdDataMS_obj['targname'] = target_princ if target_realm is not None: ChangePasswdDataMS_obj['targrealm'] = target_realm @@ -1931,6 +1940,214 @@ class RawKerberosTest(TestCaseInTempDir): return msg + def get_enc_part(self, obj, key, usage): + self.assertElementEqual(obj, 'pvno', 5) + + enc_part = obj['enc-part'] + self.assertElementEqual(enc_part, 'etype', key.etype) + self.assertElementKVNO(enc_part, 'kvno', key.kvno) + + enc_part = key.decrypt(usage, enc_part['cipher']) + + return enc_part + + def kpasswd_exchange(self, + ticket, + new_password, + expected_code, + expected_msg, + mode, + target_princ=None, + target_realm=None, + ap_options=None, + send_seq_number=True): + if mode is self.KpasswdMode.SET: + version = 0xff80 + user_data = self.ChangePasswdDataMS_create(new_password, + target_princ, + target_realm) + elif mode is self.KpasswdMode.CHANGE: + self.assertIsNone(target_princ, + 'target_princ only valid for pw set') + self.assertIsNone(target_realm, + 'target_realm only valid for pw set') + + version = 1 + user_data = new_password.encode('utf-8') + else: + self.fail(f'invalid mode {mode}') + + subkey = self.RandomKey(kcrypto.Enctype.AES256) + + if ap_options is None: + ap_options = '0' + ap_options = str(krb5_asn1.APOptions(ap_options)) + + kdc_exchange_dict = { + 'tgt': ticket, + 'authenticator_subkey': subkey, + 'auth_data': None, + 'ap_options': ap_options, + } + + if send_seq_number: + seq_number = random.randint(0, 0xfffffffe) + else: + seq_number = None + + ap_req = self.generate_ap_req(kdc_exchange_dict, + None, + req_body=None, + armor=False, + usage=KU_AP_REQ_AUTH, + seq_number=seq_number) + + self.connect(self.host, port=464) + self.assertIsNotNone(self.s) + + family = self.s.family + + if family == socket.AF_INET: + addr_type = 2 # IPv4 + elif family == socket.AF_INET6: + addr_type = 24 # IPv6 + else: + self.fail(f'unknown family {family}') + + def create_address(ip): + return { + 'addr-type': addr_type, + 'address': socket.inet_pton(family, ip), + } + + remote_ip = self.s.getpeername()[0] + local_ip = self.s.getsockname()[0] + + remote_address = create_address(remote_ip) + local_address = create_address(local_ip) + + msg = self.kpasswd_create(subkey, + user_data, + version, + seq_number, + ap_req, + local_address, + remote_address) + + self.send_msg(msg) + rep_pdu = self.recv_pdu_raw() + + self._disconnect('transaction done') + + self.assertIsNotNone(rep_pdu) + + header = rep_pdu[:6] + reply = rep_pdu[6:] + + reply_len = (header[0] << 8) | header[1] + reply_version = (header[2] << 8) | header[3] + ap_rep_len = (header[4] << 8) | header[5] + + self.assertEqual(reply_len, len(rep_pdu)) + self.assertEqual(1, reply_version) # KRB5_KPASSWD_VERS_CHANGEPW + self.assertLess(ap_rep_len, reply_len) + + self.assertNotEqual(0x7e, rep_pdu[1]) + self.assertNotEqual(0x5e, rep_pdu[1]) + + if ap_rep_len: + # We received an AP-REQ and KRB-PRIV as a response. This may or may + # not indicate an error, depending on the status code. + ap_rep = reply[:ap_rep_len] + krb_priv = reply[ap_rep_len:] + + key = ticket.session_key + + ap_rep = self.der_decode(ap_rep, asn1Spec=krb5_asn1.AP_REP()) + self.assertElementEqual(ap_rep, 'msg-type', KRB_AP_REP) + enc_part = self.get_enc_part(ap_rep, key, KU_AP_REQ_ENC_PART) + enc_part = self.der_decode( + enc_part, asn1Spec=krb5_asn1.EncAPRepPart()) + + self.assertElementPresent(enc_part, 'ctime') + self.assertElementPresent(enc_part, 'cusec') + # self.assertElementMissing(enc_part, 'subkey') # TODO + # self.assertElementPresent(enc_part, 'seq-number') # TODO + + krb_priv = self.der_decode(krb_priv, asn1Spec=krb5_asn1.KRB_PRIV()) + self.assertElementEqual(krb_priv, 'msg-type', KRB_PRIV) + priv_enc_part = self.get_enc_part(krb_priv, subkey, KU_KRB_PRIV) + priv_enc_part = self.der_decode( + priv_enc_part, asn1Spec=krb5_asn1.EncKrbPrivPart()) + + self.assertElementMissing(priv_enc_part, 'timestamp') + self.assertElementMissing(priv_enc_part, 'usec') + # self.assertElementPresent(priv_enc_part, 'seq-number') # TODO + self.assertElementEqual(priv_enc_part, 's-address', remote_address) + # self.assertElementMissing(priv_enc_part, 'r-address') # TODO + + result_data = priv_enc_part['user-data'] + else: + # We received a KRB-ERROR as a response, indicating an error. + krb_error = self.der_decode(reply, asn1Spec=krb5_asn1.KRB_ERROR()) + + sname = self.PrincipalName_create( + name_type=NT_PRINCIPAL, + names=['kadmin', 'changepw']) + realm = self.get_krbtgt_creds().get_realm().upper() + + self.assertElementEqual(krb_error, 'pvno', 5) + self.assertElementEqual(krb_error, 'msg-type', KRB_ERROR) + self.assertElementMissing(krb_error, 'ctime') + self.assertElementMissing(krb_error, 'usec') + self.assertElementPresent(krb_error, 'stime') + self.assertElementPresent(krb_error, 'susec') + + error_code = krb_error['error-code'] + if isinstance(expected_code, int): + self.assertEqual(error_code, expected_code) + else: + self.assertIn(error_code, expected_code) + + self.assertElementMissing(krb_error, 'crealm') + self.assertElementMissing(krb_error, 'cname') + self.assertElementEqual(krb_error, 'realm', realm.encode('utf-8')) + self.assertElementEqualPrincipal(krb_error, 'sname', sname) + self.assertElementMissing(krb_error, 'e-text') + + result_data = krb_error['e-data'] + + status = result_data[:2] + message = result_data[2:] + + status_code = (status[0] << 8) | status[1] + if isinstance(expected_code, int): + self.assertEqual(status_code, expected_code) + else: + self.assertIn(status_code, expected_code) + + if not message: + self.assertEqual(0, status_code, + 'got an error result, but no message') + return + + # Check the first character of the message. + if message[0]: + if isinstance(expected_msg, bytes): + self.assertEqual(message, expected_msg) + else: + self.assertIn(message, expected_msg) + else: + # We got AD password policy information. + self.assertEqual(30, len(message)) + + (empty_bytes, + min_length, + history_length, + properties, + expire_time, + min_age) = struct.unpack('>HIIIQQ', message) + def _generic_kdc_exchange(self, kdc_exchange_dict, # required cname=None, # optional @@ -2041,7 +2258,7 @@ class RawKerberosTest(TestCaseInTempDir): self.assertIsNotNone(generate_fast_fn) fast_ap_req = generate_fast_armor_fn(kdc_exchange_dict, callback_dict, - req_body, + None, armor=True) fast_armor_type = kdc_exchange_dict['fast_armor_type'] @@ -3438,31 +3655,39 @@ class RawKerberosTest(TestCaseInTempDir): kdc_exchange_dict, _callback_dict, req_body, - armor): + armor, + usage=None, + seq_number=None): + req_body_checksum = None + if armor: + self.assertIsNone(req_body) + tgt = kdc_exchange_dict['armor_tgt'] authenticator_subkey = kdc_exchange_dict['armor_subkey'] - - req_body_checksum = None else: tgt = kdc_exchange_dict['tgt'] authenticator_subkey = kdc_exchange_dict['authenticator_subkey'] - body_checksum_type = kdc_exchange_dict['body_checksum_type'] - req_body_blob = self.der_encode(req_body, - asn1Spec=krb5_asn1.KDC_REQ_BODY()) + if req_body is not None: + body_checksum_type = kdc_exchange_dict['body_checksum_type'] - req_body_checksum = self.Checksum_create(tgt.session_key, - KU_TGS_REQ_AUTH_CKSUM, - req_body_blob, - ctype=body_checksum_type) + req_body_blob = self.der_encode( + req_body, asn1Spec=krb5_asn1.KDC_REQ_BODY()) + + req_body_checksum = self.Checksum_create( + tgt.session_key, + KU_TGS_REQ_AUTH_CKSUM, + req_body_blob, + ctype=body_checksum_type) auth_data = kdc_exchange_dict['auth_data'] subkey_obj = None if authenticator_subkey is not None: subkey_obj = authenticator_subkey.export_obj() - seq_number = random.randint(0, 0xfffffffe) + if seq_number is None: + seq_number = random.randint(0, 0xfffffffe) (ctime, cusec) = self.get_KerberosTimeWithUsec() authenticator_obj = self.Authenticator_create( crealm=tgt.crealm, @@ -3477,7 +3702,8 @@ class RawKerberosTest(TestCaseInTempDir): authenticator_obj, asn1Spec=krb5_asn1.Authenticator()) - usage = KU_AP_REQ_AUTH if armor else KU_TGS_REQ_AUTH + if usage is None: + usage = KU_AP_REQ_AUTH if armor else KU_TGS_REQ_AUTH authenticator = self.EncryptedData_create(tgt.session_key, usage, authenticator_blob) -- 2.35.0 From bbc25e49fcd0ae7c4c44e398b246fdc30cc3fbd6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:35:03 +1200 Subject: [PATCH 21/46] selftest: Specify Administrator kvno for Python krb5 tests BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 Signed-off-by: Joseph Sutton --- source4/selftest/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index a01e188bcd1..cac84543f5e 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1002,6 +1002,7 @@ krb5_environ = { 'SERVICE_USERNAME': '$SERVER', 'ADMIN_USERNAME': '$DC_USERNAME', 'ADMIN_PASSWORD': '$DC_PASSWORD', + 'ADMIN_KVNO': '1', 'FOR_USER': '$DC_USERNAME', 'STRICT_CHECKING':'0', 'FAST_SUPPORT': have_fast_support, -- 2.35.0 From dc0a291ab6c453339e55521b152f99fd54c1dd5b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:59:16 +1200 Subject: [PATCH 22/46] tests/krb5: Add tests for kpasswd service BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- python/samba/tests/krb5/kdc_base_test.py | 4 +- python/samba/tests/krb5/kpasswd_tests.py | 931 +++++++++++++++++++++++ python/samba/tests/krb5/raw_testcase.py | 8 + python/samba/tests/usage.py | 1 + source4/selftest/tests.py | 4 + 5 files changed, 947 insertions(+), 1 deletion(-) create mode 100755 python/samba/tests/krb5/kpasswd_tests.py diff --git a/python/samba/tests/krb5/kdc_base_test.py b/python/samba/tests/krb5/kdc_base_test.py index d3445ab9794..aaeca0b04e5 100644 --- a/python/samba/tests/krb5/kdc_base_test.py +++ b/python/samba/tests/krb5/kdc_base_test.py @@ -1719,7 +1719,9 @@ class KDCBaseTest(RawKerberosTest): authenticator_subkey = self.RandomKey(kcrypto.Enctype.AES256) if expect_error: - expected_error_mode = KDC_ERR_TGT_REVOKED + expected_error_mode = expect_error + if expected_error_mode is True: + expected_error_mode = KDC_ERR_TGT_REVOKED check_error_fn = self.generic_check_kdc_error check_rep_fn = None else: diff --git a/python/samba/tests/krb5/kpasswd_tests.py b/python/samba/tests/krb5/kpasswd_tests.py new file mode 100755 index 00000000000..a18e2e743c9 --- /dev/null +++ b/python/samba/tests/krb5/kpasswd_tests.py @@ -0,0 +1,931 @@ +#!/usr/bin/env python3 +# Unix SMB/CIFS implementation. +# Copyright (C) Stefan Metzmacher 2020 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import sys + +from functools import partial + +from samba import generate_random_password +from samba.dcerpc import krb5pac, security +from samba.sd_utils import SDUtils + +from samba.tests.krb5.kdc_base_test import KDCBaseTest +from samba.tests.krb5.rfc4120_constants import ( + KDC_ERR_TKT_EXPIRED, + KPASSWD_ACCESSDENIED, + KPASSWD_HARDERROR, + KPASSWD_INITIAL_FLAG_NEEDED, + KPASSWD_MALFORMED, + KPASSWD_SOFTERROR, + KPASSWD_SUCCESS, + NT_PRINCIPAL, + NT_SRV_INST, +) + +sys.path.insert(0, 'bin/python') +os.environ['PYTHONUNBUFFERED'] = '1' + +global_asn1_print = False +global_hexdump = False + + +class KpasswdTests(KDCBaseTest): + + def setUp(self): + super().setUp() + self.do_asn1_print = global_asn1_print + self.do_hexdump = global_hexdump + + samdb = self.get_samdb() + + # Get the old 'dSHeuristics' if it was set + dsheuristics = samdb.get_dsheuristics() + + # Reset the 'dSHeuristics' as they were before + self.addCleanup(samdb.set_dsheuristics, dsheuristics) + + # Set the 'dSHeuristics' to activate the correct 'userPassword' + # behaviour + samdb.set_dsheuristics('000000001') + + # Get the old 'minPwdAge' + minPwdAge = samdb.get_minPwdAge() + + # Reset the 'minPwdAge' as it was before + self.addCleanup(samdb.set_minPwdAge, minPwdAge) + + # Set it temporarily to '0' + samdb.set_minPwdAge('0') + + def _get_creds(self, expired=False): + opts = { + 'expired_password': expired + } + + # Create the account. + creds = self.get_cached_creds(account_type=self.AccountType.USER, + opts=opts, + use_cache=False) + + return creds + + def issued_by_rodc(self, ticket): + krbtgt_creds = self.get_mock_rodc_krbtgt_creds() + + krbtgt_key = self.TicketDecryptionKey_from_creds(krbtgt_creds) + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: krbtgt_key, + } + + return self.modified_ticket( + ticket, + new_ticket_key=krbtgt_key, + checksum_keys=checksum_keys) + + def get_kpasswd_sname(self): + return self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=['kadmin', 'changepw']) + + def modify_lifetime(self, enc_part, lifetime): + # Note: Windows never sets nor reads the starttime field. + authtime = enc_part['authtime'] + starttime = enc_part.get('starttime', authtime) + + epoch = self.get_EpochFromKerberosTime(starttime) + endtime = self.get_KerberosTime(epoch=epoch, + offset=lifetime) + enc_part['endtime'] = endtime + + return enc_part + + def get_ticket_lifetime(self, ticket): + enc_part = ticket.ticket_private + + authtime = enc_part['authtime'] + starttime = enc_part.get('starttime', authtime) + endtime = enc_part['endtime'] + + starttime = self.get_EpochFromKerberosTime(starttime) + endtime = self.get_EpochFromKerberosTime(endtime) + + return endtime - starttime + + def add_requester_sid(self, pac, sid): + pac_buffers = pac.buffers + + buffer_types = [pac_buffer.type for pac_buffer in pac_buffers] + self.assertNotIn(krb5pac.PAC_TYPE_REQUESTER_SID, buffer_types) + + requester_sid = krb5pac.PAC_REQUESTER_SID() + requester_sid.sid = security.dom_sid(sid) + + requester_sid_buffer = krb5pac.PAC_BUFFER() + requester_sid_buffer.type = krb5pac.PAC_TYPE_REQUESTER_SID + requester_sid_buffer.info = requester_sid + + pac_buffers.append(requester_sid_buffer) + + pac.buffers = pac_buffers + pac.num_buffers += 1 + + return pac + + # Test setting a password with kpasswd. + def test_kpasswd_set(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Test the newly set password. + creds.update_password(new_password) + self.get_tgt(creds, fresh=True) + + # Test changing a password with kpasswd. + def test_kpasswd_change(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test the newly set password. + creds.update_password(new_password) + self.get_tgt(creds, fresh=True) + + # Test kpasswd without setting the canonicalize option. + def test_kpasswd_no_canonicalize(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + realm = creds.get_realm().capitalize() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + realm=realm, + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + realm=realm, + kdc_options='0') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd with the canonicalize option set. + def test_kpasswd_canonicalize(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='canonicalize') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + realm=creds.get_realm().capitalize(), + kdc_options='canonicalize') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd with the canonicalize option set and a non-canonical realm. + def test_kpasswd_canonicalize_realm_case(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + realm=creds.get_realm().capitalize(), + kdc_options='canonicalize') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get an initial ticket to kpasswd. We set the canonicalize flag here. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + realm=creds.get_realm().capitalize(), + kdc_options='canonicalize') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd rejects a password that does not meet complexity + # requirements. + def test_kpasswd_too_weak(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SOFTERROR + expected_msg = b'Password does not meet complexity requirements' + + # Set the password. + new_password = 'password' + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd rejects an empty new password. + def test_kpasswd_empty(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SOFTERROR, KPASSWD_HARDERROR + expected_msg = (b'Password too short, password must be at least 7 ' + b'characters long.', + b'String conversion failed!') + + # Set the password. + new_password = '' + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'String conversion failed!' + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test kpasswd rejects a request that does not include a random sequence + # number. + def test_kpasswd_no_seq_number(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_unwrap failed - NT_STATUS_ACCESS_DENIED\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + send_seq_number=False) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE, + send_seq_number=False) + + # Test kpasswd rejects a ticket issued by an RODC. + def test_kpasswd_from_rodc(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Have the ticket be issued by the RODC. + ticket = self.issued_by_rodc(ticket) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test setting a password, specifying the principal of the target user. + def test_kpasswd_set_target_princ_only(self): + # Create an account for testing. + creds = self._get_creds() + username = creds.get_username() + + cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_MALFORMED + expected_msg = (b'Realm and principal must be both present, or ' + b'neither present') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_princ=cname) + + # Test that kpasswd rejects a password set specifying only the realm of the + # target user. + def test_kpasswd_set_target_realm_only(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_MALFORMED, KPASSWD_ACCESSDENIED + expected_msg = (b'Realm and principal must be both present, or ' + b'neither present', + b'No such user when changing password') + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_realm=creds.get_realm()) + + # Show that a user cannot sett a password, specifying both principal and + # realm of the target user, without having control access. + def test_kpasswd_set_target_princ_and_realm_no_access(self): + # Create an account for testing. + creds = self._get_creds() + username = creds.get_username() + + cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_ACCESSDENIED + expected_msg = b'Not permitted to change password' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_princ=cname, + target_realm=creds.get_realm()) + + # Test setting a password, specifying both principal and realm of the + # target user, whem the user has control access on their account. + def test_kpasswd_set_target_princ_and_realm_access(self): + # Create an account for testing. + creds = self._get_creds() + username = creds.get_username() + + cname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=username.split('/')) + + samdb = self.get_samdb() + sd_utils = SDUtils(samdb) + + user_dn = creds.get_dn() + user_sid = self.get_objectSid(samdb, user_dn) + + # Give the user control access on their account. + ace = f'(A;;CR;;;{user_sid})' + sd_utils.dacl_add_ace(user_dn, ace) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET, + target_princ=cname, + target_realm=creds.get_realm()) + + # Test setting a password when the existing password has expired. + def test_kpasswd_set_expired_password(self): + # Create an account for testing, with an expired password. + creds = self._get_creds(expired=True) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Test changing a password when the existing password has expired. + def test_kpasswd_change_expired_password(self): + # Create an account for testing, with an expired password. + creds = self._get_creds(expired=True) + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Check the lifetime of a kpasswd ticket is not more than two minutes. + def test_kpasswd_ticket_lifetime(self): + # Create an account for testing. + creds = self._get_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Check the lifetime of the ticket is equal to two minutes. + lifetime = self.get_ticket_lifetime(ticket) + self.assertEqual(2 * 60, lifetime) + + # Ensure we cannot perform a TGS-REQ with a kpasswd ticket. + def test_kpasswd_ticket_tgs(self): + creds = self.get_client_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Change the sname of the ticket to match that of a TGT. + realm = creds.get_realm() + krbtgt_sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + ticket.set_sname(krbtgt_sname) + + # Try to use that ticket to get a service ticket. + service_creds = self.get_service_creds() + + # This fails due to missing REQUESTER_SID buffer. + self._make_tgs_request(creds, service_creds, ticket, + expect_error=True) + + # Ensure we cannot perform a TGS-REQ with a kpasswd ticket containing a + # requester SID and having a lifetime of two minutes. + def test_kpasswd_ticket_requester_sid_tgs(self): + creds = self.get_client_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Change the sname of the ticket to match that of a TGT. + realm = creds.get_realm() + krbtgt_sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + ticket.set_sname(krbtgt_sname) + + # Get the krbtgt key. + krbtgt_creds = self.get_krbtgt_creds() + + krbtgt_key = self.TicketDecryptionKey_from_creds(krbtgt_creds) + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: krbtgt_key, + } + + # Get the user's SID. + samdb = self.get_samdb() + + user_dn = creds.get_dn() + user_sid = self.get_objectSid(samdb, user_dn) + + # Modify the ticket to add a requester SID and set the lifetime to two + # minutes. + modify_fn = partial(self.modify_lifetime, lifetime=2 * 60) + modify_pac_fn = partial(self.add_requester_sid, sid=user_sid) + ticket = self.modified_ticket(ticket, + new_ticket_key=krbtgt_key, + modify_fn=modify_fn, + modify_pac_fn=modify_pac_fn, + checksum_keys=checksum_keys) + + # Try to use that ticket to get a service ticket. + service_creds = self.get_service_creds() + + # This fails due to the lifetime being too short. + self._make_tgs_request(creds, service_creds, ticket, + expect_error=KDC_ERR_TKT_EXPIRED) + + # Show we can perform a TGS-REQ with a kpasswd ticket containing a + # requester SID if the lifetime exceeds two minutes. + def test_kpasswd_ticket_requester_sid_lifetime_tgs(self): + creds = self.get_client_creds() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=self.get_kpasswd_sname(), + kdc_options='0') + + # Change the sname of the ticket to match that of a TGT. + realm = creds.get_realm() + krbtgt_sname = self.PrincipalName_create(name_type=NT_SRV_INST, + names=['krbtgt', realm]) + ticket.set_sname(krbtgt_sname) + + # Get the krbtgt key. + krbtgt_creds = self.get_krbtgt_creds() + + krbtgt_key = self.TicketDecryptionKey_from_creds(krbtgt_creds) + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: krbtgt_key, + } + + # Get the user's SID. + samdb = self.get_samdb() + + user_dn = creds.get_dn() + user_sid = self.get_objectSid(samdb, user_dn) + + # Modify the ticket to add a requester SID and set the lifetime to two + # minutes and one second. + modify_fn = partial(self.modify_lifetime, lifetime=2 * 60 + 1) + modify_pac_fn = partial(self.add_requester_sid, sid=user_sid) + ticket = self.modified_ticket(ticket, + new_ticket_key=krbtgt_key, + modify_fn=modify_fn, + modify_pac_fn=modify_pac_fn, + checksum_keys=checksum_keys) + + # Try to use that ticket to get a service ticket. + service_creds = self.get_service_creds() + + # This succeeds. + self._make_tgs_request(creds, service_creds, ticket, + expect_error=False) + + # Show that we are not prevented from providing a TGT to kpasswd to change + # the password. + def test_kpasswd_tgt(self): + # Create an account for testing, and get a TGT. + creds = self._get_creds() + tgt = self.get_tgt(creds) + + # Change the sname of the ticket to match that of kadmin/changepw. + tgt.set_sname(self.get_kpasswd_sname()) + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(tgt, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + + # Get a new TGT. + tgt = self.get_tgt(creds, fresh=True) + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(tgt, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test that kpasswd rejects requests with a service ticket. + def test_kpasswd_non_initial(self): + # Create an account for testing, and get a TGT. + creds = self._get_creds() + tgt = self.get_tgt(creds) + + # Get a non-initial ticket to kpasswd. + krbtgt_creds = self.get_krbtgt_creds() + ticket = self.get_service_ticket(tgt, + krbtgt_creds, + service='kadmin', + target_name='changepw', + kdc_options='0') + + expected_code = KPASSWD_INITIAL_FLAG_NEEDED + expected_msg = b'Expected an initial ticket\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Show that kpasswd accepts requests with a service ticket modified to set + # the 'initial' flag. + def test_kpasswd_initial(self): + # Create an account for testing, and get a TGT. + creds = self._get_creds() + + krbtgt_creds = self.get_krbtgt_creds() + + # Get a service ticket, and modify it to set the 'initial' flag. + def get_ticket(): + tgt = self.get_tgt(creds, fresh=True) + + # Get a non-initial ticket to kpasswd. + ticket = self.get_service_ticket(tgt, + krbtgt_creds, + service='kadmin', + target_name='changepw', + kdc_options='0', + fresh=True) + + set_initial_flag = partial(self.modify_ticket_flag, flag='initial', + value=True) + + checksum_keys = self.get_krbtgt_checksum_key() + return self.modified_ticket(ticket, + modify_fn=set_initial_flag, + checksum_keys=checksum_keys) + + expected_code = KPASSWD_SUCCESS + expected_msg = b'Password changed' + + ticket = get_ticket() + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + creds.update_password(new_password) + ticket = get_ticket() + + # Change the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test that kpasswd rejects requests where the ticket is encrypted with a + # key other than the krbtgt's. + def test_kpasswd_wrong_key(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + # Get a key belonging to the Administrator account. + admin_creds = self.get_admin_creds() + admin_key = self.TicketDecryptionKey_from_creds(admin_creds) + self.assertIsNotNone(admin_key.kvno, + 'a kvno is required to tell the DB ' + 'which key to look up.') + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: admin_key, + } + + # Re-encrypt the ticket using the Administrator's key. + ticket = self.modified_ticket(ticket, + new_ticket_key=admin_key, + checksum_keys=checksum_keys) + + # Set the sname of the ticket to that of the Administrator account. + admin_sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=['Administrator']) + ticket.set_sname(admin_sname) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + # Test that kpasswd rejects requests where the ticket is encrypted with a + # key belonging to a server account other than the krbtgt. + def test_kpasswd_wrong_key_server(self): + # Create an account for testing. + creds = self._get_creds() + + sname = self.get_kpasswd_sname() + + # Get an initial ticket to kpasswd. + ticket = self.get_tgt(creds, sname=sname, + kdc_options='0') + + # Get a key belonging to the Administrator account. + dc_creds = self.get_dc_creds() + dc_key = self.TicketDecryptionKey_from_creds(dc_creds) + self.assertIsNotNone(dc_key.kvno, + 'a kvno is required to tell the DB ' + 'which key to look up.') + checksum_keys = { + krb5pac.PAC_TYPE_KDC_CHECKSUM: dc_key, + } + + # Re-encrypt the ticket using the Administrator's key. + ticket = self.modified_ticket(ticket, + new_ticket_key=dc_key, + checksum_keys=checksum_keys) + + # Set the sname of the ticket to that of the DC's account. + dc_username = dc_creds.get_username() + dc_sname = self.PrincipalName_create(name_type=NT_PRINCIPAL, + names=dc_username.split('/')) + ticket.set_sname(dc_sname) + + expected_code = KPASSWD_HARDERROR + expected_msg = b'gensec_update failed - NT_STATUS_LOGON_FAILURE\n' + + # Set the password. + new_password = generate_random_password(32, 32) + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.SET) + + # Change the password. + self.kpasswd_exchange(ticket, + new_password, + expected_code, + expected_msg, + mode=self.KpasswdMode.CHANGE) + + +if __name__ == '__main__': + global_asn1_print = False + global_hexdump = False + import unittest + unittest.main() diff --git a/python/samba/tests/krb5/raw_testcase.py b/python/samba/tests/krb5/raw_testcase.py index 0deb6f0d76d..0222b87ec3d 100644 --- a/python/samba/tests/krb5/raw_testcase.py +++ b/python/samba/tests/krb5/raw_testcase.py @@ -508,6 +508,10 @@ class KerberosCredentials(Credentials): def get_upn(self): return self.upn + def update_password(self, password): + self.set_password(password) + self.set_kvno(self.get_kvno() + 1) + class KerberosTicketCreds: def __init__(self, ticket, session_key, @@ -526,6 +530,10 @@ class KerberosTicketCreds: self.ticket_private = ticket_private self.encpart_private = encpart_private + def set_sname(self, sname): + self.ticket['sname'] = sname + self.sname = sname + class RawKerberosTest(TestCaseInTempDir): """A raw Kerberos Test case.""" diff --git a/python/samba/tests/usage.py b/python/samba/tests/usage.py index f8aed2c67dd..346d57d2f20 100644 --- a/python/samba/tests/usage.py +++ b/python/samba/tests/usage.py @@ -111,6 +111,7 @@ EXCLUDE_USAGE = { 'python/samba/tests/krb5/test_idmap_nss.py', 'python/samba/tests/krb5/pac_align_tests.py', 'python/samba/tests/krb5/protected_users_tests.py', + 'python/samba/tests/krb5/kpasswd_tests.py', } EXCLUDE_HELP = { diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index cac84543f5e..6e9c7e0d6df 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -1698,6 +1698,10 @@ planoldpythontestsuite( 'ad_dc:local', 'samba.tests.krb5.protected_users_tests', environ=krb5_environ) +planoldpythontestsuite( + 'ad_dc:local', + 'samba.tests.krb5.kpasswd_tests', + environ=krb5_environ) for env in [ 'vampire_dc', -- 2.35.0 From e8070f3394437d1791850d6b2b5af430c1928aad Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 17 May 2022 20:25:19 +1200 Subject: [PATCH 23/46] lib:krb5_wrap: Use case-sensitive comparison against 'krbtgt' This matches the other comparisons against krbtgt, kadmin, etc., which are all case-sensitive. Signed-off-by: Joseph Sutton --- lib/krb5_wrap/krb5_samba.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c index 2351d172779..5d9c6595638 100644 --- a/lib/krb5_wrap/krb5_samba.c +++ b/lib/krb5_wrap/krb5_samba.c @@ -3369,7 +3369,7 @@ int smb_krb5_principal_is_tgs(krb5_context context, } eq = krb5_princ_size(context, principal) == 2 && - (strequal(p, KRB5_TGS_NAME)); + (strcmp(p, KRB5_TGS_NAME) == 0); talloc_free(p); -- 2.35.0 From 894043f82b8b857bcf4067e97941aefa4424fd89 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:48:59 +1200 Subject: [PATCH 24/46] s4:kpasswd: Don't return AP-REP on failure BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-service.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 061aedc80e5..22e1295c11e 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -256,6 +256,7 @@ kdc_code kpasswd_process(struct kdc_server *kdc, &kpasswd_dec_reply, &error_string); if (code != 0) { + ap_rep_blob = data_blob_null; error_code = code; goto reply; } @@ -265,6 +266,7 @@ kdc_code kpasswd_process(struct kdc_server *kdc, &kpasswd_dec_reply, &enc_data_blob); if (!NT_STATUS_IS_OK(status)) { + ap_rep_blob = data_blob_null; error_code = KRB5_KPASSWD_HARDERROR; error_string = talloc_asprintf(tmp_ctx, "gensec_wrap failed - %s\n", -- 2.35.0 From 65e302c28d0e102cfe0942736c26abfb12ab11c1 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:29:34 +1200 Subject: [PATCH 25/46] lib:krb5_wrap: Generate valid error codes in smb_krb5_mk_error() The error code passed in will be an offset from ERROR_TABLE_BASE_krb5, so we need to subtract that before creating the error. Heimdal does this internally, so it isn't needed there. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- lib/krb5_wrap/krb5_samba.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c index 5d9c6595638..0b98c6acb60 100644 --- a/lib/krb5_wrap/krb5_samba.c +++ b/lib/krb5_wrap/krb5_samba.c @@ -237,7 +237,7 @@ krb5_error_code smb_krb5_mk_error(krb5_context context, return code; } - errpkt.error = error_code; + errpkt.error = error_code - ERROR_TABLE_BASE_krb5; errpkt.text.length = 0; if (e_text != NULL) { -- 2.35.0 From b8fa7e9aecf254cb6ef8231c97fae5f5e7c068cc Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:49:43 +1200 Subject: [PATCH 26/46] s4:kpasswd: Return a kpasswd error code in KRB-ERROR If we attempt to return an error code outside of Heimdal's allowed range [KRB5KDC_ERR_NONE, KRB5_ERR_RCSID), it will be replaced with a GENERIC error, and the error text will be set to the meaningless result of krb5_get_error_message(). Avoid this by ensuring the error code is in the correct range. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 22e1295c11e..379ddebf3ad 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -315,7 +315,7 @@ reply: } code = smb_krb5_mk_error(kdc->smb_krb5_context->krb5_context, - error_code, + KRB5KDC_ERR_NONE + error_code, NULL, /* e_text */ &k_dec_data, NULL, /* client */ -- 2.35.0 From ae39b5f40d9a10b4a852e038fb037382e89ad875 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:21:06 +1200 Subject: [PATCH 27/46] s4:kpasswd: Correctly generate error strings The error_data we create already has an explicit length, and should not be zero-terminated, so we omit the trailing null byte. Previously, Heimdal builds would leave a superfluous trailing null byte on error strings, while MIT builds would omit the final character. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15074 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-helper.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/source4/kdc/kpasswd-helper.c b/source4/kdc/kpasswd-helper.c index ffd0a1ef060..143016e2ef9 100644 --- a/source4/kdc/kpasswd-helper.c +++ b/source4/kdc/kpasswd-helper.c @@ -48,17 +48,15 @@ bool kpasswd_make_error_reply(TALLOC_CTX *mem_ctx, } /* - * The string 's' has two terminating nul-bytes which are also - * reflected by 'slen'. Normally Kerberos doesn't expect that strings - * are nul-terminated, but Heimdal does! + * The string 's' has one terminating nul-byte which is also + * reflected by 'slen'. */ -#ifndef SAMBA4_USES_HEIMDAL - if (slen < 2) { + if (slen < 1) { talloc_free(s); return false; } - slen -= 2; -#endif + slen--; + if (2 + slen < slen) { talloc_free(s); return false; -- 2.35.0 From 8958ba8c75c332d7d1f858476a1f89442a589e85 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:06:31 +1200 Subject: [PATCH 28/46] gensec_krb5: Add helper function to check if client sent an initial ticket This will be used in the kpasswd service to ensure that the client has an initial ticket to kadmin/changepw, and not a service ticket. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source4/auth/gensec/gensec_krb5.c | 20 +----- source4/auth/gensec/gensec_krb5_helpers.c | 72 ++++++++++++++++++++++ source4/auth/gensec/gensec_krb5_helpers.h | 32 ++++++++++ source4/auth/gensec/gensec_krb5_internal.h | 47 ++++++++++++++ source4/auth/gensec/wscript_build | 4 ++ 5 files changed, 157 insertions(+), 18 deletions(-) create mode 100644 source4/auth/gensec/gensec_krb5_helpers.c create mode 100644 source4/auth/gensec/gensec_krb5_helpers.h create mode 100644 source4/auth/gensec/gensec_krb5_internal.h diff --git a/source4/auth/gensec/gensec_krb5.c b/source4/auth/gensec/gensec_krb5.c index 7d87b3ac6b9..104e4639c44 100644 --- a/source4/auth/gensec/gensec_krb5.c +++ b/source4/auth/gensec/gensec_krb5.c @@ -44,27 +44,11 @@ #include "../lib/util/asn1.h" #include "auth/kerberos/pac_utils.h" #include "gensec_krb5.h" +#include "gensec_krb5_internal.h" +#include "gensec_krb5_helpers.h" _PUBLIC_ NTSTATUS gensec_krb5_init(TALLOC_CTX *); -enum GENSEC_KRB5_STATE { - GENSEC_KRB5_SERVER_START, - GENSEC_KRB5_CLIENT_START, - GENSEC_KRB5_CLIENT_MUTUAL_AUTH, - GENSEC_KRB5_DONE -}; - -struct gensec_krb5_state { - enum GENSEC_KRB5_STATE state_position; - struct smb_krb5_context *smb_krb5_context; - krb5_auth_context auth_context; - krb5_data enc_ticket; - krb5_keyblock *keyblock; - krb5_ticket *ticket; - bool gssapi; - krb5_flags ap_req_options; -}; - static int gensec_krb5_destroy(struct gensec_krb5_state *gensec_krb5_state) { if (!gensec_krb5_state->smb_krb5_context) { diff --git a/source4/auth/gensec/gensec_krb5_helpers.c b/source4/auth/gensec/gensec_krb5_helpers.c new file mode 100644 index 00000000000..e765fc2c3c2 --- /dev/null +++ b/source4/auth/gensec/gensec_krb5_helpers.c @@ -0,0 +1,72 @@ +/* + Unix SMB/CIFS implementation. + + Kerberos backend for GENSEC + + Copyright (C) Andrew Bartlett 2004 + Copyright (C) Andrew Tridgell 2001 + Copyright (C) Luke Howard 2002-2003 + Copyright (C) Stefan Metzmacher 2004-2005 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "auth/auth.h" +#include "auth/gensec/gensec.h" +#include "auth/gensec/gensec_internal.h" +#include "gensec_krb5_internal.h" +#include "gensec_krb5_helpers.h" +#include "system/kerberos.h" +#include "auth/kerberos/kerberos.h" + +static struct gensec_krb5_state *get_private_state(const struct gensec_security *gensec_security) +{ + struct gensec_krb5_state *gensec_krb5_state = NULL; + + if (strcmp(gensec_security->ops->name, "krb5") != 0) { + /* We require that the krb5 mechanism is being used. */ + return NULL; + } + + gensec_krb5_state = talloc_get_type(gensec_security->private_data, + struct gensec_krb5_state); + return gensec_krb5_state; +} + +/* + * Returns 1 if our ticket has the initial flag set, 0 if not, and -1 in case of + * error. + */ +int gensec_krb5_initial_ticket(struct gensec_security *gensec_security) +{ + struct gensec_krb5_state *gensec_krb5_state = NULL; + + gensec_krb5_state = get_private_state(gensec_security); + if (gensec_krb5_state == NULL) { + return -1; + } + + if (gensec_krb5_state->ticket == NULL) { + /* We don't have a ticket */ + return -1; + } + +#ifdef SAMBA4_USES_HEIMDAL + return gensec_krb5_state->ticket->ticket.flags.initial; +#else /* MIT KERBEROS */ + return (gensec_krb5_state->ticket->enc_part2->flags & TKT_FLG_INITIAL) ? 1 : 0; +#endif /* SAMBA4_USES_HEIMDAL */ +} diff --git a/source4/auth/gensec/gensec_krb5_helpers.h b/source4/auth/gensec/gensec_krb5_helpers.h new file mode 100644 index 00000000000..6e52acea57d --- /dev/null +++ b/source4/auth/gensec/gensec_krb5_helpers.h @@ -0,0 +1,32 @@ +/* + Unix SMB/CIFS implementation. + + Kerberos backend for GENSEC + + Copyright (C) Andrew Bartlett 2004 + Copyright (C) Andrew Tridgell 2001 + Copyright (C) Luke Howard 2002-2003 + Copyright (C) Stefan Metzmacher 2004-2005 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +struct gensec_security; + +/* + * Returns 1 if our ticket has the initial flag set, 0 if not, and -1 in case of + * error. + */ +int gensec_krb5_initial_ticket(struct gensec_security *gensec_security); diff --git a/source4/auth/gensec/gensec_krb5_internal.h b/source4/auth/gensec/gensec_krb5_internal.h new file mode 100644 index 00000000000..0bb796f1b2a --- /dev/null +++ b/source4/auth/gensec/gensec_krb5_internal.h @@ -0,0 +1,47 @@ +/* + Unix SMB/CIFS implementation. + + Kerberos backend for GENSEC + + Copyright (C) Andrew Bartlett 2004 + Copyright (C) Andrew Tridgell 2001 + Copyright (C) Luke Howard 2002-2003 + Copyright (C) Stefan Metzmacher 2004-2005 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "auth/gensec/gensec.h" +#include "system/kerberos.h" +#include "auth/kerberos/kerberos.h" + +enum GENSEC_KRB5_STATE { + GENSEC_KRB5_SERVER_START, + GENSEC_KRB5_CLIENT_START, + GENSEC_KRB5_CLIENT_MUTUAL_AUTH, + GENSEC_KRB5_DONE +}; + +struct gensec_krb5_state { + enum GENSEC_KRB5_STATE state_position; + struct smb_krb5_context *smb_krb5_context; + krb5_auth_context auth_context; + krb5_data enc_ticket; + krb5_keyblock *keyblock; + krb5_ticket *ticket; + bool gssapi; + krb5_flags ap_req_options; +}; diff --git a/source4/auth/gensec/wscript_build b/source4/auth/gensec/wscript_build index d14a50ff273..20271f1665b 100644 --- a/source4/auth/gensec/wscript_build +++ b/source4/auth/gensec/wscript_build @@ -18,6 +18,10 @@ bld.SAMBA_MODULE('gensec_krb5', enabled=bld.AD_DC_BUILD_IS_ENABLED() ) +bld.SAMBA_SUBSYSTEM('gensec_krb5_helpers', + source='gensec_krb5_helpers.c', + deps='gensec_krb5', + enabled=bld.AD_DC_BUILD_IS_ENABLED()) bld.SAMBA_MODULE('gensec_gssapi', source='gensec_gssapi.c', -- 2.35.0 From 9e1a5e284825f3f7def9a635e38b5592c2d46116 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:52:41 +1200 Subject: [PATCH 29/46] s4:kpasswd: Require an initial ticket Ensure the client uses an AS-REQ to get the ticket to kpasswd, and not a TGS-REQ. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-service.c | 10 ++++++++++ source4/kdc/wscript_build | 1 + 2 files changed, 11 insertions(+) diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 379ddebf3ad..633f32a98f9 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -26,6 +26,7 @@ #include "auth/credentials/credentials.h" #include "auth/auth.h" #include "auth/gensec/gensec.h" +#include "gensec_krb5_helpers.h" #include "kdc/kdc-server.h" #include "kdc/kpasswd-service.h" #include "kdc/kpasswd-helper.h" @@ -234,6 +235,15 @@ kdc_code kpasswd_process(struct kdc_server *kdc, goto reply; } + rv = gensec_krb5_initial_ticket(gensec_security); + if (rv != 1) { + ap_rep_blob = data_blob_null; + error_code = KRB5_KPASSWD_INITIAL_FLAG_NEEDED; + error_string = "Expected an initial ticket\n"; + DBG_ERR("%s", error_string); + goto reply; + } + status = gensec_unwrap(gensec_security, tmp_ctx, &enc_data_blob, diff --git a/source4/kdc/wscript_build b/source4/kdc/wscript_build index 5c16e68ee0a..0c902f50534 100644 --- a/source4/kdc/wscript_build +++ b/source4/kdc/wscript_build @@ -85,6 +85,7 @@ bld.SAMBA_SUBSYSTEM('KPASSWD-SERVICE', krb5samba samba_server_gensec KPASSWD_GLUE + gensec_krb5_helpers ''') bld.SAMBA_SUBSYSTEM('KDC-GLUE', -- 2.35.0 From 83fc4a5a29c5e09f736f03525be3970559a0f736 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 17:11:49 +1200 Subject: [PATCH 30/46] s4:kpasswd: Restructure code for clarity View with 'git show -b'. Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-service-heimdal.c | 46 +++++++++++++-------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/source4/kdc/kpasswd-service-heimdal.c b/source4/kdc/kpasswd-service-heimdal.c index 21596d8d8a4..d5061af6f83 100644 --- a/source4/kdc/kpasswd-service-heimdal.c +++ b/source4/kdc/kpasswd-service-heimdal.c @@ -145,30 +145,7 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, return 0; } - if (chpw.targname != NULL && chpw.targrealm != NULL) { - code = krb5_build_principal_ext(context, - &target_principal, - strlen(*chpw.targrealm), - *chpw.targrealm, - 0); - if (code != 0) { - free_ChangePasswdDataMS(&chpw); - return kpasswd_make_error_reply(mem_ctx, - KRB5_KPASSWD_MALFORMED, - "Failed to parse principal", - kpasswd_reply); - } - code = copy_PrincipalName(chpw.targname, - &target_principal->name); - if (code != 0) { - free_ChangePasswdDataMS(&chpw); - krb5_free_principal(context, target_principal); - return kpasswd_make_error_reply(mem_ctx, - KRB5_KPASSWD_MALFORMED, - "Failed to parse principal", - kpasswd_reply); - } - } else { + if (chpw.targname == NULL || chpw.targrealm == NULL) { free_ChangePasswdDataMS(&chpw); return kpasswd_change_password(kdc, mem_ctx, @@ -177,7 +154,28 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, kpasswd_reply, error_string); } + code = krb5_build_principal_ext(context, + &target_principal, + strlen(*chpw.targrealm), + *chpw.targrealm, + 0); + if (code != 0) { + free_ChangePasswdDataMS(&chpw); + return kpasswd_make_error_reply(mem_ctx, + KRB5_KPASSWD_MALFORMED, + "Failed to parse principal", + kpasswd_reply); + } + code = copy_PrincipalName(chpw.targname, + &target_principal->name); free_ChangePasswdDataMS(&chpw); + if (code != 0) { + krb5_free_principal(context, target_principal); + return kpasswd_make_error_reply(mem_ctx, + KRB5_KPASSWD_MALFORMED, + "Failed to parse principal", + kpasswd_reply); + } if (target_principal->name.name_string.len >= 2) { is_service_principal = true; -- 2.35.0 From 56f7a3ab105ebce1a2810bb1323b16be10609a78 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 24 May 2022 10:17:00 +0200 Subject: [PATCH 31/46] testprogs: Fix auth with smbclient and krb5 ccache BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Andreas Schneider Reviewed-by: Joseph Sutton --- 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.35.0 From 0509d6ca06391a9c809782270dbea754ff5fb7f0 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 19 May 2022 16:35:28 +0200 Subject: [PATCH 32/46] 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 Reviewed-by: Joseph Sutton --- 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 < "${PREFIX}/tmpkpasswdscript" < "${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.35.0 From bc187b3471765ed1f9241acdf990cd9f33ebfd6d Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 24 May 2022 09:54:18 +0200 Subject: [PATCH 33/46] s4:kdc: Implement is_kadmin_changepw() helper function BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Andreas Schneider Reviewed-by: Joseph Sutton --- 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.35.0 From 25e6a5678991183967d95a5a04b4795c3ba96b53 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: [PATCH 34/46] s4:kdc: Split out a samba_kdc_get_entry_principal() function BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Reviewed-by: Andreas Schneider --- source4/kdc/db-glue.c | 192 +++++++++++++++++++++++------------------- 1 file changed, 107 insertions(+), 85 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index b03a32b67f5..fa6d4b16675 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -925,6 +925,101 @@ 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, + krb5_const_principal in_princ, + krb5_principal *out_princ) +{ + struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx; + krb5_error_code ret = 0; + + /* + * If we are set to canonicalize, we get back the fixed UPPER + * case realm, and the real username (ie matching LDAP + * samAccountName) + * + * Otherwise, if we are set to enterprise, we + * get back the whole principal as-sent + * + * Finally, if we are not set to canonicalize, we get back the + * fixed UPPER case realm, but the as-sent username + */ + + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { + 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, out_princ, + lpcfg_realm(lp_ctx), "krbtgt", + lpcfg_realm(lp_ctx), NULL); + if (ret) { + return ret; + } + smb_krb5_principal_set_type(context, *out_princ, KRB5_NT_SRV_INST); + } else { + ret = krb5_copy_principal(context, in_princ, out_princ); + if (ret) { + return ret; + } + /* + * this appears to be required regardless of + * the canonicalize flag from the client + */ + ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); + if (ret) { + return ret; + } + } + + } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL) { + ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); + if (ret) { + return ret; + } + } 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, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); + if (ret) { + return ret; + } + } else { + ret = krb5_copy_principal(context, in_princ, out_princ); + if (ret) { + return ret; + } + + /* 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, *out_princ, lpcfg_realm(lp_ctx)); + if (ret) { + return ret; + } + } + + return 0; +} + /* * Construct an hdb_entry from a directory entry. */ @@ -1017,93 +1112,8 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, userAccountControl |= msDS_User_Account_Control_Computed; } - /* - * If we are set to canonicalize, we get back the fixed UPPER - * case realm, and the real username (ie matching LDAP - * samAccountName) - * - * Otherwise, if we are set to enterprise, we - * get back the whole principal as-sent - * - * Finally, if we are not set to canonicalize, we get back the - * fixed UPPER case realm, but the as-sent username - */ - 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 +1306,18 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, } } + ret = samba_kdc_get_entry_principal(context, + kdc_db_ctx, + samAccountName, + ent_type, + flags, + 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.35.0 From 5618a10ab177ac397334d8b222c075f903844b3b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 25 May 2022 17:19:58 +1200 Subject: [PATCH 35/46] s4:kdc: Refactor samba_kdc_get_entry_principal() This eliminates some duplicate branches. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Signed-off-by: Joseph Sutton Pair-Programmed-With: Andreas Schneider --- source4/kdc/db-glue.c | 116 ++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 61 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index fa6d4b16675..1fa1fc94128 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -935,7 +935,8 @@ static krb5_error_code samba_kdc_get_entry_principal( krb5_principal *out_princ) { struct loadparm_context *lp_ctx = kdc_db_ctx->lp_ctx; - krb5_error_code ret = 0; + krb5_error_code code = 0; + bool canon = flags & (SDB_F_CANON|SDB_F_FORCE_CANON); /* * If we are set to canonicalize, we get back the fixed UPPER @@ -949,75 +950,68 @@ static krb5_error_code samba_kdc_get_entry_principal( * fixed UPPER case realm, but the as-sent username */ - if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT) { - 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, out_princ, - lpcfg_realm(lp_ctx), "krbtgt", - lpcfg_realm(lp_ctx), NULL); - if (ret) { - return ret; - } - smb_krb5_principal_set_type(context, *out_princ, KRB5_NT_SRV_INST); - } else { - ret = krb5_copy_principal(context, in_princ, out_princ); - if (ret) { - return ret; - } - /* - * this appears to be required regardless of - * the canonicalize flag from the client - */ - ret = smb_krb5_principal_set_realm(context, *out_princ, lpcfg_realm(lp_ctx)); - if (ret) { - return ret; - } + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) { + /* + * 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); - } else if (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL) { - ret = smb_krb5_make_principal(context, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - return ret; - } - } else if ((flags & SDB_F_FORCE_CANON) || - ((flags & SDB_F_CANON) && (flags & SDB_F_FOR_AS_REQ))) { + return 0; + } + + if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) || + (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) { /* * 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 + * 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. + * 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, out_princ, lpcfg_realm(lp_ctx), samAccountName, NULL); - if (ret) { - return ret; - } - } else { - ret = krb5_copy_principal(context, in_princ, out_princ); - if (ret) { - return ret; - } - - /* 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, *out_princ, lpcfg_realm(lp_ctx)); - if (ret) { - return ret; - } + code = smb_krb5_make_principal(context, + out_princ, + lpcfg_realm(lp_ctx), + samAccountName, + NULL); + return code; } - return 0; + /* + * 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; } /* -- 2.35.0 From c4ff9b4b1ff64be327c02c8719912f87d1bb7a7b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: [PATCH 36/46] 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. View with 'git show -b'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15047 Pair-Programmed-With: Andreas Schneider Signed-off-by: Andreas Schneider Signed-off-by: Joseph Sutton --- selftest/knownfail.d/kadmin_changew | 1 - source4/kdc/db-glue.c | 84 ++++++++++++++++------------- 2 files changed, 46 insertions(+), 39 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 1fa1fc94128..6d105668e44 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -931,6 +931,7 @@ static krb5_error_code samba_kdc_get_entry_principal( 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) { @@ -950,46 +951,52 @@ static krb5_error_code samba_kdc_get_entry_principal( * fixed UPPER case realm, but the as-sent username */ - if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) { - /* - * 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) { + /* + * We need to ensure that the kadmin/changepw principal isn't able to + * issue krbtgt tickets, even if canonicalization is turned on. + */ + if (!is_kadmin_changepw) { + if (ent_type == SAMBA_KDC_ENT_TYPE_KRBTGT && canon) { + /* + * 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 ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) || + (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) { + /* + * 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; } - smb_krb5_principal_set_type(context, - *out_princ, - KRB5_NT_SRV_INST); - - return 0; - } - - if ((canon && flags & (SDB_F_FORCE_CANON|SDB_F_FOR_AS_REQ)) || - (ent_type == SAMBA_KDC_ENT_TYPE_ANY && in_princ == NULL)) { - /* - * 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; } /* @@ -1305,6 +1312,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, samAccountName, ent_type, flags, + entry->flags.change_pw, principal, &entry->principal); if (ret != 0) { -- 2.35.0 From 00af33c776157504bd866b57ffae4b925611f9a6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:51:08 +1200 Subject: [PATCH 37/46] third_party/heimdal: Remove unused 'server' parameter from krb5_verify_ap_req{,2}() Signed-off-by: Joseph Sutton --- third_party/heimdal/kdc/fast.c | 1 - third_party/heimdal/kdc/krb5tgs.c | 1 - third_party/heimdal/lib/krb5/rd_req.c | 5 ----- 3 files changed, 7 deletions(-) diff --git a/third_party/heimdal/kdc/fast.c b/third_party/heimdal/kdc/fast.c index 392fc966050..64c6fbe669f 100644 --- a/third_party/heimdal/kdc/fast.c +++ b/third_party/heimdal/kdc/fast.c @@ -574,7 +574,6 @@ fast_unwrap_request(astgs_request_t r, ret = krb5_verify_ap_req2(r->context, &ac, &ap_req, - armor_server_principal, &r->armor_key->key, 0, &ap_req_options, diff --git a/third_party/heimdal/kdc/krb5tgs.c b/third_party/heimdal/kdc/krb5tgs.c index aab6806fbe1..f5f1422b1b9 100644 --- a/third_party/heimdal/kdc/krb5tgs.c +++ b/third_party/heimdal/kdc/krb5tgs.c @@ -1072,7 +1072,6 @@ next_kvno: ret = krb5_verify_ap_req2(r->context, &ac, &ap_req, - princ, &tkey->key, verify_ap_req_flags, &ap_req_options, diff --git a/third_party/heimdal/lib/krb5/rd_req.c b/third_party/heimdal/lib/krb5/rd_req.c index 371037c8403..60e79a275ef 100644 --- a/third_party/heimdal/lib/krb5/rd_req.c +++ b/third_party/heimdal/lib/krb5/rd_req.c @@ -277,7 +277,6 @@ KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_verify_ap_req(krb5_context context, krb5_auth_context *auth_context, krb5_ap_req *ap_req, - krb5_const_principal server, krb5_keyblock *keyblock, krb5_flags flags, krb5_flags *ap_req_options, @@ -286,7 +285,6 @@ krb5_verify_ap_req(krb5_context context, return krb5_verify_ap_req2 (context, auth_context, ap_req, - server, keyblock, flags, ap_req_options, @@ -298,7 +296,6 @@ KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_verify_ap_req2(krb5_context context, krb5_auth_context *auth_context, krb5_ap_req *ap_req, - krb5_const_principal server, krb5_keyblock *keyblock, krb5_flags flags, krb5_flags *ap_req_options, @@ -947,7 +944,6 @@ krb5_rd_req_ctx(krb5_context context, ret = krb5_verify_ap_req2(context, auth_context, &ap_req, - server, o->keyblock, 0, &o->ap_req_options, @@ -995,7 +991,6 @@ krb5_rd_req_ctx(krb5_context context, ret = krb5_verify_ap_req2(context, auth_context, &ap_req, - server, &entry.keyblock, 0, &o->ap_req_options, -- 2.35.0 From 67c9ca90b747d534b5f5ecaf4fb5be6b7e44869a Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:53:49 +1200 Subject: [PATCH 38/46] s4:kdc: Limit kpasswd ticket lifetime to two minutes or less This matches the behaviour of Windows. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source4/kdc/db-glue.c | 5 +++++ source4/kdc/mit-kdb/kdb_samba_principals.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 6d105668e44..26fe625d75d 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -1337,6 +1337,11 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context, kdc_db_ctx->policy.usr_tkt_lifetime); } + if (entry->flags.change_pw) { + /* Limit lifetime of kpasswd tickets to two minutes or less. */ + *entry->max_life = MIN(*entry->max_life, 2 * 60); + } + entry->max_renew = malloc(sizeof(*entry->max_renew)); if (entry->max_renew == NULL) { ret = ENOMEM; diff --git a/source4/kdc/mit-kdb/kdb_samba_principals.c b/source4/kdc/mit-kdb/kdb_samba_principals.c index 31983a7da6c..1fef1bbe58e 100644 --- a/source4/kdc/mit-kdb/kdb_samba_principals.c +++ b/source4/kdc/mit-kdb/kdb_samba_principals.c @@ -35,7 +35,7 @@ #define DBGC_CLASS DBGC_KERBEROS #define ADMIN_LIFETIME 60*60*3 /* 3 hours */ -#define CHANGEPW_LIFETIME 60*5 /* 5 minutes */ +#define CHANGEPW_LIFETIME 60*2 /* 2 minutes */ krb5_error_code ks_get_principal(krb5_context context, krb5_const_principal principal, -- 2.35.0 From fb350a7bd81ff6a9d6caae6f8b81c5a88f0c2088 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:54:36 +1200 Subject: [PATCH 39/46] third_party/heimdal: Don't accept tickets living two minutes or less This matches the behaviour of Windows. The object of this requirement is to ensure we don't allow kpasswd tickets, not having a lifetime of more than two minutes, to be passed off as TGTs. An existing requirement for TGTs to contain a REQUESTER_SID PAC buffer suffices to prevent kpasswd ticket misuse, so this is just an additional precaution on top. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- third_party/heimdal/kdc/krb5tgs.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/third_party/heimdal/kdc/krb5tgs.c b/third_party/heimdal/kdc/krb5tgs.c index f5f1422b1b9..9cdd2b8c31d 100644 --- a/third_party/heimdal/kdc/krb5tgs.c +++ b/third_party/heimdal/kdc/krb5tgs.c @@ -209,6 +209,7 @@ check_tgs_flags(astgs_request_t r, KDC_REQ_BODY *b, const EncTicketPart *tgt, EncTicketPart *et) { KDCOptions f = b->kdc_options; + time_t old_life; if(f.validate){ if (!tgt->flags.invalid || tgt->starttime == NULL) { @@ -304,23 +305,31 @@ check_tgs_flags(astgs_request_t r, KDC_REQ_BODY *b, _kdc_fix_time(&b->rtime); *et->renew_till = *b->rtime; } + + old_life = tgt->endtime; + if(tgt->starttime) + old_life -= *tgt->starttime; + else + old_life -= tgt->authtime; + if(f.renew){ - time_t old_life; if (!tgt->flags.renewable || tgt->renew_till == NULL) { kdc_audit_addreason((kdc_request_t)r, "Request to renew non-renewable ticket"); return KRB5KDC_ERR_BADOPTION; } - old_life = tgt->endtime; - if(tgt->starttime) - old_life -= *tgt->starttime; - else - old_life -= tgt->authtime; et->endtime = *et->starttime + old_life; if (et->renew_till != NULL) et->endtime = min(*et->renew_till, et->endtime); } + if (old_life <= 2 * 60) { + /* This may be a kpasswd ticket rather than a TGT, so don't accept it. */ + kdc_audit_addreason((kdc_request_t)r, + "Ticket lifetime is too short"); + return KRB5KRB_AP_ERR_TKT_EXPIRED; + } + /* * RFC 8062 section 3 defines an anonymous ticket as one containing * the anonymous principal and the anonymous ticket flag. @@ -404,6 +413,11 @@ _kdc_verify_flags(krb5_context context, kdc_log(context, config, 4, "Ticket expired (%s)", pstr); return KRB5KRB_AP_ERR_TKT_EXPIRED; } + if (rk_time_sub(et->endtime, et->authtime) <= 2 * 60) { + /* This may be a kpasswd ticket rather than a TGT, so don't accept it. */ + kdc_log(context, config, 4, "Ticket lifetime too short (%s)", pstr); + return KRB5KRB_AP_ERR_TKT_EXPIRED; + } if(et->flags.invalid){ kdc_log(context, config, 4, "Ticket not valid (%s)", pstr); return KRB5KRB_AP_ERR_TKT_NYV; -- 2.35.0 From f7f3441beb0d07a0c025173d47db7da8cff998d6 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 21:36:10 +1200 Subject: [PATCH 40/46] third_party/heimdal: Note that sname checks provide no security BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- third_party/heimdal/kdc/krb5tgs.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/third_party/heimdal/kdc/krb5tgs.c b/third_party/heimdal/kdc/krb5tgs.c index 9cdd2b8c31d..ecbd5ee2047 100644 --- a/third_party/heimdal/kdc/krb5tgs.c +++ b/third_party/heimdal/kdc/krb5tgs.c @@ -980,6 +980,10 @@ tgs_parse_request(astgs_request_t r, } if(!get_krbtgt_realm(&ap_req.ticket.sname)){ + /* + * Note: this check is a safeguard against error, but does not prevent a + * malicious user who changes the sname in the ticket. + */ /* XXX check for ticket.sname == req.sname */ kdc_log(r->context, config, 4, "PA-DATA is not a ticket-granting ticket"); ret = KRB5KDC_ERR_POLICY; /* ? */ @@ -1672,6 +1676,10 @@ server_lookup: } t = &b->additional_tickets->val[0]; if(!get_krbtgt_realm(&t->sname)){ + /* + * Note: this check is a safeguard against error, but does not prevent a + * malicious user who changes the sname in the ticket. + */ kdc_log(context, config, 4, "Additional ticket is not a ticket-granting ticket"); kdc_audit_addreason((kdc_request_t)priv, -- 2.35.0 From 3b82d244c18b42788847c530dd476100dfc19e78 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:52:05 +1200 Subject: [PATCH 41/46] third_party/heimdal: Don't allow HDB keytab iteration A fallback in krb5_rd_req_ctx() means that Samba's kpasswd service will try many inappropriate keys to decrypt the ticket supplied to it. For example, it will accept a ticket encrypted with the Administrator's key, when it should rather accept only tickets encrypted with the krbtgt's key (and not an RODC krbtgt). To fix this, declare the HDB keytab using the HDBGET ops, which do not support iteration. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kdc-heimdal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index 0d2a410fc3b..542986c5ad3 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -463,7 +463,7 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) return; } - kdc->keytab_name = talloc_asprintf(kdc, "HDB:samba4:&%p", kdc->base_ctx); + kdc->keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); if (kdc->keytab_name == NULL) { task_server_terminate(task, "kdc: Failed to set keytab name", @@ -471,7 +471,7 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) return; } - ret = krb5_kt_register(kdc->smb_krb5_context->krb5_context, &hdb_kt_ops); + ret = krb5_kt_register(kdc->smb_krb5_context->krb5_context, &hdb_get_kt_ops); if(ret) { task_server_terminate(task, "kdc: failed to register keytab plugin", true); return; -- 2.35.0 From 8bed08acfaeed2578fed4bdc81df63a5bc81d5da Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 25 May 2022 20:00:55 +1200 Subject: [PATCH 42/46] s4:kdc: Don't use strncmp to compare principal components We would only compare the first 'n' characters, where 'n' is the length of the principal component string, so 'k@REALM' would erroneously be considered equal to 'krbtgt@REALM'. Signed-off-by: Joseph Sutton --- source4/kdc/db-glue.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c index 26fe625d75d..09acc826f89 100644 --- a/source4/kdc/db-glue.c +++ b/source4/kdc/db-glue.c @@ -870,15 +870,19 @@ static int principal_comp_strcmp_int(krb5_context context, bool do_strcasecmp) { const char *p; - size_t len; #if defined(HAVE_KRB5_PRINCIPAL_GET_COMP_STRING) p = krb5_principal_get_comp_string(context, principal, component); if (p == NULL) { return -1; } - len = strlen(p); + if (do_strcasecmp) { + return strcasecmp(p, string); + } else { + return strcmp(p, string); + } #else + size_t len; krb5_data *d; if (component >= krb5_princ_size(context, principal)) { return -1; @@ -890,13 +894,26 @@ static int principal_comp_strcmp_int(krb5_context context, } p = d->data; - len = d->length; -#endif + + len = strlen(string); + + /* + * We explicitly return -1 or 1. Subtracting of the two lengths might + * give the wrong result if the result overflows or loses data when + * narrowed to int. + */ + if (d->length < len) { + return -1; + } else if (d->length > len) { + return 1; + } + if (do_strcasecmp) { return strncasecmp(p, string, len); } else { - return strncmp(p, string, len); + return memcmp(p, string, len); } +#endif } static int principal_comp_strcasecmp(krb5_context context, -- 2.35.0 From 7f41b867be9bde2a8957d08199327f7588aa32d4 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:36:30 +1200 Subject: [PATCH 43/46] s4:kdc: Rename keytab_name -> kpasswd_keytab_name This makes explicitly clear the purpose of this keytab. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kdc-heimdal.c | 4 ++-- source4/kdc/kdc-server.h | 2 +- source4/kdc/kdc-service-mit.c | 4 ++-- source4/kdc/kpasswd-service.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index 542986c5ad3..5b2b3e36652 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -463,8 +463,8 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) return; } - kdc->keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); - if (kdc->keytab_name == NULL) { + kdc->kpasswd_keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); + if (kdc->kpasswd_keytab_name == NULL) { task_server_terminate(task, "kdc: Failed to set keytab name", true); diff --git a/source4/kdc/kdc-server.h b/source4/kdc/kdc-server.h index fd883c2e4b4..89b30f122f5 100644 --- a/source4/kdc/kdc-server.h +++ b/source4/kdc/kdc-server.h @@ -40,7 +40,7 @@ struct kdc_server { struct ldb_context *samdb; bool am_rodc; uint32_t proxy_timeout; - const char *keytab_name; + const char *kpasswd_keytab_name; void *private_data; }; diff --git a/source4/kdc/kdc-service-mit.c b/source4/kdc/kdc-service-mit.c index f9aaedefc23..31cfef2eae1 100644 --- a/source4/kdc/kdc-service-mit.c +++ b/source4/kdc/kdc-service-mit.c @@ -307,8 +307,8 @@ NTSTATUS mitkdc_task_init(struct task_server *task) return NT_STATUS_INTERNAL_ERROR; } - kdc->keytab_name = talloc_asprintf(kdc, "KDB:"); - if (kdc->keytab_name == NULL) { + kdc->kpasswd_keytab_name = talloc_asprintf(kdc, "KDB:"); + if (kdc->kpasswd_keytab_name == NULL) { task_server_terminate(task, "KDC: Out of memory", true); diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 633f32a98f9..96ac2670add 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -171,7 +171,7 @@ kdc_code kpasswd_process(struct kdc_server *kdc, rv = cli_credentials_set_keytab_name(server_credentials, kdc->task->lp_ctx, - kdc->keytab_name, + kdc->kpasswd_keytab_name, CRED_SPECIFIED); if (rv != 0) { DBG_ERR("Failed to set credentials keytab name\n"); -- 2.35.0 From 2d11086a40efd3e233eb108345dab77c00073464 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:39:20 +1200 Subject: [PATCH 44/46] s4:kdc: Replace usage of HDB plugin with kpasswd HDB plugin This plugin ensures we only look up the kadmin/changepw principal, and can't be fooled into accepting tickets for other service principals. We make sure not to specify the kvno, to avoid accepting RODC-issued tickets. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/hdb-samba4-kpasswd-plugin.c | 84 +++++++++++++++++++++++++ source4/kdc/hdb-samba4.c | 58 +++++++++++++++++ source4/kdc/kdc-glue.h | 3 + source4/kdc/kdc-heimdal.c | 6 +- source4/kdc/samba_kdc.h | 2 +- source4/kdc/wscript_build | 2 +- 6 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 source4/kdc/hdb-samba4-kpasswd-plugin.c diff --git a/source4/kdc/hdb-samba4-kpasswd-plugin.c b/source4/kdc/hdb-samba4-kpasswd-plugin.c new file mode 100644 index 00000000000..29002b97d86 --- /dev/null +++ b/source4/kdc/hdb-samba4-kpasswd-plugin.c @@ -0,0 +1,84 @@ +/* + Unix SMB/CIFS implementation. + + KDC Server startup + + Copyright (C) Andrew Bartlett 2005-20011 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + +#include "includes.h" +#include "kdc/kdc-glue.h" +#include "lib/param/param.h" + +static krb5_error_code hdb_samba4_kpasswd_create(krb5_context context, struct HDB **db, const char *arg) +{ + NTSTATUS nt_status; + void *ptr = NULL; + struct samba_kdc_base_context *base_ctx = NULL; + + if (sscanf(arg, "&%p", &ptr) != 1) { + return EINVAL; + } + + base_ctx = talloc_get_type_abort(ptr, struct samba_kdc_base_context); + + /* The global kdc_mem_ctx and kdc_lp_ctx, Disgusting, ugly hack, but it means one less private hook */ + nt_status = hdb_samba4_kpasswd_create_kdc(base_ctx, context, db); + + if (NT_STATUS_IS_OK(nt_status)) { + return 0; + } else if (NT_STATUS_EQUAL(nt_status, NT_STATUS_ERROR_DS_INCOMPATIBLE_VERSION)) { + return EINVAL; + } else if (NT_STATUS_EQUAL(nt_status, NT_STATUS_CANT_ACCESS_DOMAIN_INFO)) { + krb5_set_error_message(context, EINVAL, "Failed to open Samba4 LDB at %s", lpcfg_private_path(base_ctx, base_ctx->lp_ctx, "sam.ldb")); + } else { + krb5_set_error_message(context, EINVAL, "Failed to connect to Samba4 DB: %s (%s)", get_friendly_nt_error_msg(nt_status), nt_errstr(nt_status)); + } + + return EINVAL; +} + +#if (HDB_INTERFACE_VERSION != 11) +#error "Unsupported Heimdal HDB version" +#endif + +#if HDB_INTERFACE_VERSION >= 8 +static krb5_error_code hdb_samba4_kpasswd_init(krb5_context context, void **ctx) +{ + *ctx = NULL; + return 0; +} + +static void hdb_samba4_kpasswd_fini(void *ctx) +{ +} +#endif + +/* Only used in the hdb-backed keytab code + * for a keytab of 'samba4kpasswd&
' or samba4, to find + * kpasswd's key in the main DB + * + * The
is the string form of a pointer to a talloced struct hdb_samba_context + */ +struct hdb_method hdb_samba4_kpasswd_interface = { + HDB_INTERFACE_VERSION, +#if HDB_INTERFACE_VERSION >= 8 + .init = hdb_samba4_kpasswd_init, + .fini = hdb_samba4_kpasswd_fini, +#endif + .prefix = "samba4kpasswd", + .create = hdb_samba4_kpasswd_create +}; diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 480d2c06e5e..67f8fbed8fd 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -293,6 +293,47 @@ static krb5_error_code hdb_samba4_fetch_kvno(krb5_context context, HDB *db, return code; } +static krb5_error_code hdb_samba4_kpasswd_fetch_kvno(krb5_context context, HDB *db, + krb5_const_principal _principal, + unsigned flags, + krb5_kvno _kvno, + hdb_entry *entry) +{ + struct samba_kdc_db_context *kdc_db_ctx = NULL; + krb5_error_code ret; + krb5_principal kpasswd_principal = NULL; + + kdc_db_ctx = talloc_get_type_abort(db->hdb_db, + struct samba_kdc_db_context); + + ret = smb_krb5_make_principal(context, &kpasswd_principal, + lpcfg_realm(kdc_db_ctx->lp_ctx), + "kadmin", "changepw", + NULL); + if (ret) { + return ret; + } + smb_krb5_principal_set_type(context, kpasswd_principal, KRB5_NT_SRV_INST); + + /* + * For the kpasswd service, always ensure we get the latest kvno. This + * also means we (correctly) refuse RODC-issued tickets. + */ + flags &= ~HDB_F_KVNO_SPECIFIED; + + /* Don't bother looking up a client or krbtgt. */ + flags &= ~(SDB_F_GET_CLIENT|SDB_F_GET_KRBTGT); + + ret = hdb_samba4_fetch_kvno(context, db, + kpasswd_principal, + flags, + 0, + entry); + + krb5_free_principal(context, kpasswd_principal); + return ret; +} + static krb5_error_code hdb_samba4_firstkey(krb5_context context, HDB *db, unsigned flags, hdb_entry *entry) { @@ -813,3 +854,20 @@ NTSTATUS hdb_samba4_create_kdc(struct samba_kdc_base_context *base_ctx, return NT_STATUS_OK; } + +NTSTATUS hdb_samba4_kpasswd_create_kdc(struct samba_kdc_base_context *base_ctx, + krb5_context context, struct HDB **db) +{ + NTSTATUS nt_status; + + nt_status = hdb_samba4_create_kdc(base_ctx, context, db); + if (!NT_STATUS_IS_OK(nt_status)) { + return nt_status; + } + + (*db)->hdb_fetch_kvno = hdb_samba4_kpasswd_fetch_kvno; + (*db)->hdb_firstkey = NULL; + (*db)->hdb_nextkey = NULL; + + return NT_STATUS_OK; +} diff --git a/source4/kdc/kdc-glue.h b/source4/kdc/kdc-glue.h index 47642e12432..7a0184c4021 100644 --- a/source4/kdc/kdc-glue.h +++ b/source4/kdc/kdc-glue.h @@ -46,6 +46,9 @@ kdc_code kpasswdd_process(struct kdc_server *kdc, NTSTATUS hdb_samba4_create_kdc(struct samba_kdc_base_context *base_ctx, krb5_context context, struct HDB **db); +NTSTATUS hdb_samba4_kpasswd_create_kdc(struct samba_kdc_base_context *base_ctx, + krb5_context context, struct HDB **db); + /* from kdc-glue.c */ int kdc_check_pac(krb5_context krb5_context, DATA_BLOB server_sig, diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index 5b2b3e36652..d34a1db8a65 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -456,14 +456,14 @@ static void kdc_post_fork(struct task_server *task, struct process_details *pd) } ret = krb5_plugin_register(kdc->smb_krb5_context->krb5_context, - PLUGIN_TYPE_DATA, "hdb_samba4_interface", - &hdb_samba4_interface); + PLUGIN_TYPE_DATA, "hdb_samba4kpasswd_interface", + &hdb_samba4_kpasswd_interface); if(ret) { task_server_terminate(task, "kdc: failed to register hdb plugin", true); return; } - kdc->kpasswd_keytab_name = talloc_asprintf(kdc, "HDBGET:samba4:&%p", kdc->base_ctx); + kdc->kpasswd_keytab_name = talloc_asprintf(kdc, "HDBGET:samba4kpasswd:&%p", kdc->base_ctx); if (kdc->kpasswd_keytab_name == NULL) { task_server_terminate(task, "kdc: Failed to set keytab name", diff --git a/source4/kdc/samba_kdc.h b/source4/kdc/samba_kdc.h index 2caefd58ae9..29d63237b15 100644 --- a/source4/kdc/samba_kdc.h +++ b/source4/kdc/samba_kdc.h @@ -66,6 +66,6 @@ struct samba_kdc_entry { NTSTATUS reject_status; }; -extern struct hdb_method hdb_samba4_interface; +extern struct hdb_method hdb_samba4_kpasswd_interface; #endif /* _SAMBA_KDC_H_ */ diff --git a/source4/kdc/wscript_build b/source4/kdc/wscript_build index 0c902f50534..0beb42c02b8 100644 --- a/source4/kdc/wscript_build +++ b/source4/kdc/wscript_build @@ -47,7 +47,7 @@ if bld.CONFIG_GET('SAMBA_USES_MITKDC'): internal_module=False) bld.SAMBA_LIBRARY('HDB_SAMBA4', - source='hdb-samba4.c hdb-samba4-plugin.c', + source='hdb-samba4.c hdb-samba4-kpasswd-plugin.c', deps='ldb auth4_sam common_auth samba-credentials hdb kdc db-glue samba-hostconfig com_err sdb_hdb RPC_NDR_WINBIND', includes=kdc_include, private_library=True, -- 2.35.0 From 1cae828c16d69b5db97161cbc25b1a44b0780de1 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:28:11 +1200 Subject: [PATCH 45/46] [TODO] note MIT plugin --- source4/kdc/mit-kdb/kdb_samba.c | 2 +- source4/kdc/mit-kdb/kdb_samba_principals.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/kdc/mit-kdb/kdb_samba.c b/source4/kdc/mit-kdb/kdb_samba.c index 0ff1bfe6c5c..3fc69d9a66d 100644 --- a/source4/kdc/mit-kdb/kdb_samba.c +++ b/source4/kdc/mit-kdb/kdb_samba.c @@ -153,7 +153,7 @@ kdb_vftabl kdb_function_table = { .lock = kdb_samba_db_lock, .unlock = kdb_samba_db_unlock, - .get_principal = kdb_samba_db_get_principal, + .get_principal = kdb_samba_db_get_principal, //// .put_principal = kdb_samba_db_put_principal, .delete_principal = kdb_samba_db_delete_principal, diff --git a/source4/kdc/mit-kdb/kdb_samba_principals.c b/source4/kdc/mit-kdb/kdb_samba_principals.c index 1fef1bbe58e..945dd88a833 100644 --- a/source4/kdc/mit-kdb/kdb_samba_principals.c +++ b/source4/kdc/mit-kdb/kdb_samba_principals.c @@ -301,7 +301,7 @@ static krb5_error_code ks_get_admin_principal(krb5_context context, return code; } -krb5_error_code kdb_samba_db_get_principal(krb5_context context, +krb5_error_code kdb_samba_db_get_principal(krb5_context context, /////////// krb5_const_principal princ, unsigned int kflags, krb5_db_entry **kentry) -- 2.35.0 From d475b8073ca2d8def43cbe889fb87717e8efa88b Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 20:03:32 +1200 Subject: [PATCH 46/46] [TODO] MIT knownfails --- selftest/knownfail_mit_kdc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc index 9b55627bbc8..af613401a65 100644 --- a/selftest/knownfail_mit_kdc +++ b/selftest/knownfail_mit_kdc @@ -433,3 +433,14 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_ ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_ts_rc4_not_protected.ad_dc ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_ts_rc4_protected.ad_dc ^samba.tests.krb5.protected_users_tests.samba.tests.krb5.protected_users_tests.ProtectedUsersTests.test_ts_rc4_protected_nested.ad_dc +# +# Kpasswd tests +# +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_canonicalize_realm_case.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_empty.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_initial.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_no_canonicalize.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_non_initial.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_ticket_requester_sid_tgs.ad_dc:local +^samba.tests.krb5.kpasswd_tests.samba.tests.krb5.kpasswd_tests.KpasswdTests.test_kpasswd_wrong_key_server.ad_dc:local -- 2.35.0