This is not a very good fix, though the warnings remain. Such errors
can in principle be a problem because in C there is no standard enum
sizing.
In this case we have two enums with the same elements and so the same
size, so it's clearly not a problem.
We have a Heimdal special where when the acceptor sends back an error
token for clock skew or ticket-not-yet-valid errors then the acceptor
application will get GSS_S_CONTINUE_NEEDED from gss_accept_sec_context()
so that the initiator may retry with the same context.
But we were retaining the auth_context, which means that when the
initiator does send a new token, the acceptor leaks memory because
krb5_verify_ap_req2() doesn't clean up the auth_context on reuse. The
end result is that we leak a lot in those cases.
The implementation of GSS name attributes for Kerberos (or any mechanism
with more than a tiny handful) is much nicer as a table-driven
implementation.
We now have stubs for setting and deleting attributes as well, though
these currently always fail.
The Heimdal KDC does not add a PAC if an anonymous ticket was issued. As such,
test_context should not expect PAC naming attributes to be present if the
--anonymous option was passed. (This is irrelevant for now as GSS_C_ANON_FLAG
is not honored by the krb5 mechanism.)
When unsigned char values are shifted, they are promoted to int (unless
sizeof(int) == sizeof(char)). This means that the change in be708ca3cf
ultimately leads to a sign extension bug.
The generated .x source and .hx header files are plain C source files.
Generate them as .c source files and avoid unnecessary file copying
and special makefile rules.
Change-Id: Ifc4bbe3c46dd357fdd642040ad964c7cfe1d395c
The LIB_ASN1 definition instructs the library objects to access
exported ASN1 generated DATA symbols as internal symbols.
Change-Id: Ia8c674c879c9bc46ca9dc7f249114f22b1d0dfd5
This adds Kerberos mechanism support for:
- composite principal name export/import
- getting rudimentary name attributes from GSS names using
gss_get_name_attribute():
- all (raw) authorization data from the Ticket
- all (raw) authorization data from the Authenticator
- transit path
- realm
- component count
- each component
- gss_inquire_name()
- gss_display_name_ext() (just for the hostbased service name type
though)
The test exercises almost all of the functionality, except for:
- getting the PAC
- getting authz-data from the Authenticator
- getting the transit path
TBD (much) later:
- amend test_context to do minimal name attribute checks as well
- gss_set_name_attribute() (to request authz-data)
- gss_delete_name_attribute()
- getting specific authorization data elements via URN fragments (as
opposed to all of them)
- parsing the PAC, extracting SIDs (each one as a separate value)
- some configurable local policy (?)
- plugin interface for additional local policy
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.
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.
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
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.
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
Correctly implement gss_krb5_ccache_name() in terms of
gss_set_sec_context_option(GSS_KRB5_CCACHE_NAME_X). The previous implementation
was a NOOP.
Note: global ccache name should really be thread-specific rather than global.
Closes#803.
We do not need to zero out the local variable output_token
if we do not later call gss_release_buffer() on it.
This aovids a -Werror=address compile failure under the
strict compiler options Samba uses when compiled on Ubuntu
20.04 with gcc version 9.3.0 (Ubuntu 9.3.0-17ubuntu1~20.04)
In file included from ../../source4/heimdal/lib/gssapi/preauth/pa_client.c:34:
../../source4/heimdal/lib/gssapi/preauth/pa_client.c:148:21: error: the address of ‘output_token’ will always evaluate as ‘true’ [-Werror=address]
148 | _mg_buffer_zero(&output_token);
| ^
../../source4/heimdal/lib/gssapi/mech/mech_locl.h:72:7: note: in definition of macro ‘_mg_buffer_zero’
72 | if (buffer) { \
| ^~~~~~
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
When importing a Kerberos name for GSS pre-auth, first try to import the name
as GSS_KRB5_NT_PRINCIPAL_NAME. If that fails, fall back to GSS_C_NT_USER_NAME.
The target (acceptor) name for GSS-API pre-authentication should be the name of
the TGS, not the server name in the AS-REQ, as it is the KDC which is being
mutually authenticated. If the client is not requesting a TGT, they may differ.
