Commit Graph

1907 Commits

Author SHA1 Message Date
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
Luke Howard
6b635f66de kdc: fix regression when validating armor client
Resolving the FAST armor client principal must use the same logic as the AS
itself. Allow synthetic client principals when validating FAST armor TGTs.
2021-12-16 16:05:07 +11:00
Joseph Sutton
b8c58191dc kdc: Optionally require that PAC be be present
This is from Samba's patches for CVE-2020-25719.

This allows Heimdal to match AD behaviour, when configured,
for the behaviour after Microsoft's CVE-2021-42287 when
PacRequestorEnforcement is set to 2.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14686
REF: https://support.microsoft.com/en-au/topic/kb5008380-authentication-updates-cve-2021-42287-9dafac11-e0d0-4cb8-959a-143bd0201041

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

[abarlet@samba.org based on Samba commit
 756934f14cc87dc1adfd9315672ae5d49cb24d95
 and f7a2fef8f49a86f63c3dc2f6a2d7d979fb53238a]
2021-12-16 14:41:10 +11:00
Joseph Sutton
83a80cd53b 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>
2021-12-16 12:59:01 +11:00
Joseph Sutton
caf7e173a4 kdc: Make check_PAC() and verify_flags() accessible to KDC code
This allows them to be used for checking FAST armor TGTs.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-16 12:59:01 +11:00
Joseph Sutton
c0fa930590 kdc: Don't advertise padata types that will not be accepted
Instead of always advertising PADATA-ENCRYPTED-CHALLENGE, we only
advertise it if FAST is in use.

We also check configuration options to only advertise
PADATA-ENC-TIMESTAMP if it will be accepted based on whether FAST is
currently in use. This permits the non-RFC behaviour required to avoid
breaking earlier Heimdal clients.

This incorporates changes from Apple Heimdal-597.140.2.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-16 10:49:15 +11:00
Nicolas Williams
4aa8677081 kadm5: Use KADM5_PASS_Q_GENERIC 2021-12-16 10:40:01 +11:00
Joseph Sutton
54129c319c kdc: Don't keep trying keys for encrypted-challenge if one decrypts but fails to verify
This also enables a KRB5KRB_AP_ERR_SKEW error to be returned to the
client.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-14 17:55:54 +11:00
Joseph Sutton
0ea840ebfc kdc: Check authdata in ticket rather than in request body
This matches Windows behaviour and the RFC6113 specification.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-14 17:36:10 +11:00
Nicolas Williams
660f875a34 kdc: Add [kdc] params to control PA-ENC-TIMESTAMP 2021-12-14 17:32:20 +11:00
Joseph Sutton
717ad8b043 kdc: Add support for explicit armoring from MS-KILE
Normally when FAST is used with a TGS-REQ, the armor key is implicitly
derived from the TGT rather than armor being explicitly present, as for
AS-REQs. However, Windows allows a TGS-REQ to be explicitly armored with
a computer's TGT, so that the armor key also depends on the ticket
session key.

This is used for compound identity, where the computer's group
membership and claims are added to the PAC of the resulting ticket.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-14 16:19:01 +11:00
Joseph Sutton
96ee28c32c kdc: Allow RODC-issued armor tickets
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-14 14:30:00 +11:00
Luke Howard
2087e07c1e kdc: update PAC hooks for Samba
Samba includes the user's long-term credentials (encrypted in the AS reply key)
to allow legacy authentication protocols such as NTLM to work even if the
pre-authentication mechanism replaced the reply key (as PKINIT does).

Samba also needs to know whether the client explicitly requested a PAC be
included (or excluded), in order to defer PAC exclusion until a service ticket
is issued (thereby avoiding a name binding attack if the user is renamed
between TGT and service ticket issuance).

References:

https://bugzilla.samba.org/show_bug.cgi?id=11441
https://bugzilla.samba.org/show_bug.cgi?id=14561

Closes: #864

Original authors:
 - Joseph Sutton <josephsutton@catalyst.net.nz>
 - Andrew Bartlett <abartlet@samba.org>
 - Stefan Metzmacher <metze@samba.org>
