Commit Graph

29897 Commits

Author SHA1 Message Date
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
Luke Howard
fcd8e33a98 tests: use KRB5_CALLCONV for windc test plugin 2021-12-14 13:51:53 +11:00
Joseph Sutton
814e58fda8 heimdal: Make _krb5_pac_get_kdc_checksum_info() into a global function
This lets us call it from Samba.

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

[abartlet@samba.org Similar to Samba commit 3bdce12789af1e7a7aba56691f184625a432410d
 but also fixed for caller in Heimdal windc plugin tests]
2021-12-14 13:44:01 +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
Joseph Sutton
f1255da03c krb5: Check asprintf return value
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-13 21:20:10 -05:00
Joseph Sutton
b8f8906822 asn1: Fix binary search off-by-one read
Previously, if left==right==A1_HEADER_LEN(tos), this would read past the
end of the template array. Now we treat [left, right) as a half-open
interval and no longer try to read from 'right'.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-13 19:18:24 -06: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
f95f4bc99e krb5: require strengthen_key when FAST + GSS PA
Always require a strengthen key to be used if GSS pre-authentication is used
within FAST. TRhis avoids a MITM attack that could result in unintended
privilege escalation, should a KDC add additional positive authorization data
from the armor ticket to the issued ticket.

An upcoming revision of the draft will reflect this.
2021-12-14 09:03:42 +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
22c7e85c57 kinit: add --kdc-hostname option 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
47282cae34 krb5: import Heimdal-597.121.1 AS/TGS client
Sync with most changes in AS/TGS client from Apple's Heimdal-597.121.1
(opensource.apple.com).

Changes include:

 - FAST support in TGS client
 - Refactored pre-auth client to be more easily extensible
 - Pin KDC host and AD site name in API calls

Note the completely refactored TGS client loop is not imported as that was
considered too intrusive.
2021-12-14 09:03:42 +11:00
Nicolas Williams
b5a58df8eb krb5: Document return of krb5_cc_get_config() 2021-12-10 17:20:05 -06:00
Nicolas Williams
f44596b14b krb5: Fix null deref in krb5_init_creds_free() 2021-12-10 17:20:05 -06:00
Luke Howard
cfa49a461c krb5: do not pack ccapi on Apple Silicon
Fix fb6f89f2 so #pragma(pop) is also similarly guarded to exclude ARM
2021-12-10 11:11:16 +11:00
Luke Howard
eb85614c24 Revert "s4/heimdal/lib/krb5/pac.c: Align PAC buffers to match Windows"
This reverts commit 24a7a82e82.

After further discussion in #863, the alignment (which differs according to
info buffer type) should be handled by the caller.
2021-12-10 11:02:17 +11:00
Joseph Sutton
24a7a82e82 s4/heimdal/lib/krb5/pac.c: Align PAC buffers to match Windows
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14642

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from Samba commit 28a5a586c8e9cd155d676dcfcb81a2587ace99d1)
2021-12-09 09:53:03 +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
Joseph Sutton
527906c821 heimdal:kdc: Properly check for unsupported critical FAST options
Decoding a FAST request will only give us the FastOptions flags that are
explicitly declared in the ASN1 source. This meant that the check for
unsupported mandatory options would never succeed, and an unsupported
option would go undetected.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-07 15:31:58 +11:00
Joseph Sutton
d2dc61c720 heimdal: Initialise KDC reply
The reply structure was not being zero-initialised in all cases, leading
to crashes or possible heap corruption on error paths when we later
freed it.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2021-12-07 15:03:22 +11:00
Nicolas Williams
83625d349e gss: Make initiator inq. ctx. return canon. target
Make gss_inquire_context() on the established context on the initiator
side return the canonical target acceptor name.
2021-12-06 17:39:22 -06:00
Nicolas Williams
5ace5f5a6a gss: Fix dst TGT deleg w/o dns_lookup_realm
Setting `dns_lookup_realm = false` in `[libdefaults]` and setting name
canon rules that force the empty realm causes destination-TGT delegation
to break because the client doesn't know the service's realm.

