From 861dc25f14a31f88020ff0ffd74bfe344361f4ea Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:17:02 +1200 Subject: [PATCH 01/44] 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 d5b54f78c69d6a6aa6ae7dc66cfa55d0501b6a4f Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 May 2022 19:17:41 +1200 Subject: [PATCH 02/44] s4:kpasswd: Add MIT fallback for decoding setpw structure The target principal and realm fields of the setpw structure are supposed to be optional, but in MIT Kerberos they are mandatory. For better compatibility and ease of testing, fall back to parsing the simpler (containing only the new password) structure if the MIT function fails to decode it. Although the target principal and realm fields should be optional, one is not supposed to specified without the other, so we don't have to deal with the case where only one is specified. 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 | 92 ++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 15 deletions(-) diff --git a/source4/kdc/kpasswd-service-mit.c b/source4/kdc/kpasswd-service-mit.c index 670210819a1..bd8b89b0950 100644 --- a/source4/kdc/kpasswd-service-mit.c +++ b/source4/kdc/kpasswd-service-mit.c @@ -28,6 +28,7 @@ #include "kdc/kpasswd_glue.h" #include "kdc/kpasswd-service.h" #include "kdc/kpasswd-helper.h" +#include "../lib/util/asn1.h" #define RFC3244_VERSION 0xff80 @@ -35,6 +36,52 @@ krb5_error_code decode_krb5_setpw_req(const krb5_data *code, krb5_data **password_out, krb5_principal *target_out); +/* + * A fallback for when MIT refuses to parse a setpw structure without the + * (optional) target principal and realm + */ +static bool decode_krb5_setpw_req_simple(TALLOC_CTX *mem_ctx, + const DATA_BLOB *decoded_data, + DATA_BLOB *clear_data) +{ + struct asn1_data *asn1 = NULL; + bool ret; + + asn1 = asn1_init(mem_ctx, 3); + if (asn1 == NULL) { + return false; + } + + ret = asn1_load(asn1, *decoded_data); + if (!ret) { + goto out; + } + + ret = asn1_start_tag(asn1, ASN1_SEQUENCE(0)); + if (!ret) { + goto out; + } + ret = asn1_start_tag(asn1, ASN1_CONTEXT(0)); + if (!ret) { + goto out; + } + ret = asn1_read_OctetString(asn1, mem_ctx, clear_data); + if (!ret) { + goto out; + } + + ret = asn1_end_tag(asn1); + if (!ret) { + goto out; + } + ret = asn1_end_tag(asn1); + +out: + asn1_free(asn1); + + return ret; +} + static krb5_error_code kpasswd_change_password(struct kdc_server *kdc, TALLOC_CTX *mem_ctx, struct auth_session_info *session_info, @@ -92,9 +139,10 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, const char **error_string) { krb5_context context = kdc->smb_krb5_context->krb5_context; + DATA_BLOB clear_data; krb5_data k_dec_data; - krb5_data *k_clear_data; - krb5_principal target_principal; + krb5_data *k_clear_data = NULL; + krb5_principal target_principal = NULL; krb5_error_code code; DATA_BLOB password; char *target_realm = NULL; @@ -113,29 +161,43 @@ static krb5_error_code kpasswd_set_password(struct kdc_server *kdc, code = decode_krb5_setpw_req(&k_dec_data, &k_clear_data, &target_principal); - if (code != 0) { - DBG_WARNING("decode_krb5_setpw_req failed: %s\n", - error_message(code)); - ok = kpasswd_make_error_reply(mem_ctx, - KRB5_KPASSWD_MALFORMED, - "Failed to decode packet", - kpasswd_reply); + if (code == 0) { + clear_data.data = (uint8_t *)k_clear_data->data; + clear_data.length = k_clear_data->length; + } else { + /* + * The MIT decode failed, so fall back to trying the simple + * case, without target_principal. + */ + ok = decode_krb5_setpw_req_simple(mem_ctx, + decoded_data, + &clear_data); if (!ok) { - *error_string = "Failed to create reply"; - return KRB5_KPASSWD_HARDERROR; + DBG_WARNING("decode_krb5_setpw_req failed: %s\n", + error_message(code)); + ok = kpasswd_make_error_reply(mem_ctx, + KRB5_KPASSWD_MALFORMED, + "Failed to decode packet", + kpasswd_reply); + if (!ok) { + *error_string = "Failed to create reply"; + return KRB5_KPASSWD_HARDERROR; + } + return 0; } - return 0; } ok = convert_string_talloc_handle(mem_ctx, lpcfg_iconv_handle(kdc->task->lp_ctx), CH_UTF8, CH_UTF16, - (const char *)k_clear_data->data, - k_clear_data->length, + clear_data.data, + clear_data.length, (void **)&password.data, &password.length); - krb5_free_data(context, k_clear_data); + if (k_clear_data != NULL) { + krb5_free_data(context, k_clear_data); + } if (!ok) { DBG_WARNING("String conversion failed\n"); *error_string = "String conversion failed"; -- 2.35.0 From b936e1055623f861837d1333e9691e3bb18c43fb Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:32:44 +1200 Subject: [PATCH 03/44] 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 4f6e41e754ea662af6ea95953f21db5e8c32e7de Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:37:48 +1200 Subject: [PATCH 04/44] 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 f09db1c284033230cac28ee7b9702aedc96bcde0 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:43:08 +1200 Subject: [PATCH 05/44] 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 98419da74da28ed8f4dece55925856254b2e898d Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:43:42 +1200 Subject: [PATCH 06/44] 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 62e369a5fc83e77b6c9943d79fea378433a67bef Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:43:58 +1200 Subject: [PATCH 07/44] 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 169de744518a8d83d938e36f564963f270770329 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 28 May 2022 17:44:32 +1200 Subject: [PATCH 08/44] 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 09/44] 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 10/44] 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 28f13b336ec083b9d56d020001e4725132ba723a Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:09:58 +1200 Subject: [PATCH 11/44] 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 57f15e7207bdf0278f84a5e6fcff16df76a3e2b4 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 20:52:04 +1200 Subject: [PATCH 12/44] 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 c08723e6748ffc36e95e26c7363e34d5430b03b9 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:06:53 +1200 Subject: [PATCH 13/44] 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 8cfe785c5ca90369f83209616ee6c5c52f6e4672 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:13:54 +1200 Subject: [PATCH 14/44] 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 8bf9f35351e4156a3de6fccd3eca2c38e1395742 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:17:45 +1200 Subject: [PATCH 15/44] 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 f18ba290a540403feac29d2e7da1b8d6c23cb299 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:21:37 +1200 Subject: [PATCH 16/44] 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 c8721b0be1f9d1c2c66b01984adce2bbcf0db14f Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:20:28 +1200 Subject: [PATCH 17/44] 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 73413971f1fd3b5de86efdf72fee26dce6e7acd9 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:26:56 +1200 Subject: [PATCH 18/44] 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 021f64f4bb08b178032bd9065536b690dbcec641 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:30:12 +1200 Subject: [PATCH 19/44] 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 fbd06a8d2a14b0f7d8cab37fd73cb4e972ee4906 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:34:59 +1200 Subject: [PATCH 20/44] 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 0d43a0ac510487c58cd3b111c09fd613964ed619 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:57:57 +1200 Subject: [PATCH 21/44] 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 af8a881bf7be8d05da49d3860c34acd8219b6704 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:35:03 +1200 Subject: [PATCH 22/44] 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 5419aadf5e61a4746089dbf7c7ae35b0eff441f2 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 19:59:16 +1200 Subject: [PATCH 23/44] 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 | 933 +++++++++++++++++++++++ python/samba/tests/krb5/raw_testcase.py | 8 + python/samba/tests/usage.py | 1 + source4/selftest/tests.py | 4 + 5 files changed, 949 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..42f02ffefc8 --- /dev/null +++ b/python/samba/tests/krb5/kpasswd_tests.py @@ -0,0 +1,933 @@ +#!/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_TGT_REVOKED, + 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=(KDC_ERR_TGT_REVOKED, + KDC_ERR_TKT_EXPIRED)) + + # 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 80d8872d0dc1678cd2218779f267fd8bd61e1ed7 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 17 May 2022 20:25:19 +1200 Subject: [PATCH 24/44] 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 5269948ea0bd805436daf0f24bf2e90d24919fd2 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:48:59 +1200 Subject: [PATCH 25/44] 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 8dfc7f532c3de765902b68b23d4a91a45eff7455 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:29:34 +1200 Subject: [PATCH 26/44] 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 d5eb0d489fe1846cd9a18e342e854a8012df6518 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:49:43 +1200 Subject: [PATCH 27/44] 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 a4c0d520dfb2696e4dfc076ef0815afa71a9d672 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Fri, 27 May 2022 19:21:06 +1200 Subject: [PATCH 28/44] 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 49ffd1ef183fad42b8d0d6ffac027a883479d62e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:06:31 +1200 Subject: [PATCH 29/44] 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..21f2f1e884e --- /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(const 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..d7b694dad0c --- /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(const 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 223486800df7428c672a545457d4e942a73de582 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:52:41 +1200 Subject: [PATCH 30/44] 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 198b0761a8fe4622a14c41e461bdc6df40b97a95 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 17:11:49 +1200 Subject: [PATCH 31/44] 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 2b6b2b3a54199042a93a8708f84ead84dde28e2c Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 24 May 2022 10:17:00 +0200 Subject: [PATCH 32/44] 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 6c1b1501b5c53b7377045fa2b4394f342c1f40a4 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 19 May 2022 16:35:28 +0200 Subject: [PATCH 33/44] 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 603febd60430cef63bc25ff1c83f0e6eafdf7cf9 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 24 May 2022 09:54:18 +0200 Subject: [PATCH 34/44] 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 f3c4d39aeb48b813a686824ffdd32d589cddc55e Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: [PATCH 35/44] 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 299324f97c346986ab6bf4803be7b522724349d2 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 25 May 2022 17:19:58 +1200 Subject: [PATCH 36/44] 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 0ae73b63ce352812a2424fe226fdebd66ecf0de5 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 18 May 2022 16:56:01 +1200 Subject: [PATCH 37/44] 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 44f1f229e981251cfd76a8a64415f777a78f9236 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:53:49 +1200 Subject: [PATCH 38/44] 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 dd58a86e63b5b2934f567bfa39eae92614ca6efc Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 May 2022 19:18:17 +1200 Subject: [PATCH 39/44] s4:kdc: Don't accept tickets living two minutes or less For Heimdal, this now 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. Windows never seems to look at the starttime of the ticket, only the authtime. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15049 Signed-off-by: Joseph Sutton --- source4/kdc/wdc-samba4.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c index d1825c2902c..2bf56339b03 100644 --- a/source4/kdc/wdc-samba4.c +++ b/source4/kdc/wdc-samba4.c @@ -415,6 +415,36 @@ static krb5_error_code samba_wdc_reget_pac(void *priv, astgs_request_t r, krbtgt = &signing_krbtgt_hdb; } } + } else { + /* + * We expect to have received a TGT, so check that we haven't + * been given a kpasswd ticket instead. + */ + krb5_ticket *tgt = kdc_request_get_ticket(r); + + KerberosTime starttime; + KerberosTime endtime; + KerberosTime lifetime; + + if (ticket->ticket.starttime != NULL) { + start_time = *ticket->ticket.starttime; + } else { + start_time = ticket->ticket.authtime; + } + + endtime = tgt->ticket.endtime; + + lifetime = rk_time_sub(endtime, start_time); + + if (lifetime <= 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 is not a ticket-granting ticket"); + return KRB5KRB_AP_ERR_TKT_EXPIRED; + } } ret = samba_wdc_reget_pac2(r, -- 2.35.0 From b07f65765a7a98f2d26f80a655a30802716b88fe Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Tue, 24 May 2022 17:52:05 +1200 Subject: [PATCH 40/44] s4:kdc: 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 f2ae71c1c4940cb00d805b1df6bf8e7d61116533 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Wed, 25 May 2022 20:00:55 +1200 Subject: [PATCH 41/44] 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 292d9e27430bc75ff85f2a5a4922fda14ca925d9 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:36:30 +1200 Subject: [PATCH 42/44] 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 a9bcb90e8d998eee9bb7cb60dd86bcf63564b056 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Thu, 26 May 2022 16:39:20 +1200 Subject: [PATCH 43/44] 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 967d86a2d89822349c60ab69e8b8988aec111aa3 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Mon, 30 May 2022 19:16:02 +1200 Subject: [PATCH 44/44] s4:kpasswd: Ensure we pass the kpasswd server principal into krb5_rd_req_ctx() To ensure that, when decrypting the kpasswd ticket, we look up the correct principal and don't trust the sname from the ticket, we should pass the principal name of the kpasswd service into krb5_rd_req_ctx(). However, gensec_krb5_update_internal() will pass in NULL unless the principal in our credentials is CRED_SPECIFIED. At present, our principal will be considered obtained as CRED_SMB_CONF (from the cli_credentials_set_conf() a few lines up), so we explicitly set the realm again, but this time as CRED_SPECIFIED. Now the value of server_in_keytab that we provide to smb_krb5_rd_req_decoded() will not be NULL. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15077 Signed-off-by: Joseph Sutton --- source4/kdc/kpasswd-service.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/source4/kdc/kpasswd-service.c b/source4/kdc/kpasswd-service.c index 96ac2670add..ce18ea36aa7 100644 --- a/source4/kdc/kpasswd-service.c +++ b/source4/kdc/kpasswd-service.c @@ -30,6 +30,7 @@ #include "kdc/kdc-server.h" #include "kdc/kpasswd-service.h" #include "kdc/kpasswd-helper.h" +#include "param/param.h" #define HEADER_LEN 6 #ifndef RFC3244_VERSION @@ -162,6 +163,20 @@ kdc_code kpasswd_process(struct kdc_server *kdc, goto done; } + /* + * After calling cli_credentials_set_conf(), explicitly set the realm + * with CRED_SPECIFIED. We need to do this so the result of + * principal_from_credentials() called from the gensec layer is + * CRED_SPECIFIED rather than CRED_SMB_CONF, avoiding a fallback to + * match-by-key (very undesirable in this case). + */ + ok = cli_credentials_set_realm(server_credentials, + lpcfg_realm(kdc->task->lp_ctx), + CRED_SPECIFIED); + if (!ok) { + goto done; + } + ok = cli_credentials_set_username(server_credentials, "kadmin/changepw", CRED_SPECIFIED); -- 2.35.0