2021-12-14 13:51:53 +11:00
Joseph Sutton
2f21cf9933 kdc: Fix leak
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-14 13:24:02 +11:00
Luke Howard
d8af2eb730 kdc: don't leak armor crypto in TGS path
9b55215a added support for FAST in the TGS (#805) but forgot to free the armor
crypto context.
2021-12-14 12:12:40 +11:00
Luke Howard
007cc9fdc3 kdc: remove temporary krb5_context variable
Use r->context (from request) instead of a temporary context variable, where
available.
2021-12-14 09:03:42 +11:00
Luke Howard
2b95853df4 kdc: salt FAST cookie key with client name 2021-12-14 09:03:42 +11:00
Luke Howard
9b55215a2a kdc: sync KDC FAST with Heimdal-597.121.1
Import KDC FAST from Apple's Heimdal-597.121.1, adding support for:

  - PA-ENC-CHALLENGE
  - reply key strengthening
  - FAST authentication in TGS

kuser: Apple sync (squash)

krb5_init_creds_store_config/krb5_init_creds_warn_user in kinit
2021-12-14 09:03:42 +11:00
Luke Howard
c7a8ee1439 kdc: update windc header guard to reflect filename
Change HEIMDAL_KRB5_PAC_PLUGIN_H to HEIMDAL_KDC_WINDC_PLUGIN_H, reflecting
current plugin API file name.
2021-12-08 15:28:00 +11:00
Joseph Sutton
9b62d72d51 heimdal:kdc: Match Windows error code for unsupported critical FAST options
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-07 15:31:58 +11:00
Andrew Bartlett
7686028718 Use UTF-8 in KTH copyright notice
Samba is starting to protect against bi-di attacks and the starting point
is to require that input files be fully UTF-8.  In 2021 this is a reasonable
starting point anyway.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
2021-11-29 12:50:26 +11:00
Nicolas Williams
5f63215d0d Always perform == or != operation on cmp function result
Although not required to address bad code generation in
some versions of gcc 9 and 10, a coding style that requires
explicit comparison of the result to zero before use is
both clearer and would have avoided the generation of bad
code.

This change converts all use of cmp function usage from

```
    if (strcmp(a, b) || !strcmp(c, d)) ...
```

to

```
    if (strcmp(a, b) != 0 || strcmp(c, d)) == 0
```

for all C library cmp functions and related:

 - strcmp(), strncmp()
 - strcasecmp(), strncasecmp()
 - stricmp(), strnicmp()
 - memcmp()

Change-Id: Ic60c15e1e3a07e4faaf10648eefe3adae2543188
2021-11-24 22:30:44 -05:00
Andrew Bartlett
8ed36cee5c kdc: Fix ‘header_key’ may be used uninitialized in this function
krb5tgs.c: In function ‘_kdc_tgs_rep’:
krb5tgs.c:1785:25: warning: ‘header_key’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 1785 |       &tkey_check->key, &tkey_check->key, tgt, &kdc_issued, &mspac);
      |                         ^~~~~~~~~~~~~~~~
krb5tgs.c:2302:10: note: ‘header_key’ was declared here
 2302 |     Key *header_key;
      |          ^~~~~~~~~~

On Ubuntu 20.04 in a default Heimdal build with
gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04)

The compiler doesn't trust that *header_key = tkey; is always
executed in tgs_parse_request() for ret == 0.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
2021-11-24 02:51:12 -05:00
Jeffrey Altman
6cfbde4d86 plugin interface functions must specify calling convention
The plugin interfaces were originally implemented for use on
Unix where KRB5_CALLCONV, HEIM_CALLCONV and similar are defined
as nothing.  However, on 32-bit Windows the calling convention
matters and executing a __stdcall function through a __cdecl
function pointer will result in failures.

This change updates the krb5plugin_windc, krb5plugin_service_locate,
krb5plugin_send_to_kdc plugins to specify the KRB5_CALLCONV for
functions.   This brings the plugins into compliance with the
heim_plugin_common interface requirement that init() and fini()
use the platform specified HEIM_CALLCONV.

The krb5-plugin(7) man page is updated and the lib/krb5/test_plugin
test is also fixed.

With this change all tests pass on 32-bit Windows.

Change-Id: Ic9d2e1285c9c179e3898dc9d071ed092bcddc406
2021-11-18 18:52:54 -06:00
Robert Crowston
6d1e3c3d5b Fix spelling/grammar in various PKINIT messages
Only error messages and code comments touched.
2021-10-16 12:32:04 +11:00
Nicolas Williams
7672ad31db kdc: Fix leak and loss of kdc_check_flags() reason
We were losing and leaking the reason for which kdc_check_flags() was
rejecting any S4U requests, yielding incomplete error messages.

The issue is that kdc_check_flags() wants to check the client and server
principals in the input state structure, but doesn't know about
impersonated principal name, and so we want to pass it a state structure
that has the impersonated instead of the impersonator client name.  This
is a bad design, but I'm ignoring that for now and just fixing this one
leak.
2021-10-11 13:58:15 -05:00
Nicolas Williams
7e17db9f04 kdc: Fix leak on TGS referral 2021-10-11 13:58:15 -05:00
Luke Howard
e7863e2af9 kdc: correctly generate PAC TGS signature
When generating an AS-REQ, the TGS signature was incorrectly generated using
the server key, which would fail to validate if the server was not also the
TGS. Fix this.

Patch from Isaac Bourkis <iboukris@gmail.com>.
2021-09-23 17:51:51 +10:00
Luke Howard
fab07c4274 kdc: move _kdc_verify_checksum() to misc.c 2021-09-23 17:41:43 +10:00