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?
'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,
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>
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
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>
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
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>
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.
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
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
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
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
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.
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.
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.
Deleting an alias causes the HDB_entry_alias entry value encoding to be
written to the iprop log, which later cannot be decoded as an HDB_entry.
Meanwhile, the alias is removed from the HDB but not from the list of
aliases in the canonical principal's HDB entry.
This commit makes deletion of alias names an error.
Sort of. It already knew.
We have a mess where new things get sent to the server as
KRB5_TL_EXTENSION, but old things get sent to the client as whatever
appropriate KRB5_TL we have, and... we call perform_tl_data() on all TL,
but we don't remove unmodified TL on the client side, and...
Anyways. This commit is a band-aid, but it works.
Fix _gss_spnego_set_sec_context_option() to return GSS_S_UNAVAILABLE if no
context handle is provided, so that mechglue will skip to the next mechanism.
There are no globally settable options on SPNEGO itself.
Fixes: #803