diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 64962cc9a..7d47fb13e 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -983,8 +983,10 @@ static krb5_error_code ipadb_get_pac_attrs_blob(TALLOC_CTX *mem_ctx, #endif krb5_error_code ipadb_get_pac(krb5_context kcontext, - krb5_db_entry *client, unsigned int flags, + krb5_db_entry *client, + krb5_db_entry *server, + krb5_keyblock *replaced_reply_key, krb5_timestamp authtime, krb5_pac *pac) { @@ -1064,9 +1066,13 @@ krb5_error_code ipadb_get_pac(krb5_context kcontext, goto done; } - kerr = krb5_pac_init(kcontext, pac); - if (kerr) { - goto done; + /* krb5 1.20+ passes in a pre-created PAC structure but for previous + * versions we have to create it ourselves */ + if (pac != NULL && *pac == NULL) { + kerr = krb5_pac_init(kcontext, pac); + if (kerr) { + goto done; + } } data.magic = KV5M_DATA; @@ -1637,7 +1643,7 @@ static void filter_logon_info_log_message_rid(struct dom_sid *sid, uint32_t rid) static krb5_error_code check_logon_info_consistent(krb5_context context, TALLOC_CTX *memctx, - krb5_const_principal client_princ, + krb5_db_entry *client, krb5_boolean is_s4u, struct PAC_LOGON_INFO_CTR *info) { @@ -1645,7 +1651,6 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, struct ipadb_context *ipactx; bool result; bool is_from_trusted_domain = false; - krb5_db_entry *client_actual = NULL; struct ipadb_e_data *ied = NULL; int flags = 0; struct dom_sid client_sid; @@ -1701,18 +1706,12 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, } } - if (is_s4u && is_from_trusted_domain) { + if (client == NULL || (is_s4u && is_from_trusted_domain)) { /* If the PAC belongs to a user from the trusted domain, we cannot compare SIDs */ return 0; } - kerr = ipadb_get_principal(context, client_princ, flags, &client_actual); - if (kerr != 0) { - krb5_klog_syslog(LOG_ERR, "PAC issue: ipadb_get_principal failed."); - return KRB5KDC_ERR_TGT_REVOKED; - } - - ied = (struct ipadb_e_data *)client_actual->e_data; + ied = (struct ipadb_e_data *)client->e_data; if (ied == NULL || ied->magic != IPA_E_DATA_MAGIC) { krb5_klog_syslog(LOG_ERR, "PAC issue: client e_data fetching failed."); kerr = EINVAL; @@ -1748,8 +1747,6 @@ static krb5_error_code check_logon_info_consistent(krb5_context context, } done: - ipadb_free_principal(context, client_actual); - return kerr; } @@ -1980,7 +1977,7 @@ krb5_error_code filter_logon_info(krb5_context context, static krb5_error_code ipadb_check_logon_info(krb5_context context, - krb5_const_principal client_princ, + krb5_db_entry *client, krb5_boolean is_cross_realm, krb5_boolean is_s4u, krb5_data *pac_blob, @@ -1989,7 +1986,7 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context, struct PAC_LOGON_INFO_CTR info; krb5_error_code kerr; TALLOC_CTX *tmpctx; - krb5_data origin_realm = client_princ->realm; + krb5_data origin_realm = {0}; bool result; tmpctx = talloc_new(NULL); @@ -2051,7 +2048,7 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context, * is ours but operates on behalf of the cross-realm principal, we will * search through the trusted domains but otherwise skip the exact SID check * as we are not responsible for the principal from the trusted domain */ - kerr = check_logon_info_consistent(context, tmpctx, client_princ, is_s4u, &info); + kerr = check_logon_info_consistent(context, tmpctx, client, is_s4u, &info); goto done; } @@ -2224,24 +2221,19 @@ done: return kerr; } -krb5_error_code ipadb_verify_pac(krb5_context context, - unsigned int flags, - krb5_const_principal client_princ, - krb5_db_entry *proxy, - krb5_db_entry *server, - krb5_db_entry *krbtgt, - krb5_keyblock *server_key, - krb5_keyblock *krbtgt_key, - krb5_timestamp authtime, - krb5_authdata **authdata, - krb5_pac *pac) +krb5_error_code ipadb_common_verify_pac(krb5_context context, + unsigned int flags, + krb5_db_entry *client, + krb5_db_entry *server, + krb5_db_entry *signing_krbtgt, + krb5_keyblock *krbtgt_key, + krb5_timestamp authtime, + krb5_pac old_pac, + krb5_pac *pac) { - krb5_keyblock *srv_key = NULL; - krb5_keyblock *priv_key = NULL; krb5_error_code kerr; krb5_ui_4 *types = NULL; size_t num_buffers; - krb5_pac old_pac = NULL; krb5_pac new_pac = NULL; krb5_data data; krb5_data pac_blob = { 0 , 0, NULL}; @@ -2250,42 +2242,15 @@ krb5_error_code ipadb_verify_pac(krb5_context context, struct dom_sid *requester_sid = NULL; struct dom_sid req_sid; - kerr = krb5_pac_parse(context, - authdata[0]->contents, - authdata[0]->length, - &old_pac); - if (kerr) { - goto done; - } - - /* for cross realm trusts cases we need to check the right checksum. - * when the PAC is signed by our realm, we can always just check it - * passing our realm krbtgt key as the kdc checksum key (privsvr). - * But when a trusted realm passes us a PAC the kdc checksum is - * generated with that realm krbtgt key, so we need to use the cross - * realm krbtgt to check the 'server' checksum instead. */ - if (ipadb_is_cross_realm_krbtgt(krbtgt->princ)) { + if (signing_krbtgt != NULL && + ipadb_is_cross_realm_krbtgt(signing_krbtgt->princ)) { /* krbtgt from a trusted realm */ is_cross_realm = true; - - srv_key = krbtgt_key; - - } else { - /* krbtgt from our own realm */ - priv_key = krbtgt_key; } - /* only pass with_realm TRUE when it is cross-realm ticket and S4U - * extension (S4U2Self or S4U2Proxy (RBCD)) was requested */ - kerr = krb5_pac_verify_ext(context, old_pac, authtime, - client_princ, srv_key, priv_key, - (is_cross_realm && - (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION))); - if (kerr) { - goto done; - } - - /* Now that the PAC is verified, do additional checks. + /* In krb5 1.20 the PAC signatures are verified prior to call to issue_pac(). + * In krb5 before 1.20, we do verify PAC signatures before ipadb_common_verify_pac(). + * Now we can do additional checks. * Augment it with additional info if it is coming from a different realm */ kerr = krb5_pac_get_buffer(context, old_pac, KRB5_PAC_LOGON_INFO, &pac_blob); @@ -2302,7 +2267,7 @@ krb5_error_code ipadb_verify_pac(krb5_context context, #endif kerr = ipadb_check_logon_info(context, - client_princ, + client, is_cross_realm, (flags & KRB5_KDB_FLAGS_S4U), &pac_blob, @@ -2310,12 +2275,19 @@ krb5_error_code ipadb_verify_pac(krb5_context context, if (kerr != 0) { goto done; } - /* extract buffers and rebuilt pac from scratch so that when re-signing - * with a different cksum type does not cause issues due to mismatching - * signature buffer lengths */ - kerr = krb5_pac_init(context, &new_pac); - if (kerr) { - goto done; + + /* krb5 1.20+ passes in a pre-created PAC structure but for previous + * versions we have to create it ourselves */ + if (pac != NULL && *pac == NULL) { + /* extract buffers and rebuilt pac from scratch so that when re-signing + * with a different cksum type does not cause issues due to mismatching + * signature buffer lengths */ + kerr = krb5_pac_init(context, &new_pac); + if (kerr) { + goto done; + } + } else { + new_pac = *pac; } kerr = krb5_pac_get_types(context, old_pac, &num_buffers, &types); @@ -2334,7 +2306,6 @@ krb5_error_code ipadb_verify_pac(krb5_context context, pac_blob.length != 0) { kerr = krb5_pac_add_buffer(context, new_pac, types[i], &pac_blob); if (kerr) { - krb5_pac_free(context, new_pac); goto done; } @@ -2351,21 +2322,18 @@ krb5_error_code ipadb_verify_pac(krb5_context context, TALLOC_CTX *tmpctx = talloc_new(NULL); if (tmpctx == NULL) { kerr = ENOMEM; - krb5_pac_free(context, new_pac); goto done; } kerr = ipadb_client_requested_pac(context, old_pac, tmpctx, &pac_requested); if (kerr != 0) { talloc_free(tmpctx); - krb5_pac_free(context, new_pac); goto done; } kerr = ipadb_get_pac_attrs_blob(tmpctx, &pac_requested, &pac_attrs_data); if (kerr) { talloc_free(tmpctx); - krb5_pac_free(context, new_pac); goto done; } data.magic = KV5M_DATA; @@ -2375,7 +2343,6 @@ krb5_error_code ipadb_verify_pac(krb5_context context, kerr = krb5_pac_add_buffer(context, new_pac, PAC_TYPE_ATTRIBUTES_INFO, &data); if (kerr) { talloc_free(tmpctx); - krb5_pac_free(context, new_pac); goto done; } @@ -2395,22 +2362,23 @@ krb5_error_code ipadb_verify_pac(krb5_context context, krb5_free_data_contents(context, &data); } if (kerr) { - krb5_pac_free(context, new_pac); goto done; } } if (flags & KRB5_KDB_FLAG_CONSTRAINED_DELEGATION) { - if (proxy == NULL) { + if (client == NULL) { + if (new_pac != *pac) { + krb5_pac_free(context, new_pac); + } *pac = NULL; kerr = 0; goto done; } - kerr = ipadb_add_transited_service(context, proxy, server, + kerr = ipadb_add_transited_service(context, client, server, old_pac, new_pac); if (kerr) { - krb5_pac_free(context, new_pac); goto done; } } @@ -2418,8 +2386,9 @@ krb5_error_code ipadb_verify_pac(krb5_context context, *pac = new_pac; done: - krb5_free_authdata(context, authdata); - krb5_pac_free(context, old_pac); + if (kerr != 0 && (new_pac != *pac)) { + krb5_pac_free(context, new_pac); + } krb5_free_data_contents(context, &pac_blob); free(types); return kerr; diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_private.h b/daemons/ipa-kdb/ipa_kdb_mspac_private.h index 58b0ed8ff..5347b2355 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac_private.h +++ b/daemons/ipa-kdb/ipa_kdb_mspac_private.h @@ -59,18 +59,18 @@ void get_authz_data_types(krb5_context context, krb5_db_entry *entry, bool ipadb_is_cross_realm_krbtgt(krb5_const_principal princ); krb5_error_code ipadb_get_pac(krb5_context kcontext, - krb5_db_entry *client, unsigned int flags, + krb5_db_entry *client, + krb5_db_entry *server, + krb5_keyblock *replaced_reply_key, krb5_timestamp authtime, krb5_pac *pac); -krb5_error_code ipadb_verify_pac(krb5_context context, - unsigned int flags, - krb5_const_principal client_princ, - krb5_db_entry *proxy, - krb5_db_entry *server, - krb5_db_entry *krbtgt, - krb5_keyblock *server_key, - krb5_keyblock *krbtgt_key, - krb5_timestamp authtime, - krb5_authdata **authdata, - krb5_pac *pac); +krb5_error_code ipadb_common_verify_pac(krb5_context context, + unsigned int flags, + krb5_db_entry *client, + krb5_db_entry *server, + krb5_db_entry *signing_krbtgt, + krb5_keyblock *krbtgt_key, + krb5_timestamp authtime, + krb5_pac old_pac, + krb5_pac *pac); diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_v6.c b/daemons/ipa-kdb/ipa_kdb_mspac_v6.c index fc6ca890e..e506a0fd2 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac_v6.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac_v6.c @@ -31,6 +31,68 @@ #include "ipa_kdb_mspac_private.h" +static +krb5_error_code ipadb_verify_pac(krb5_context context, + unsigned int flags, + krb5_const_principal client_princ, + krb5_db_entry *proxy, + krb5_db_entry *server, + krb5_db_entry *krbtgt, + krb5_keyblock *server_key, + krb5_keyblock *krbtgt_key, + krb5_timestamp authtime, + krb5_authdata **authdata, + krb5_pac *pac) +{ + krb5_error_code kerr; + krb5_pac old_pac = NULL; + kerr = krb5_pac_parse(context, + authdata[0]->contents, + authdata[0]->length, + &old_pac); + if (kerr == 0) { + krb5_keyblock *srv_key = NULL; + krb5_keyblock *priv_key = NULL; + bool is_cross_realm = false; + + /* for cross realm trusts cases we need to check the right checksum. + * when the PAC is signed by our realm, we can always just check it + * passing our realm krbtgt key as the kdc checksum key (privsvr). + * But when a trusted realm passes us a PAC the kdc checksum is + * generated with that realm krbtgt key, so we need to use the cross + * realm krbtgt to check the 'server' checksum instead. */ + if (ipadb_is_cross_realm_krbtgt(krbtgt->princ)) { + /* krbtgt from a trusted realm */ + is_cross_realm = true; + srv_key = krbtgt_key; + } else { + /* krbtgt from our own realm */ + priv_key = krbtgt_key; + } + + /* only pass with_realm TRUE when it is cross-realm ticket and S4U + * extension (S4U2Self or S4U2Proxy (RBCD)) was requested */ + kerr = krb5_pac_verify_ext(context, old_pac, authtime, + NULL, srv_key, priv_key, + (is_cross_realm && + (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION))); + if (kerr) { + goto done; + } + + kerr = ipadb_common_verify_pac(context, flags, + proxy, server, krbtgt, + krbtgt_key, + authtime, old_pac, pac); + } + +done: + krb5_free_authdata(context, authdata); + krb5_pac_free(context, old_pac); + return kerr; +} + + static krb5_error_code ipadb_sign_pac(krb5_context context, unsigned int flags, @@ -239,7 +301,7 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, (void)ipadb_reinit_mspac(ipactx, force_reinit_mspac); - kerr = ipadb_get_pac(context, client, flags, authtime, &pac); + kerr = ipadb_get_pac(context, flags, client, server, NULL, authtime, &pac); if (kerr != 0 && kerr != ENOENT) { goto done; } @@ -253,7 +315,7 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, /* check or generate pac data */ if ((pac_auth_data == NULL) || (pac_auth_data[0] == NULL)) { if (flags & KRB5_KDB_FLAG_CONSTRAINED_DELEGATION) { - kerr = ipadb_get_pac(context, client_entry, flags, authtime, &pac); + kerr = ipadb_get_pac(context, flags, client_entry, server, NULL, authtime, &pac); if (kerr != 0 && kerr != ENOENT) { goto done; }