Initialize the tgs_req buffer in init_tgs_req() so pointers are valid when
freed. Fixes regression introduced when Apple TGS-REQ FAST code was imported in
PR #805.
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]
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>
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)
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)
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>
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]
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>
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.
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
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.
This reverts commit 24a7a82e82.
After further discussion in #863, the alignment (which differs according to
info buffer type) should be handled by the caller.
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>
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>
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.
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.
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.
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?
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>
