Commit Graph

30021 Commits

Author SHA1 Message Date
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
Jeffrey Altman
357a38fc7f lib/wind: find_normalize read past end of array
find_normalize() can under some circumstances read one element
beyond the input array.  The contents are discarded immediately
without further use.

This change prevents the unintended read.

Change-Id: Ia2759a5632d64f7fa6553f879b5bbbf43ba3513e
2021-11-17 20:03:03 -05:00
Nicolas Williams
8ed48bc54d gss-token: Fix exit code 2021-11-17 17:27:40 -06:00
Quanah Gibson-Mount
6415a2032e Fix #696 - Find python as part of the configure process instead of hard coding it.
Change-Id: I66d91f16d156d1a940f41ab16a049fb38f0e8bc4
2021-11-16 22:41:39 -05:00
Jeffrey Altman
d269c30b2b lib/asn1: all exported functions must use ASN1CALL convention
Otherwise, on 32-bit Windows there is a mismatch that and the
ESP register will not be populated correctly.

Prior to this change some exports were ASN1CALL and others
were not.   All of the tests assume ASN1CALL.

Change-Id: Icecff44aed4726b86100c939f64628d121bbd7ee
2021-11-16 22:09:16 -05:00
Jeffrey Altman
e27e056b45 asn1: use roken for generated source files
roken ensures the correct headers are used for each platform,
ensures availability of non-portable constants (e.g. ENOTSUP),
and on Windows enforces a consistent source for memory management.

Change-Id: I31aa2935d0af9f3d9529166679d9eff35ccedfad
2021-11-16 13:29:32 -05:00
Nicolas Williams
26df35d5f0 hdb: Fix test_namespace crash 2021-11-14 17:50:33 -06:00
Nicolas Williams
52e5cba08b Fix tests/check-kdc.in (fix 6d1e3c3d5) 2021-11-14 17:50:33 -06:00
Eric Hawicz
526317e80e Initialize local variable in kimpersonate to avoid crash in krb5_free_principal() 2021-11-13 09:13:49 +11:00
Nicolas Williams
2f31063e97 spnego: Minor style cleanup 2021-11-11 22:41:13 -06:00
Nicolas Williams
7a19658c1f spnego: Fix NULL deref
Reported to Heimdal by Michał Kępień <michal@isc.org>.

From the report:

Acknowledgement
---------------

This flaw was found while working on addressing ZDI-CAN-12302: ISC BIND
TKEY Query Heap-based Buffer Overflow Remote Code Execution
Vulnerability, which was reported to ISC by Trend Micro's Zero Day
Initiative.
2021-11-11 22:41:13 -06:00
Nicolas Williams
6cbe35ad5d kadm5: Fix crash in principal creation
This manifests with an upcomming patch that adds support for
aliasing of namespaces.
2021-10-30 15:21:54 -05:00
Nicolas Williams
66dabe7d39 hdb: Fix bug in hdb_clear_extension() 2021-10-30 15:21:54 -05:00
Nicolas Williams
831a5f9db3 hdb: Fix crash when expected KR is missing 2021-10-30 15:21:54 -05:00
Nicolas Williams
2a9b57cdad kadmin: Add command aliases to man page 2021-10-30 15:21:54 -05:00
Robert Crowston
5d462795ce Add stub for gss_acquire_cred_impersonate_name(). 2021-10-19 20:45:40 +11: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
a7f0b14f59 kdc: Fix check-pkinit UPN test misquoting 2021-10-15 14:00:11 -05:00
Luke Howard
a8bd9b8c72 hdb: update HDB_F_SYNTHETIC_OK description
Note that HDB_F_SYNTHETIC_OK is also used for GSS-API pre-authentication as
well as PKINIT.

Fixes: #812
2021-10-15 11:12:06 +11:00
Nicolas Williams
341848a27b base: Fix leak on ENOMEM 2021-10-11 13:58:15 -05: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
Nicolas Williams
54581d2d52 krb5: Fix PAC signature leak affecting KDC 2021-10-11 13:58:15 -05:00
Nicolas Williams
403a445f5b krb5: Document TGS HDB entry alias referral feature 2021-10-11 13:58:15 -05:00
Nicolas Williams
4e7c0fd129 kdc: Test referrals via HDB entry aliases
When a principal name is an alias of another in a different realm, the
KDC will return a referral to that realm.  Test that.
2021-10-11 13:58:15 -05:00
Nicolas Williams
ba98690a0a kadmin: Add add_alias, del_alias 2021-10-11 13:58:15 -05:00
Nicolas Williams
decd8f4102 hdb: Support referrals via aliases
The TGS will issue referrals based on [domain_realm] mappings.

With this change the TGS will also issue referrals based on HDB entry
aliases.

The TGS needed no changes for this, only support in lib/hdb was missing.

All we had to do was return HDB_ERR_WRONG_REALM from hdb_fetch_kvno()
when the given principal is an alias and its canonical name's realm is
different from the alias'.

This feature is important because the KDC currently does not re-read
krb5.conf and must be restarted for changes to e.g., [domain_realm]
mappings to take effect.  As well, making krb5.conf changes to all the
KDCs for a realm would need to be arranged.  But with aliases in the
HDB, these problems go away.

Relatedly, we should really have an option to store the KDC's entire
configuration in the HDB...

Futures:

 - Add support for aliasing of entire namespaces via HDB aliases with
   WELLKNOWN namespace name forms.  This will round out domain-to-realm
   mapping configuration support via HDB.
2021-10-11 13:58:15 -05:00