On (02/03/16 10:02), Simo Sorce wrote: >On Wed, 2016-03-02 at 15:34 +0100, Lukas Slebodnik wrote: >> On (01/03/16 18:28), Simo Sorce wrote: >> >On Tue, 2016-03-01 at 18:22 -0500, Simo Sorce wrote: >> >> On Tue, 2016-03-01 at 22:34 +0100, Lukas Slebodnik wrote: >> >> > On (01/03/16 12:05), Simo Sorce wrote: >> >> > >On Tue, 2016-03-01 at 17:51 +0100, Lukas Slebodnik wrote: >> >> > >> On (01/03/16 17:45), Lukas Slebodnik wrote: >> >> > >> >On (31/01/16 11:53), Simo Sorce wrote: >> >> > >> >>Expired != Disabled >> >> > >> >>this change is intentional. >> >> > >> >> >> >> > >> >Yes, but explain it to Active directory :-) >> >> > >> > >> >> > >> >Attached is patch with workaround/hack >> >> > >> >regression with expired AD users. >> >> > >> > >> >> > >> ENOPATCH >> >> > >> >> >> > >> LS >> >> > > >> >> > >I think a better approach is to return the KRBKDC error from the child >> >> > >without mapping (or with an intermediate mapping) and have the IPA and >> >> > >AD providers map it on their own. >> >> > > >> >> > It's not related to mapping KRBKDC error codes to internal error code. >> >> > The main problem is that AD return the same error code for expired >> >> > and disabled user. And ad provider used generic krb5 functions. >> >> > >> >> > BTW the same issue would be with id_provider ldap + >> >> > auth_provider = krb5 with AD :-( >> >> > I'm not sure how your proposal would help. >> >> >> >> I think AD returns additional information in edata, maybe we can use >> >> that to do the proper mapping in the generic krb5 code. >> >> >> >> Absence of AD specific edata would indicate MIT mapping, presence would >> >> allow us to use that additional data to figure out the correct mapping. >> >> >> >> Simo. >> >> >> > >> >See MS-KILE[1] 2.2.1, I bet the two conditions returns two different >> >windows Style errors in etext (not edata, sorry). >> > >> >[1] https://msdn.microsoft.com/en-us/library/cc233855.aspx >> > >> Interesting idea and it seems to work. >> The main difference was in time and last octet string. >> * response for expired user [2] >> the last octet string in ASN: >> 930100C00000000001000000 > >This is error C0000193 aka NT_STATUS_ACCOUNT_EXPIRED NT_STATUS >https://git.samba.org/?p=samba.git;a=blob;f=libcli/util/ntstatus.h;h=572093b431d80c9e12461255d01063412bbfa5b5;hb=HEAD#l494 > >> * response for disabled user [3] >> the last octet string in ASN: >> 720000C00000000001000000T > >This is error C0000072, aka NT_STATUS_ACCOUNT_DISABLED NT_STATUS >https://git.samba.org/?p=samba.git;a=blob;f=libcli/util/ntstatus.h;h=572093b431d80c9e12461255d01063412bbfa5b5;hb=HEAD#l211 >> >> The only question is how to get etext from krb5 response. >> I do not want to implement ASN.1 parser. > >We use asn1c in FreeIPA if we need to generate a parser, but I do not >think we need to get that far. >The main issue is whether the krb5 fucntions let us get access to the >etext data at all, it seem buried pretty low in the internal of krb5. > It took me a while to find out why my patch broke password change then I realized I have unused argument in_tkt_service in my version of krb5_get_init_creds_password. We might enable gcc warning Wunused-argument :-)
Please review attached patch. LS
>From 8108259744e944b3521e8cf98105d791db82c402 Mon Sep 17 00:00:00 2001 From: root <r...@host1.kautest.com> Date: Wed, 2 Mar 2016 11:21:44 -0500 Subject: [PATCH] KRB5_CHILD: Discern between expired & disabled AD user Active directory return krb5 error code KRB5KDC_ERR_CLIENT_REVOKED "-1765328366/Clients credentials have been revoked" for expired and disabled user. This is difference between AD and IPA https://fedorahosted.org/sssd/ticket/2924. Therefore we need to distinguish between these two states. AD provides krb5 error data together with krb5 error code. They contains KERB-EXT-ERROR which is documeted in "[MS-KILE]: Kerberos Protocol Extensions"[1] and contains NTSTATUS in KERB-EXT-ERROR [1] https://msdn.microsoft.com/en-us/library/cc233855.aspx Related to: https://fedorahosted.org/sssd/ticket/2924 --- src/providers/krb5/krb5_child.c | 168 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 153 insertions(+), 15 deletions(-) diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index fff6a0a0cb186ef72350b5906b85455db561e4b8..06e3fa5e7d8c10b62e26c18a36c3d69877f54fb0 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -1191,6 +1191,144 @@ done: } +/* [MS-KILE]: Kerberos Protocol Extensions + * https://msdn.microsoft.com/en-us/library/cc233855.aspx + * http://download.microsoft.com/download/9/5/E/95EF66AF-9026-4BB0-A41D-A4F81802D92C/%5BMS-KILE%5D.pdf + * 2.2.1 KERB-EXT-ERROR + */ +bool have_ms_kile_ext_error(unsigned char *data, unsigned int length, + uint32_t *_ntstatus) +{ + /* [MS-KILE] 2.2.2 KERB-ERROR-DATA + * Kerberos V5 messages are defined using Abstract Syntax Notation One + * (ASN.1) + * KERB-ERROR-DATA ::= SEQUENCE { + * data-type [1] INTEGER, + * data-value [2] OCTET STRING OPTIONAL + * } + * We are interested in data-type 3 KERB_ERR_TYPE_EXTENDED + */ + uint8_t kile_asn1_begining[] = { + 0x30, 0x15, /* 0x30 is SEQUENCE, 0x15 length */ + 0xA1, 0x03, /* 0xA1 is 1st element of sequence, 0x03 length */ + 0x02, 0x01, 0x03, /* 0x02 is INTEGER, 0x01 length, 0x03 value */ + 0xA2, 0x0E, /* 0xA2 is 2nd element of sequence, 0x0E length */ + 0x04, 0x0C, /* 0x04 is OCTET STRING, 0x0C length (12 bytes) */ + }; + const size_t offset = sizeof(kile_asn1_begining); + uint32_t value; + + if (length != 23 || data == NULL) { + return false; + } + + if (memcmp(data, kile_asn1_begining, offset) != 0) { + return false; + } + + /* [MS-KILE] 2.2.1 KERB-EXT-ERROR + * typedef struct KERB_EXT_ERROR { + * unsigned long status; + * unsigned long reserved; + * unsigned long flags; + * } KERB_EXT_ERROR; + * Status: An NTSTATUS value. See [MS-ERREF] section 2.3. + */ + value = data[offset + 3] << 24 + | data[offset + 2] << 16 + | data[offset + 1] << 8 + | data[offset + 0]; + + *_ntstatus = value; + return true; +} + +/* Following NTSTATUS values are from: + * [MS-ERREF]: Windows Error Codes -> Section 2.3.1 + * https://msdn.microsoft.com/en-us/library/cc231196.aspx + * http://download.microsoft.com/download/9/5/E/95EF66AF-9026-4BB0-A41D-A4F81802D92C/%5BMS-ERREF%5D.pdf + */ +#define NT_STATUS_ACCOUNT_EXPIRED 0xC0000193 +#define NT_STATUS_ACCOUNT_DISABLED 0xC0000072 + +void check_ms_kile_ext_krb5err(krb5_context context, + krb5_init_creds_context init_cred_ctx, + krb5_error_code *_kerr) +{ + krb5_error_code err; + krb5_error *error = NULL; + uint32_t ntstatus; + + err = krb5_init_creds_get_error(context, init_cred_ctx, &error); + if (err != 0 || error == NULL) { + KRB5_CHILD_DEBUG(SSSDBG_TRACE_FUNC, err); + return; + } + + if (have_ms_kile_ext_error((unsigned char *)error->e_data.data, error->e_data.length, + &ntstatus)) { + switch (ntstatus) { + case NT_STATUS_ACCOUNT_EXPIRED: + *_kerr = KRB5KDC_ERR_NAME_EXP; + break; + case NT_STATUS_ACCOUNT_DISABLED: + *_kerr = KRB5KDC_ERR_CLIENT_REVOKED; + break; + } + } +} + +krb5_error_code +sss_krb5_get_init_creds_password(krb5_context context, krb5_creds *creds, + krb5_principal client, const char *password, + krb5_prompter_fct prompter, void *data, + krb5_deltat start_time, + const char *in_tkt_service, + krb5_get_init_creds_opt *k5_gic_options) +{ + krb5_error_code kerr; + krb5_init_creds_context init_cred_ctx = NULL; + + kerr = krb5_init_creds_init(context, client, prompter, data, + start_time, k5_gic_options, + &init_cred_ctx); + if (kerr != 0) { + KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr); + goto done; + } + + kerr = krb5_init_creds_set_password(context, init_cred_ctx, password); + if (kerr != 0) { + KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr); + goto done; + } + + if (in_tkt_service != NULL) { + kerr = krb5_init_creds_set_service(context, init_cred_ctx, + in_tkt_service); + if (kerr != 0) { + KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr); + goto done; + } + } + + kerr = krb5_init_creds_get(context, init_cred_ctx); + if (kerr == KRB5KDC_ERR_CLIENT_REVOKED) { + check_ms_kile_ext_krb5err(context, init_cred_ctx, &kerr); + } + + if (kerr != 0) { + KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr); + goto done; + } + + kerr = krb5_init_creds_get_creds(context, init_cred_ctx, creds); + +done: + krb5_init_creds_free(context, init_cred_ctx); + return kerr; +} + static krb5_error_code get_and_save_tgt(struct krb5_req *kr, const char *password) { @@ -1216,15 +1354,15 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr, DEBUG(SSSDBG_TRACE_FUNC, "Attempting kinit for realm [%s]\n",realm_name); - kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ, - discard_const(password), - sss_krb5_prompter, kr, 0, - NULL, kr->options); + kerr = sss_krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ, + discard_const(password), + sss_krb5_prompter, kr, 0, NULL, + kr->options); if (kr->pd->cmd == SSS_PAM_PREAUTH) { /* Any errors are ignored during pre-auth, only data is collected to * be send back to the client.*/ DEBUG(SSSDBG_TRACE_FUNC, - "krb5_get_init_creds_password returned [%d} during pre-auth.\n", + "sss_krb5_get_init_creds_password returned [%d} during pre-auth.\n", kerr); return 0; } else { @@ -1395,11 +1533,11 @@ static errno_t changepw_child(struct krb5_req *kr, bool prelim) DEBUG(SSSDBG_TRACE_FUNC, "Attempting kinit for realm [%s]\n",realm_name); - kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ, - discard_const(password), - prompter, kr, 0, - SSSD_KRB5_CHANGEPW_PRINCIPAL, - kr->options); + kerr = sss_krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ, + discard_const(password), + prompter, kr, 0, + SSSD_KRB5_CHANGEPW_PRINCIPAL, + kr->options); DEBUG(SSSDBG_TRACE_INTERNAL, "chpass is%s using OTP\n", kr->otp ? "" : " not"); if (kerr != 0) { @@ -1588,11 +1726,11 @@ static errno_t tgt_req_child(struct krb5_req *kr) } set_changepw_options(kr->options); - kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ, - discard_const(password), - sss_krb5_prompter, kr, 0, - SSSD_KRB5_CHANGEPW_PRINCIPAL, - kr->options); + kerr = sss_krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ, + discard_const(password), + sss_krb5_prompter, kr, 0, + SSSD_KRB5_CHANGEPW_PRINCIPAL, + kr->options); krb5_free_cred_contents(kr->ctx, kr->creds); if (kerr == 0) { -- 2.7.2
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
