Remove hdb_entry_ex and revert to the original design of hdb_entry (except with
an additional context member in hdb_entry which is managed by the free_entry
method in HDB).
Decorate HDB_entry with context and move free_entry callback into HDB structure
itself. Requires updating hdb_free_entry() signature to include HDB parameter.
A follow-up commit will consolidate hdb_entry_ex (which has a single hdb_entry
member) into hdb_entry.
Move the Services for User (SFU/S4U) implementation -- protocol transition and
constrained delegation -- into its own compilation unit, with an interface that
only takes an astgs_request_t, so it can be easily factored out into a plugin
module in the future.
This refactoring is also careful to update all client names in the request
structure after the SFU/S4U validation has successfully completed.
Now heim_audit_addkv() correctly supports multiple values, the other functions
that (by design) replace existing values with a single value should use the
heim_audit_setkv prefix.
A subsequent commit could add variants that support multiple values for
non-string types.
Allow the windc plugin to also implement an audit callback. As part of this
change, both the HDB and windc audit function signatures are changed to return
void.
Active Directory/Samba-specific referral policy should be implemented by a
windc plugin, not in the main KDC code. It looks like it probably isn't
necessary at all if Samba wishes to bypass Heimdal's referral handling.
This reverts commit 6f0cafa6cf.
Add a referral policy hook to the TGS as a more elegant way of resolving
referral detection for Samba). The hook can either rewrite the server_princ in
the request, or it can return an error to disable built-in referral processing.
Preserve integer/boolean audit values as their native types; convert to strings
when logging only. This commit goes some way towards unifying the two auditing
APIs.
We're logging SUCCESS even when the KDC sends error replies. That's
because we're returning success to process_request() even when we send
errors to clients. The error we want to send to the client, and that we
succeed or fail to send it, are different statuses.
Also, further move things into `r` and out of function arguments.
Remove the outpadata field from astgs_request_t, because it's not something we
wish to expose publically (yet it is something that Samba needs in the
client_access plugin API, to add Windows error information).
Instead, allocate rep->padata at the start of AS/TGS request handling, and
ensure it is valid for the lifetime of the request until it is encoded (at
which point it will be freed and set to NULL if zero length, to avoid sending a
zero length METHOD-DATA to the client).
(The previous approach of setting rep->padata to point to &r->outpadata was
fragile, because it required clearing the pointer before freeing the KDC-REP.)
Make client_access plugin API take a single argument, astgs_request_t. Note: in
order to avoid making r->outpadata public (it's an internal buffer), but allow
Samba to modify the reply pa data, some pointer magic is required.
Add a helper function that returns TRUE if a PAC should be included in ticket
authorization data, that can be called from both AS and TGS paths.
Per [MS-KILE] 3.3.5.3, PACs are always included for TGTs; for service
tickets, policy is governed by whether the client explicitly requested
a PAC be omitted when requesting a TGT, or if the no-auth-data-reqd
flag is set on the service principal entry.
Remove the unwieldy pac_attributes_present field from the request structure.
Instead, if PAC attributes are not present in the TGT, the default is to assume
they were set to KRB5_PAC_WAS_GIVEN_IMPLICITLY.
PACs are included when issuing TGTs, if there is no PAC_ATTRIBUTES buffer
(legacy behavior) or if the attributes buffer indicates the AS client requested
one.
Only include the PAC_ATTRIBUTES PAC info buffer if we are issuing a TGT, as its
contents are not relevant to application services. (The buffer contains a bit
indicating whether the client explicitly or implicitly requested a PAC.)
Add PAC_ATTRIBUTES_INFO to the PAC. This info buffer indicates whether the user
explicitly requested a PAC be present or absent.
Note: this changes the windc plugin ABI.
Use the UPN_DNS_INFO buffer of the PAC to include the canonical principal name.
Arguably we should use AD-LOGIN-ALIAS as defined in RFC6806, but we may not
always know all the principal's aliases, and this approach allows us to share
application service logic with Windows.
08e0305b introduced a patch to validate armor ticket PACs, but required that
the armor client principal was in the local realm (as it did not allow
_kdc_db_fetch() to fail).
Allow cross-realm FAST armor clients by using the same logic to look up the
client principal as the TGS itself does, i.e. use db_fetch_client() which
handles the case where the client is not local to the KDC.
Note: the armor PAC is only required to be validated when issuing a final,
non-referral service ticket, as this is the only point upon which device claims
should be inserted (according to my understanding).
Closes: #901
Add a new authorization data type to indicate a synthetic principal was used,
to allow synthetic clients acquired outside of PKINIT (e.g. with GSS-API
pre-authentication) to use the TGS.
Note: we continue to honor KRB5_AUTHDATA_INITIAL_VERIFIED_CAS to indicate that
it is OK for the client to be synthetic, even though it is only an indication
that the client *may* have been synthetic.
This reverts commit 6f81e4c93b.
This patch may have originated in Samba from before force_canonicalize was
supported. Because the server_principal argument to tgs_make_reply() is
canonicalized prior to calling (if the server entry has force_canonicalize
set), honoring it again in tgs_make_reply() is superfluous.
This allows Samba to have a behaviour where
even if canonicalize flag is not set, the canonical realm
is returned, allowing a HDB module to require behaviour
that is the same as Windows.
Regression (for a HDB module wanting AD behaviour) was
introduced by commit:
378f34b4be
Signed-off-by: Isaac Boukris <iboukris@gmail.com>
[abartlet@samba.org Similar to Samba commit a9e6119ca0c2a78ef314c3162122539ee834aa04
but made conditional on server->entry.flags.force_canonicalize to
allow upstream submission]
Assists Samba to address CVE-2020-25719
It is critical to ensure that the name in the U2U TGT is still associated with
the account was issued to, so we must check the PAC to verify the SID.
Otherwise the SPN check via the S4U2Self hook might be mislead.
Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(Similar to Samba commit 49a13f0fc942d1cfb767d5b6bf49d62241d52046)
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
This may be contary to RFC4120 3.3.3 at
https://datatracker.ietf.org/doc/html/rfc4120/#section-3.3.3
(clearer in the GSS mechanism 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.
Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=14873
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(Similar to Samba commit fd50fecbe99ae4fc63843c796d0a516731a1fe6a)
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)
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=11441https://bugzilla.samba.org/show_bug.cgi?id=14561Closes: #864
Original authors:
- Joseph Sutton <josephsutton@catalyst.net.nz>
- Andrew Bartlett <abartlet@samba.org>
- Stefan Metzmacher <metze@samba.org>
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
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
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>
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.
