Commit Graph

1935 Commits

Author SHA1 Message Date
Luke Howard
f49e99e15f kdc: remove e_text argument to _kdc_encode_reply()
e_text is part of astgs_request_t, so it doesn't need to be an explicit
argument to _kdc_encode_reply()
2021-12-23 17:49:36 -06:00
Luke Howard
b6be850e0d kdc: remove outpadata from astgs_request_t
Remove the outpadata field from astgs_request_t, because it's not something we
wish to expose publically (yet it is something that Samba needs in the
client_access plugin API, to add Windows error information).

Instead, allocate rep->padata at the start of AS/TGS request handling, and
ensure it is valid for the lifetime of the request until it is encoded (at
which point it will be freed and set to NULL if zero length, to avoid sending a
zero length METHOD-DATA to the client).

(The previous approach of setting rep->padata to point to &r->outpadata was
fragile, because it required clearing the pointer before freeing the KDC-REP.)
2021-12-23 17:49:36 -06:00
Luke Howard
64dad876a4 kdc: simplify client_access windc plugin API
Make client_access plugin API take a single argument, astgs_request_t. Note: in
order to avoid making r->outpadata public (it's an internal buffer), but allow
Samba to modify the reply pa data, some pointer magic is required.
2021-12-23 17:49:36 -06:00
Luke Howard
36fe09f433 kdc: remove unused padata field from astgs_request_t 2021-12-23 17:49:36 -06:00
Luke Howard
e81bcb71f3 kdc: add finalize_reply API to windc plugin
Allow a windc plugin to finalize the KDC reply (including the encrypted ticket
and reply parts) before encoding for transmission.
2021-12-23 17:49:36 -06:00
Luke Howard
6b312659cb kdc: use PAC from request in _kdc_gss_finalize_pac()
Pass astgs_request_t to _kdc_gss_finalize_pac() in order to harmonize with
other functions.
2021-12-23 17:49:36 -06:00
Luke Howard
d3549c4ab7 kdc: remove superflous request arg to _kdc_check_access()
KDC_REQ is accessible as r->req
2021-12-23 17:49:36 -06:00
Luke Howard
805ea5e0a0 kdc: use rep/et/ek fields in astgs_request_t
Use rep/et/ek fields in astgs_request_t that were previously present but not
globally used.
2021-12-23 17:49:36 -06:00
Luke Howard
e3a5169862 kdc: make common astgs_request_t elements public API 2021-12-23 17:49:36 -06:00
Luke Howard
2e8b172f38 kdc: move PAC into astgs_request_t structure 2021-12-23 17:49:36 -06:00
Luke Howard
d95be72681 kdc: separate astgs_request_t common elements
Separate out some astgs_request_t common elements with the intention of making
them public API.
2021-12-23 17:49:36 -06:00
Luke Howard
5b7cf5d56f kdc: centralize include PAC logic
Add a helper function that returns TRUE if a PAC should be included in ticket
authorization data, that can be called from both AS and TGS paths.

Per [MS-KILE] 3.3.5.3, PACs are always included for TGTs; for service
tickets, policy is governed by whether the client explicitly requested
a PAC be omitted when requesting a TGT, or if the no-auth-data-reqd
flag is set on the service principal entry.
2021-12-23 13:52:15 +11:00
Luke Howard
317df4dbd4 kdc: remove pac_attributes_present field
Remove the unwieldy pac_attributes_present field from the request structure.

Instead, if PAC attributes are not present in the TGT, the default is to assume
they were set to KRB5_PAC_WAS_GIVEN_IMPLICITLY.
2021-12-23 13:26:40 +11:00
Luke Howard
ebd7092dff kdc: distinguish long-term key from replacing reply key
Distinguish pre-authentication mechanisms that replace the reply key from those
that use the client's long-term key. In the general case, one is the inverse of
the other, however KRB5_PADATA_ENCRYPTED_CHALLENGE replaces the reply-key with
one derived from the long-term key.

PA_REPLACE_REPLY_KEY indicates that the kvno should be set to zero in the
reply; the absence of PA_USES_LONG_TERM_KEY indicates that the client's
long-term key may be included in the PAC (PAC_CREDENTIAL_INFO with Samba only).

Corrects e8984871.
2021-12-23 11:24:35 +11:00
Luke Howard
bdb3e1020c Revert "kdc: distinguish long-term key from replacing reply key"
This reverts commit e89848719a.

(Reverting: typo.)
2021-12-23 11:24:00 +11:00
Luke Howard
e89848719a kdc: distinguish long-term key from replacing reply key
Distinguish pre-authentication mechanisms that replace the reply key from those
that use the client's long-term key. In the general case, one is the inverse of
the other, however KRB5_PADATA_ENCRYPTED_CHALLENGE replaces the reply-key with
one derived from the long-term key.

PA_REPLACE_REPLY_KEY indicates that the kvno should be set to zero in the
reply; the absence of PA_USES_LONG_TERM_KEY indicates that the client's
long-term key may be included in the PAC (PAC_CREDENTIAL_INFO with Samba only).
2021-12-23 11:21:30 +11:00
Luke Howard
357c73e013 kdc: avoid linear search of PA types
Store a dispatch table pointer for the used pre-authentication type to avoid
needing to traverse the table.
2021-12-23 11:04:38 +11:00
Luke Howard
f18c9e06a8 kdc: honor PAC_ATTRIBUTES in presented TGT
PACs are included when issuing TGTs, if there is no PAC_ATTRIBUTES buffer
(legacy behavior) or if the attributes buffer indicates the AS client requested
one.
2021-12-22 17:13:13 +11:00
Luke Howard
b4f3617964 kdc: don't add PAC_ATTRIBUTES PAC buffer for service tickets
Only include the PAC_ATTRIBUTES PAC info buffer if we are issuing a TGT, as its
contents are not relevant to application services. (The buffer contains a bit
indicating whether the client explicitly or implicitly requested a PAC.)
2021-12-22 16:38:52 +11:00
Luke Howard
b36ac3c15e kdc: don't add REQUESTOR_SID PAC buffer for service tickets
In the altsecid GSS pre-authentication authorizer plugin, don't add the
REQUESTOR_SID PAC info buffer unless the client requested a TGT. It is not
relevant to application services.
2021-12-22 16:37:32 +11:00
Luke Howard
8590499b53 kdc: include SID in PAC with GSS authorizer
Update the sample GSS pre-authentication authorizer plugin to allow the PAC to
be pinned to the authenticating user's SID.

There is still a race condition between the time the user authenticates and the
time the SID is looked up via LDAP, but it should be sufficient as an example;
if more security is required, then users should be enrolled with their SIDs.
2021-12-22 10:36:26 +11:00
Luke Howard
a2c225fe0a kdc: add PA PAC finalize callback
Note the selected pre-authentication mechanism, and add a callback to allow the
pre-authentication mechanism to update the PAC immediately prior to signing.
2021-12-22 10:36:26 +11:00
Luke Howard
f7964251ff kdc: support for PAC_ATTRIBUTES_INFO
Add PAC_ATTRIBUTES_INFO to the PAC. This info buffer indicates whether the user
explicitly requested a PAC be present or absent.

Note: this changes the windc plugin ABI.
2021-12-22 10:36:26 +11:00
Luke Howard
e50033aec2 kdc: add canonical principal name to authz data
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.
2021-12-22 10:36:26 +11:00
Joseph Sutton
f5823216d0 kdc: Return error code from _kdc_check_pac() with an armor ticket
This allows a TGT_REVOKED error to be returned to match Windows
behaviour.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-20 14:34:21 +11:00
Joseph Sutton
b16f93240e kdc: Fix NULL pointer dereference on failure to verify armor ticket PAC
r->client_princ and r->server_princ are only set in the AS-REQ case, but
we perform the PAC check in the TGS-REQ case, so calling
krb5_unparse_name() will dereference a NULL pointer. Instead, use
r->cname and r->sname.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-20 14:34:21 +11:00
Luke Howard
45ea54a44a kdc: audit armor client principal name 2021-12-19 21:06:08 +11:00
Luke Howard
a3392b099e kdc: allow cross-realm FAST armor TGT
08e0305b introduced a patch to validate armor ticket PACs, but required that
the armor client principal was in the local realm (as it did not allow
_kdc_db_fetch() to fail).

Allow cross-realm FAST armor clients by using the same logic to look up the
client principal as the TGS itself does, i.e. use db_fetch_client() which
handles the case where the client is not local to the KDC.

Note: the armor PAC is only required to be validated when issuing a final,
non-referral service ticket, as this is the only point upon which device claims
should be inserted (according to my understanding).

Closes: #901
2021-12-18 15:09:38 +11:00
Luke Howard
e7588952ce kdc: add auth data type for synthetic principals
Add a new authorization data type to indicate a synthetic principal was used,
to allow synthetic clients acquired outside of PKINIT (e.g. with GSS-API
pre-authentication) to use the TGS.

Note: we continue to honor KRB5_AUTHDATA_INITIAL_VERIFIED_CAS to indicate that
it is OK for the client to be synthetic, even though it is only an indication
that the client *may* have been synthetic.
2021-12-18 15:09:38 +11:00
Nicolas Williams
27d6526e3a kdc: Check unparse_flags() result differently 2021-12-18 11:34:12 +11:00
Nicolas Williams
90899b2361 kdc: parse_bytes() returns ssize_t 2021-12-18 11:34:12 +11:00
Luke Howard
fc76c83ab1 Revert "tgs-rep: always return canonical realm when force_canonicalize set"
This reverts commit 6f81e4c93b.

This patch may have originated in Samba from before force_canonicalize was
supported. Because the server_principal argument to tgs_make_reply() is
canonicalized prior to calling (if the server entry has force_canonicalize
set), honoring it again in tgs_make_reply() is superfluous.
2021-12-18 11:32:49 +11:00
Luke Howard
e0929d16b7 kdc: don't leak msg in pa_enc_chal_validate()
Move initialization of msg in pa_enc_chal_validate() to avoid leak on invalid
password.
2021-12-17 13:41:55 +11:00
Joseph Sutton
08e0305b26 kdc: Check PAC of armor tickets
Look up the client name of the armor TGT in the database, and check that
the PAC is valid.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

Closes: #891
2021-12-17 13:34:58 +11:00
Isaac Boukris
6f81e4c93b tgs-rep: always return canonical realm when force_canonicalize set
This allows Samba to have a behaviour where
even if canonicalize flag is not set, the canonical realm
is returned, allowing a HDB module to require behaviour
that is the same as Windows.

Regression (for a HDB module wanting AD behaviour) was
introduced by commit:
378f34b4be

Signed-off-by: Isaac Boukris <iboukris@gmail.com>

[abartlet@samba.org Similar to Samba commit a9e6119ca0c2a78ef314c3162122539ee834aa04
 but made conditional on server->entry.flags.force_canonicalize to
 allow upstream submission]
2021-12-17 13:28:59 +11:00
Luke Howard
5d92219788 kdc: set kvno to zero if reply key replaced
If the pre-authentication mechanism replaced the reply key, then the kvno in
the KDC-REP should be zero, as the reply is not encrypted in the client's
(versioned) long-term key.

Closes: #899
2021-12-17 13:27:31 +11:00
Andrew Bartlett
4a2e40a5b6 kdc: Use HDB_AUTHSTATUS_INVALID to mark that the status has not be set yet
-1 is unclear, so use a named constant.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
2021-12-17 13:19:52 +11:00
Andrew Bartlett
93deac696f hdb: Improve naming of constants for hdb_auth_status()
We drop the unused HDB_AUTH_INVALID_SIGNATURE and
rebase the set to start at an invalid 0.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
2021-12-17 13:19:52 +11:00
Joseph Sutton
49420aa3a1 kdc: Don't return PREAUTH_FAILED if encrypted challenge did not fail to decrypt
Instead we return ETYPE_NOSUPP.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-17 13:19:52 +11:00
Andrew Bartlett
842b856e4c kdc: Pass extra information to hdb_auth_status() to log success and failures
We now pass on the original client name and the client address to allow
consistent audit logging in Samba across multiple protocols.

We also log the authentication duration.

This is not a general purpose profiling solution, but in Smaba
these JSON logs are already being generated and stored, so this
is worth adding.

Some administrators are very keen to know how long authentication
takes, particularly due to long replication transactions in other
Samba processes.

We use config->db[0] to find the first database to record incorrect
users.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
(Similar to Samba commit f498ba77df2313e78863e5f2706840c43e232a96 and
 bb2a1c6b3eaccf114ac3f3b5b51f57828a04996)
[metze@samba.org: improved for heimdal upstream]
Signed-off-by: Stefan Metzmacher <metze@samba.org>
[abartlet@samba.org: improved again for Heimdal based on feedback]
2021-12-17 13:19:52 +11:00
Joseph Sutton
22515634cf kdc: Optionally allow missing additional ticket PAC for user-to-user
If the 'require_pac' option is not set, we should not require a PAC in
this situation.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-17 09:00:41 +11:00
Luke Howard
4dc369ef8f Revert "kdc: Check PAC of armor tickets"
This reverts commit 83a80cd53b.
2021-12-16 16:45:10 +11:00
Luke Howard
a208b9dcb7 Revert "kdc: fix regression when validating armor client"
This reverts commit 6b635f66de.
2021-12-16 16:44:48 +11:00
Joseph Sutton
91e86460cd kdc: Add krb5_is_enctype_old() to determine whether an enctype is older
AES256 and AES128 are newer enctypes because they are officially
specified in RFC4120 and RFC8009, while enctypes not officially
specified since RFC4120 are considered older. This function differs from
older_enctype() in that it does not report unknown or non-existent
enctypes as being 'newer'.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-16 16:11:29 +11:00
Joseph Sutton
87348cf27a kdc: Verify PAC in TGT provided for user-to-user authentication
Assists Samba to address CVE-2020-25719

It is critical to ensure that the name in the U2U TGT is still associated with
the account was issued to, so we must check the PAC to verify the SID.

Otherwise the SPN check via the S4U2Self hook might be mislead.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

(Similar to Samba commit 49a13f0fc942d1cfb767d5b6bf49d62241d52046)
2021-12-16 16:09:07 +11:00
Joseph Sutton
3e197ecbee kdc: Check name in request against name in user-to-user TGT
Assists Samba to address CVE-2020-25719

Again, this may be contary to RFC4120 3.3.3
https://datatracker.ietf.org/doc/html/rfc4120/#section-3.3.3
(clearer at the GSS spec here:
https://datatracker.ietf.org/doc/html/draft-swift-win2k-krb-user2user-03 )
as server-name is decribed as optional, however Windows AD and Samba
both require that the server-name exist and be a valid SPN matching
the provided TGT.

The lookup of SPN -> entry ensures that the SPN the client thought it
was connecting to was held by the target server. it could be the
typical user principal, or a service principal, but needs to be checked
for the client not to be fooled into connecting to the wrong service.

The check is the same as needed for S4U2Self so the same HDB hook is re-used.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873

(Similar to Samba commit f08e6ac86226dcd939fd0e40b6f7dc80c5c00e79)
2021-12-16 16:09:07 +11:00
Joseph Sutton
4112f6fc79 kdc: Use sname from request rather than user-to-user TGT client name
Assists Samba to address CVE-2020-25719

This may be contary to RFC4120 3.3.3 at
https://datatracker.ietf.org/doc/html/rfc4120/#section-3.3.3
(clearer in the GSS mechanism here:
https://datatracker.ietf.org/doc/html/draft-swift-win2k-krb-user2user-03 )
as server-name is decribed as optional, however Windows AD and Samba
both require that the server-name exist and be a valid SPN matching
the provided TGT.

The lookup of SPN -> entry ensures that the SPN the client thought it
was connecting to was held by the target server. it could be the
typical user principal, or a service principal, but needs to be checked
for the client not to be fooled into connecting to the wrong service.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

(Similar to Samba commit fd50fecbe99ae4fc63843c796d0a516731a1fe6a)
2021-12-16 16:09:07 +11:00
Joseph Sutton
b768c78fca kdc: Move fetching krbtgt entry to before enctype selection
Assists Samba to address CVE-2020-25719

This allows us to use it when validating user-to-user.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

(Similar to Samba commit f170f1eb4989d7f337eed0f45a558fe5231ea367)
2021-12-16 16:09:07 +11:00
Joseph Sutton
5cb5b6d748 kdc: Check return code
Assists Samba to address CVE-2020-25719

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

(cherry-picked from Samba commit a5db5c7fa2bdf5c651f77749b4e79c515d164e4f)
2021-12-16 16:09:07 +11:00
Joseph Sutton
ea8e8a4a8a kdc: Avoid races and multiple DB lookups in s4u2self check
Assists Samba to address CVE-2020-25719

Passing in target_server as a string principal means that for
an alias we must looking up the DB twice.

This is subject to a race and is a poor use of resources,
so instead just pass in the record we
already got when trying to confirm that the server in
S4U2Self is the same as the requesting client.

We also avoid doing a name comparison if the HDB plugin provides
a validation hook, this allows the HDB layer more freedom
to choose how to handle things.

In Samba AD the client record has already been bound to the the
original client by the SID check in the PAC, so the record is
known to match the ticket.

Likewise by looking up server only once we ensure that the
keys looked up originally (to decrypt) are in the record
we confirm the SID for here.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14686

Signed-off-by: Andrew Bartlett <abartlet@samba.org>

(Based on Samba commit 05898cfb139ae0674c8251acc9d64c4c3d4c8376)
2021-12-16 16:09:07 +11:00