Because MIT and Heimdal check that the (unauthenticated plaintext)
sname/realm of the Ticket in the KDC reply matches the sname/srealm in
the enc-part of the KDC reply, we know we can trust the realm of the
ticket found in the ccache.  So use that.
2021-12-06 17:39:22 -06:00
Nicolas Williams
bba573f286 krb5: Fix dst TGT deleg w/o dns_lookup_realm
Setting `dns_lookup_realm = false` in `[libdefaults]` and setting name
canon rules that force the empty realm causes destination-TGT delegation
to break because the client doesn't know the service's realm.

Because MIT and Heimdal check that the (unauthenticated plaintext)
sname/realm of the Ticket in the KDC reply matches the sname/srealm in
the enc-part of the KDC reply, we know we can trust the realm of the
ticket found in the ccache.  So use that.
2021-12-06 17:39:22 -06:00
Luke Howard
eb293680a8 gss: fix regression in non-8003 checksums
Samba3 sends an AP-REQ, rather than 8003, checksum in a Kerberos inital context
token. This regressed in #835 as we forgot to set the
KRB5_CRYPTO_FLAG_ALLOW_UNKEYED_CHECKSUM flag before processing the AP-REQ
checksum in this path.
2021-12-07 10:41:40 +11:00
Nicolas Williams
6f7c6a7f67 krb5: Fix out-of-tree SQLite3 ccache perms issue
SQLite3 defaults to 0644 unless overridden, relying on the process'
umask to make that tighter.

Our in-tree SQLite3 uses 0600 as the permissions for DB files it
creates.

Out-of-tree builds of SQLite3 probably get the 0644 default.

We can't change the umask in libraries -- it's not thread-safe.

So this commit changes the SCC ccache type's default ccname to include
an intermediate directory which is created with `mkdir(2)` with
permissions set to 0700, then it chmods the DB file to 0644.
2021-11-30 11:34:04 -06:00
Nicolas Williams
a025788a37 krb5: Make test_cc run keyctl new_session 2021-11-30 11:42:00 -05:00
Nicolas Williams
90db9b96a4 krb5: Make test_cc not step on user ccaches
And cleanup on exit.
2021-11-30 11:42:00 -05:00
Nicolas Williams
6918322c79 krb5: Fix FILE ccache my_basename() bug 2021-11-30 11:42:00 -05:00
Nicolas Williams
bacc484b2a krb5: Fix umask issue with SQLite3 2021-11-30 11:42:00 -05:00
Nicolas Williams
aeac1186c8 sqlite: Be thread-safe on Windows too 2021-11-30 11:42:00 -05:00
Nicolas Williams
250eee7acf sqlite: Use 0600
The in-tree SQLite3 is used for HDB and ccache -- these should have
0600 permissions.

Of course, if we're using an out-of-tree SQLite3, then we need to rely
on the process' umask, but we use SQLite3 in libraries, where we can't
set the umask...

What to do?

TBD:

 - On Windows: nothing to do.

 - On Unix: if SQLite3 is out of tree then [v]fork() to create the
   connection then close, then connect again after??

   Or... maybe make sure to create an intermediate directory with 0700
   permissions?
2021-11-30 11:42:00 -05:00
Nicolas Williams
beae9c3c43 roken: Use ptsname_r() if we have it 2021-11-30 11:42:00 -05:00
Nicolas Williams
ed6f3f1786 autoconf: Remove unused tests 2021-11-30 11:42:00 -05:00
Nicolas Williams
c84384c544 krb5: Fix doxygen comments 2021-11-30 11:42:00 -05:00
Nicolas Williams
63034f36ae base: Fix doxygen comments 2021-11-30 11:42:00 -05:00
Luke Howard
8a54096266 hx509: revert UTF-8 change to hx509 test data
76860287 changed all KTH copyrights to use UTF-8, but that broke test_cms.
Revert that commit as applied to the test data.
2021-11-30 12:12:47 +11:00
Romain Fihue
2f0c985b47 Revert "KCM wrong size memcmp"
'uuid' is seen as an 'unsigned char*', thus '*uuid' is an 'unsigned char' where size is 1.

