Skip to content

Commit

Permalink
kdc: add canonical principal name to authz data
Browse files Browse the repository at this point in the history
Use the UPN_DNS_INFO buffer of the PAC to include the canonical principal name.

Arguably we should use AD-LOGIN-ALIAS as defined in RFC6806, but we may not
always know all the principal's aliases, and this approach allows us to share
application service logic with Windows.
  • Loading branch information
lhoward committed Dec 19, 2021
1 parent cbceaab commit 0686b52
Show file tree
Hide file tree
Showing 10 changed files with 296 additions and 28 deletions.
2 changes: 1 addition & 1 deletion kdc/fast.c
Expand Up @@ -848,7 +848,7 @@ _kdc_fast_check_armor_pac(astgs_request_t r)
armor_client, r->armor_server,
r->armor_server, r->armor_server,
&r->armor_key->key, &r->armor_key->key,
&r->armor_ticket->ticket, &ad_kdc_issued, &mspac);
&r->armor_ticket->ticket, &ad_kdc_issued, &mspac, NULL);
if (ret) {
const char *msg = krb5_get_error_message(r->context, ret);
char *client_princ_name = NULL;
Expand Down
6 changes: 4 additions & 2 deletions kdc/kdc_locl.h
Expand Up @@ -84,8 +84,10 @@ struct astgs_request_desc {
krb5_keyblock session_key;

/* state */
krb5_principal client_princ;
hdb_entry_ex *client;
krb5_principal client_princ; /* AS: principal requested by client
* TGS: opt. canon principal from TGT PAC */
hdb_entry_ex *client; /* AS: client entry
* TGS: opt. client entry, if local to KDC */
HDB *clientdb;

krb5_principal server_princ;
Expand Down
31 changes: 31 additions & 0 deletions kdc/kerberos5.c
Expand Up @@ -1826,6 +1826,7 @@ generate_pac(astgs_request_t r, const Key *skey, const Key *tkey)
uint16_t rodc_id;
krb5_principal client;
krb5_boolean client_sent_pac_req, pac_request;
krb5_boolean will_canonicalize;

client_sent_pac_req =
(check_pa_pac_request(r->context, &r->req, &pac_request) == 0);
Expand Down Expand Up @@ -1860,6 +1861,36 @@ generate_pac(astgs_request_t r, const Key *skey, const Key *tkey)
return ret;
}

will_canonicalize = r->req.req_body.kdc_options.canonicalize ||
r->client->entry.flags.force_canonicalize;

/*
* For non-synthetic principals which match the realm of the issued
* ticket, include the canonical name in the authorization data.
* (If the realms don't match, then the KDC could impersonate any
* realm. Windows always canonicalizes the realm, but Heimdal permits
* aliases between realms.)
*/
if (!r->client->entry.flags.synthetic &&
(will_canonicalize ||
krb5_realm_compare(r->context, r->client_princ,
r->client->entry.principal))) {
char *cpn = NULL;

krb5_unparse_name(r->context, r->client->entry.principal, &cpn);
_kdc_audit_addkv((kdc_request_t)r, 0, "canon_client_name", "%s",
cpn ? cpn : "<unknown>");
krb5_xfree(cpn);

ret = _kdc_pac_add_canon_name_buffer(r->context, p,
r->client_princ,
r->client->entry.principal);
if (ret) {
krb5_pac_free(r->context, p);
return ret;
}
}

ret = _krb5_pac_sign(r->context, p, r->et.authtime,
client,
&skey->key, /* Server key */
Expand Down
68 changes: 62 additions & 6 deletions kdc/krb5tgs.c
Expand Up @@ -88,14 +88,17 @@ _kdc_check_pac(krb5_context context,
const EncryptionKey *krbtgt_check_key,
EncTicketPart *tkt,
krb5_boolean *kdc_issued,
krb5_pac *ppac)
krb5_pac *ppac,
krb5_principal *pac_canon_name)
{
krb5_pac pac = NULL;
krb5_error_code ret;
krb5_boolean signedticket;

*kdc_issued = FALSE;
*ppac = NULL;
if (pac_canon_name)
*pac_canon_name = NULL;

ret = _krb5_kdc_pac_ticket_parse(context, tkt, &signedticket, &pac);
if (ret)
Expand All @@ -118,7 +121,14 @@ _kdc_check_pac(krb5_context context,
/* Verify the KDC signatures. */
ret = _kdc_pac_verify(context, client_principal, delegated_proxy_principal,
client, server, krbtgt, &pac);
if (ret == KRB5_PLUGIN_NO_HANDLE) {
if (ret == 0) {
ret = _krb5_pac_get_upn_dns_info(context, pac, NULL, NULL,
pac_canon_name, NULL);
if (ret && ret != ENOENT) {
krb5_pac_free(context, pac);
return ret;
}
} else if (ret == KRB5_PLUGIN_NO_HANDLE) {
/*
* We can't verify the KDC signatures if the ticket was issued by
* another realm's KDC.
Expand All @@ -132,16 +142,36 @@ _kdc_check_pac(krb5_context context,
return ret;
}
}

ret = _krb5_pac_get_upn_dns_info(context, pac, NULL, NULL,
pac_canon_name, NULL);
if (ret && ret != ENOENT) {
krb5_pac_free(context, pac);
return ret;
}

/* Discard the PAC if the plugin didn't handle it */
krb5_pac_free(context, pac);
ret = krb5_pac_init(context, &pac);
if (ret)
return ret;
} else if (ret) {
} else {
krb5_pac_free(context, pac);
return ret;
}

/*
* Validate that the realm of the canonical name in the PAC matches
* the client name in the TGT, otherwise KDCs could masquerade realms.
*/
if (pac_canon_name && *pac_canon_name &&
!krb5_realm_compare(context, client_principal, *pac_canon_name)) {
krb5_free_principal(context, *pac_canon_name);
*pac_canon_name = NULL;
krb5_pac_free(context, pac);
return KRB5KDC_ERR_TGT_REVOKED;
}

*kdc_issued = signedticket ||
krb5_principal_is_krbtgt(context,
ticket_server->entry.principal);
Expand Down Expand Up @@ -787,6 +817,20 @@ tgs_make_reply(astgs_request_t r,
* is implementation dependent.
*/
if (mspac && !et.flags.anonymous) {
if (r->client_princ) {
char *cpn = NULL;

krb5_unparse_name(r->context, r->client_princ, &cpn);
_kdc_audit_addkv((kdc_request_t)r, 0, "canon_client_name", "%s",
cpn ? cpn : "<unknown>");
krb5_xfree(cpn);

ret = _kdc_pac_add_canon_name_buffer(r->context, mspac,
tgt_name, /* client from TGT */
r->client_princ); /* client from PAC */
if (ret)
goto out;
}

/* The PAC should be the last change to the ticket. */
ret = _krb5_kdc_pac_sign_ticket(r->context, mspac, tgt_name, serverkey,
Expand Down Expand Up @@ -1812,7 +1856,8 @@ tgs_build_reply(astgs_request_t priv,
/* Verify the PAC of the TGT. */
ret = _kdc_check_pac(context, config, user2user_princ, NULL,
user2user_client, user2user_krbtgt, user2user_krbtgt, user2user_krbtgt,
&uukey->key, &priv->ticket_key->key, &adtkt, &user2user_kdc_issued, &user2user_pac);
&uukey->key, &priv->ticket_key->key, &adtkt,
&user2user_kdc_issued, &user2user_pac, NULL);
_kdc_free_ent(context, user2user_client);
if (ret) {
const char *msg = krb5_get_error_message(context, ret);
Expand Down Expand Up @@ -1936,8 +1981,11 @@ tgs_build_reply(astgs_request_t priv,
flags &= ~HDB_F_SYNTHETIC_OK;
priv->client = client;

heim_assert(priv->client_princ == NULL, "client_princ should be NULL for TGS");

ret = _kdc_check_pac(context, config, cp, NULL, client, server, krbtgt, krbtgt,
&priv->ticket_key->key, &priv->ticket_key->key, tgt, &kdc_issued, &mspac);
&priv->ticket_key->key, &priv->ticket_key->key, tgt,
&kdc_issued, &mspac, &priv->client_princ);
if (ret) {
const char *msg = krb5_get_error_message(context, ret);
_kdc_audit_addreason((kdc_request_t)priv, "PAC check failed");
Expand Down Expand Up @@ -2171,6 +2219,9 @@ tgs_build_reply(astgs_request_t priv,
krb5_pac_free(context, mspac);
mspac = NULL;

krb5_free_principal(context, priv->client_princ);
priv->client_princ = NULL;

t = &b->additional_tickets->val[0];

ret = hdb_enctype2key(context, &client->entry,
Expand Down Expand Up @@ -2265,7 +2316,8 @@ tgs_build_reply(astgs_request_t priv,
* a S4U_DELEGATION_INFO blob to the PAC.
*/
ret = _kdc_check_pac(context, config, tp, dp, adclient, server, krbtgt, client,
&clientkey->key, &priv->ticket_key->key, &adtkt, &ad_kdc_issued, &mspac);
&clientkey->key, &priv->ticket_key->key, &adtkt,
&ad_kdc_issued, &mspac, &priv->client_princ);
if (adclient)
_kdc_free_ent(context, adclient);
if (ret) {
Expand Down Expand Up @@ -2567,6 +2619,10 @@ _kdc_tgs_rep(astgs_request_t r)
free(csec);
free(cusec);

if (r->client_princ) {
krb5_free_principal(r->context, r->client_princ);
r->client_princ = NULL;
}
if (r->armor_crypto) {
krb5_crypto_destroy(r->context, r->armor_crypto);
r->armor_crypto = NULL;
Expand Down
94 changes: 94 additions & 0 deletions kdc/windc.c
Expand Up @@ -250,3 +250,97 @@ kdc_get_instance(const char *libname)

return 0;
}

/*
* Add canonical name info buffer to PAC, if absent.
*/

#define UPN_DNS_INFO_EX_LENGTH 20

krb5_error_code
_kdc_pac_add_canon_name_buffer(krb5_context context,
krb5_pac mspac,
krb5_const_principal client_princ,
krb5_const_principal canon_princ)
{
krb5_error_code ret;
krb5_data upn_dns_info;
krb5_storage *sp = NULL;
char *client_princ_name = NULL;
char *canon_princ_name = NULL;
uint32_t flags;

krb5_data_zero(&upn_dns_info);

ret = krb5_pac_get_buffer(context, mspac, PAC_UPN_DNS_INFO,
&upn_dns_info);
if (ret != ENOENT)
goto out;

sp = krb5_storage_emem();
if (sp == NULL) {
ret = krb5_enomem(context);
goto out;
}

krb5_storage_set_flags(sp, KRB5_STORAGE_BYTEORDER_LE);

ret = krb5_unparse_name_flags(context, client_princ,
KRB5_PRINCIPAL_UNPARSE_DISPLAY,
&client_princ_name);
if (ret)
goto out;

ret = krb5_storage_truncate(sp, UPN_DNS_INFO_EX_LENGTH);
if (ret)
goto out;

ret = _krb5_store_utf8_as_ucs2le_at_offset(sp, (off_t)-1, client_princ_name);
if (ret)
goto out;

if (canon_princ) {
ret = krb5_unparse_name_flags(context, canon_princ,
KRB5_PRINCIPAL_UNPARSE_NO_REALM |
KRB5_PRINCIPAL_UNPARSE_DISPLAY,
&canon_princ_name);
if (ret)
goto out;

ret = _krb5_store_utf8_as_ucs2le_at_offset(sp, (off_t)-1,
canon_princ->realm);
if (ret)
goto out;
}

flags = PAC_EXTRA_LOGON_INFO_FLAGS_UPN_DEFAULTED;
if (canon_princ)
flags |= PAC_EXTRA_LOGON_INFO_FLAGS_HAS_SAM_NAME_AND_SID;

ret = krb5_store_uint32(sp, flags);
if (ret)
goto out;

if (canon_princ) {
ret = _krb5_store_utf8_as_ucs2le_at_offset(sp, (off_t)-1,
canon_princ_name);
if (ret)
goto out;
}

ret = krb5_storage_to_data(sp, &upn_dns_info);
if (ret)
goto out;

ret = krb5_pac_add_buffer(context, mspac, PAC_UPN_DNS_INFO,
&upn_dns_info);
if (ret)
goto out;

out:
krb5_xfree(canon_princ_name);
krb5_xfree(client_princ_name);
krb5_data_free(&upn_dns_info);

return ret;
}
7 changes: 7 additions & 0 deletions lib/krb5/krb5_locl.h
Expand Up @@ -479,6 +479,13 @@ struct krb5_decrypt_tkt_with_subkey_state {
/* Flag in KRB5_AUTHDATA_AP_OPTIONS */
#define KERB_AP_OPTIONS_CBT 0x00004000

#define PAC_SERVER_CHECKSUM 6
#define PAC_PRIVSVR_CHECKSUM 7
#define PAC_LOGON_NAME 10
#define PAC_CONSTRAINED_DELEGATION 11
#define PAC_UPN_DNS_INFO 12
#define PAC_TICKET_CHECKSUM 16

/* Flag in PAC_UPN_DNS_INFO, _krb5_pac_get_upn_dns_info() */
#define PAC_EXTRA_LOGON_INFO_FLAGS_UPN_DEFAULTED 0x1
#define PAC_EXTRA_LOGON_INFO_FLAGS_HAS_SAM_NAME_AND_SID 0x2
Expand Down
1 change: 1 addition & 0 deletions lib/krb5/libkrb5-exports.def.in
Expand Up @@ -844,6 +844,7 @@ EXPORTS
_krb5_expand_path_tokens ;!
_krb5_make_pa_enc_challenge
_krb5_validate_pa_enc_challenge
_krb5_store_utf8_as_ucs2le_at_offset

; kinit helper
krb5_get_init_creds_opt_set_pkinit_user_certs
Expand Down

0 comments on commit 0686b52

Please sign in to comment.