This solves a problem where two KCM ccaches's uuid have the same first byte hides each other.

What we observe:
  * A user cannot discover tickets with (klist -l) but can access it with it's name
  * The 'rpc.gssd' daemon is doing the same kind of pattern but using GSS calls (gss_acquire_cred)

Whet GDB told us:
  * The 'kcm_ccache_get_uuids' is okay, all ccache are really present
  * The 'kcm_ccache_resolve_by_uuid' is buggy, it only compare the first byte of each uuid.
    Which may be the same as the one we're seeking. Selected ccache that will be, most probably, filtered-out afterward with a call to 'kcm_access'.
    This leads to 'KRB5_FCC_NOFILE' errors while the uuid is correct.

Similar calls may be present.

This reverts commit 936017e4d6,
2021-11-29 10:26:19 -05: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
Jeffrey Altman
02200d55ea Address GCC Bug 95189 memcmp wrongly stripped like strcmp
As documented in Russell O'Connor's blog, Heimdal when compiled with
some versions of gcc 9 and 10 would generate incorrect behaviors from
_gssapi_verify_mic_arcfour(), _gssapi_unwrap_arcfour(),
_gssapi_unwrap_iov_arcfour() and _gssapi_unwrap_iov_arcfour().

As a result of the bug, code of the form

   if (memcmp(a, "\x00\x00\x00\x00"))

and

   cmp = memcmp(a, "\x00\x00\x00\x00")

will be compiled as if it were written as

   if (strcmp(a, "\x00\x00\x00\x00"))

and

   cmp = strcmp(a, "\x00\x00\x00\x00")

but not

   if (memcmp(a, "\x00\x00\x00\x00") != 0)

and

   cmp = (memcmp(a, "\x00\x00\x00\x00") != 0)

Bad code is generated whenever one of the parameters to memcmp()
is a constant with at least one NUL in the first four octets and
the return value is used immediated without a boolean comparison.

The gcc bug 95189 has since been fixed.

This change applies a defensive programming technique to avoid
the broken code generation.

Change-Id: I1db2a561735317cb6cac66a0ec9caf5443e65e03
Link: https://r6.ca/blog/20200929T023701Z.html
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189
Reported-by: Buck Huppmann (buckh@pobox.com) <buckh@pobox.com>
2021-11-24 22:30:44 -05:00
Jeffrey Altman
8123ffc3f2 _gssapi_unwrap_iov_arcfour remove duplicate code block
ad3acc2aef ("lib/gssapi/krb5:
implement gss_[un]wrap_iov[_length] with arcfour-hmac-md5")
introduced a duplicate block of code.

This change removes the unnecessary code duplication.

Change-Id: I557c590aea51b73dc25d6ce4be4ea84b9dfadd9f
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
Nicolas Williams
232c936ea3 asn1: Work around missing ENOTSUP (WIN32)
Sufficiently old CRTs on Windows lack ENOTSUP.  Use EINVAL instead then.
2021-11-23 18:12:45 -06:00
Nicolas Williams
92e5a4b7e5 Revert "asn1: use roken for generated source files"
This reverts commit e27e056b45.

e27e056b45 was needed mainly for ENOTSUP.
ENOTSUP is not available in older C run-times.

Also, lib/roken has wrappers for the CRT allocator, but we don't need
those in lib/asn1 because all the functions generated by the compiler
effectively encapsulate the corresponding DLL's CRT's allocator.

This will be followed by a change to not use ENOTSUP.
2021-11-23 18:12:45 -06: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