Commit Graph

87 Commits

Author SHA1 Message Date
Taylor R Campbell
8f2f159f7f krb/pac.c: Sprinkle const on global data never changed. 2023-06-20 12:19:48 -05:00
Joseph Sutton
b90b219ab8 krb5: CVE-2022-42898 PAC parse integer overflows
Catch overflows that result from adding PAC_INFO_BUFFER_SIZE.

Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15203

Heavily edited by committer Nico Williams <nico@twosigma.com>, original by
Joseph Sutton <josephsutton@catalyst.net.nz>.

Signed-off-by: Nico Williams <nico@twosigma.com>
2022-11-15 17:51:45 -06:00
Joseph Sutton
7ae4292986 lib/krb5: Fix leak in error path
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-09-27 17:35:38 +10:00
Joseph Sutton
4a23cd5e23 lib/krb5: Make parameters to PAC functions 'const'
This allows these functions to be used with PACs obtained from KDC
accessor functions such as kdc_request_get_pac().

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-03-08 18:58:01 +11:00
Luke Howard
50fb794ef1 lib/krb5: re-allow data->length == 0 in krb5_pac_add_buffer()
PAC_TYPE_CLIENT_CLAIMS_INFO and PAC_TYPE_DEVICE_CLAIMS_INFO are
of zero length unless any claims are actually defined.

Signed-off-by: Stefan Metzmacher <metze@samba.org>

Closes: #969
2022-03-03 10:12:40 +11:00
Luke Howard
89cf441e8d Revert "lib/krb5: re-allow data->length == 0 in krb5_pac_add_buffer()"
This reverts commit f3301fc94c.
2022-03-03 10:11:32 +11:00
Stefan Metzmacher
f3301fc94c lib/krb5: re-allow data->length == 0 in krb5_pac_add_buffer()
PAC_TYPE_CLIENT_CLAIMS_INFO and PAC_TYPE_DEVICE_CLAIMS_INFO are
of zero length unless any claims are actually defined.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
2022-03-03 10:10:57 +11:00
Jeffrey Altman
543b94637f more dealloc functions require HEIM_CALLCONV
Change-Id: I68168a387c088b45e2572d5c982d33dfe0aa38a8
2022-01-29 00:15:59 -05:00
Jeffrey Altman
190263bb7a assert non-NULL ptrs before calling mem funcs
The definitions of memcpy(), memmove(), and memset() state that
the behaviour is undefined if any of the pointer arguments are
NULL, and some compilers are known to make use of this to
optimise away existing NULL checks in the source.

Change-Id: I489bc256e3eac7ff41d91becb0b43aba73dbb3f9
Link: https://www.imperialviolet.org/2016/06/26/nonnull.html
2022-01-24 00:07:51 -05:00
Nicolas Williams
a25cf6335e krb5: Fix ENOMEM path null deref in _krb5_pac_sign 2022-01-17 12:16:28 -06:00
Jeffrey Altman
5838fcaa61 lib/krb5: build_logon_name use common 'out' do not leak 's2'
CHECK(ret, func(), label) jumps to the label if the function
returns non-zero.  As a result it bypasses the free(s2) call.
Use the common out label to free(s2).

Change-Id: I9a79df669e6407a82991f4a849fc16a0afaae561
2022-01-16 19:13:26 -05:00
Luke Howard
7432589757 krb5: zeros is static, does not need explicit initializer 2022-01-17 10:05:59 +11:00
Luke Howard
867457871a krb5: make krb5_pac a heim_object_t
Make krb5_pac a heim_object_t and use heim_retain() (i.e. reference count +1)
as the copy constructor for the type decoration.

Note: this assumes that PACs included in naming attributes are immutable.
2022-01-15 18:54:57 +11:00
Luke Howard
5a952ee7b5 krb5: decorate PrincipalNameAttrs with krb5_pac
Add krb5_pac to PrincipalNameAttrs to avoid needing to re-parse it each time
gss_get_name_attribute() is called.
2022-01-15 18:54:57 +11:00
Nicolas Williams
768d193e01 krb5: Fix warnings (from 54581d2d) 2022-01-06 16:24:08 -06:00
Luke Howard
2a826d769f gss: expose PAC info buffers under urn:mspac:
Expose PAC info buffers using naming attributes prefixed with urn:mspac:,
aligned with MIT.
2022-01-03 15:59:16 +11:00
Luke Howard
c974f8625c krb5: PAC_EXTRA_LOGON_INFO_FLAGS_UPN_DEFAULTED fix
Only set the PAC_EXTRA_LOGON_INFO_FLAGS_UPN_DEFAULTED flag in the
UPN_DNS_INFO_EX PAC buffer, if the provided UPN matches the cname.

This is a NOOP at present, as no UPN is provided by the Heimdal KDC.
2021-12-22 16:37:24 +11:00
Luke Howard
f7964251ff kdc: support for PAC_ATTRIBUTES_INFO
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.
2021-12-22 10:36:26 +11:00
Luke Howard
e50033aec2 kdc: add canonical principal name to authz data
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.
2021-12-22 10:36:26 +11:00
Luke Howard
0ab3b7b2dd krb5: support for canonical name in PAC
If the UPN_DNS_INFO buffer in the Windows PAC contains a canonical principal
name, use it in lieu of the ticket client name to determine the GSS-API
initiator name.
2021-12-22 10:36:26 +11:00
Joseph Sutton
814e58fda8 heimdal: Make _krb5_pac_get_kdc_checksum_info() into a global function
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]
2021-12-14 13:44:01 +11:00
Luke Howard
eb85614c24 Revert "s4/heimdal/lib/krb5/pac.c: Align PAC buffers to match Windows"
This reverts commit 24a7a82e82.

After further discussion in #863, the alignment (which differs according to
info buffer type) should be handled by the caller.
2021-12-10 11:02:17 +11:00
Joseph Sutton
24a7a82e82 s4/heimdal/lib/krb5/pac.c: Align PAC buffers to match Windows
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14642

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from Samba commit 28a5a586c8e9cd155d676dcfcb81a2587ace99d1)
2021-12-09 09:53:03 +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
Nicolas Williams
54581d2d52 krb5: Fix PAC signature leak affecting KDC 2021-10-11 13:58:15 -05:00
Luke Howard
cba3f9a563 krb5: return KRB5KRB_AP_ERR_INAPP_CKSUM if PAC checksum fails
Return KRB5KRB_AP_ERR_INAPP_CKSUM instead of EINVAL when verifying a PAC, if
the checksum is absent or unkeyed.
2021-09-21 18:17:00 +10:00
Isaac Boukris
6df8be5091 krb5: rework PAC validation loop
Avoid allocating the PAC on error.

Closes: #836
2021-09-20 11:29:11 +10:00
Isaac Boukris
b295167208 krb5: allow NULL parameter to krb5_pac_free() 2021-09-20 11:29:09 +10:00
Isaac Boukris
2ffaba9401 kdc: sign ticket using Windows PAC
Split Windows PAC signing and verification logic, as the signing has to be when
the ticket is ready.

Create sign and verify the PAC KDC signature if the plugin did not, allowing
for S4U2Proxy to work, instead of KRB5SignedPath.

Use the header key to verify PAC server signature, as the same key used to
encrypt/decrypt the ticket should be used for PAC server signature, like U2U
tickets are signed witht the tgt session-key and not with the longterm key,
and so krbtgt should be no different and the header key should be used.

Lookup the delegated client in DB instead of passing the delegator DB entry.

Add PAC ticket-signatures and related functions.

Note: due to the change from KRB5SignedPath to PAC, S4U2Proxy requests
against new KDC will not work if the evidence ticket was acquired from
an old KDC, and vide versa.

Closes: #767
2021-09-19 13:25:27 +10:00
Chris Lamb
2f3f88e53a Correct "allignment" typos.
Signed-off-by: Chris Lamb <chris@chris-lamb.co.uk>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Garming Sam <garming@catalyst.net.nz>

(cherry-picked from Samba commit 8fcc49ecd44e69d7f9ada212c63896e0dcf488a4)
2018-09-10 14:42:18 -04:00
Simon Wilkinson
e50faea7f0 krb5: Store a digest context in the crypto structure
Creating and destroying an EVP_CTX_MD structure with every hash
operation is very expensive. Speed things up by caching one within
the krb5_crypto structure. krb5_crypto can already only be safely
used by one thread at a time - adding a message digest context here
shouldn't introduce any further threading risks.

Users of the stashed context must be careful to ensure that they
call no other hash functions whilst they are in the middle of using
the context.
2018-05-23 09:54:27 -04:00
Simon Wilkinson
ca756f0f7f krb5: Use iovecs for internal checksum handling
Modify the signature of the checksum operation in the
krb5_checksum_type structure so that it processes iovecs rather than
solid blocks of data.

Update all of the implementations of these functions for all of the
checksum types that we support so that they process iovecs, either
by iterating through the iovec in each function, or by calling
_krb5_evp_digest_iov or _krb5_evp_hmac_iov()

Update callers of these functions so that they turn their single blocks
of data into a single iovec of the correct type before calling checksum
2018-05-23 09:54:27 -04:00
Nicolas Williams
122cdc578e Fix some krb5_storage_write() usages 2017-05-26 23:24:30 -04:00
Nicolas Williams
f38089257b Misc fixes (coverity) 2016-11-20 17:43:51 -06:00
Nicolas Williams
953dc07391 Round #1 of scan-build warnings cleanup 2016-11-15 21:27:20 -06:00
Andrew Bartlett
2ff110cfa6 Merge pull request #154 from metze-samba/heimdal-for-upstream
lib/krb5: allow predefined PAC_{LOGON_NAME,PRIVSVR_CHECKSUM,SERVER_CH…
2016-05-14 22:32:13 +12:00
aaptel
08a2d1a320 lib/krb5/pac.c: fix NULL check typo 2016-03-10 10:33:53 +01:00
Stefan Metzmacher
7cd40a6105 lib/krb5: allow predefined PAC_{LOGON_NAME,PRIVSVR_CHECKSUM,SERVER_CHECKSUM} elements in _krb5_pac_sign()
A caller may want to specify an explicit order of PAC elements,
e.g. the PAC_UPN_DNS_INFO element should be placed after the PAC_LOGON_NAME
element.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
2016-02-02 15:31:18 +01:00
Stefan Metzmacher
ae4d222f58 lib/krb5: verify_logonname() to handle multi component principal
FreeIPA can generate tickets with a client principal of
'host/hostname.example.com'.

verify_logonname() should just verify the principal name
in the PAC_LOGON_NAME is the same as the principal of
the client principal (without realm) of the ticket.

Samba commit b7cc8c1187ff967e44587cd0d09185330378f366
break this. We try to compare ['host']['hostname.example.com']
with ['host/hostname.example.com]' (as we interpret it as enterprise principal)
this fail if we don't compare them as strings.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11142

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2015-07-31 17:30:23 +12:00
Stefan Metzmacher
8c8a39b0b7 heimdal:lib/krb5: let build_logon_name() use KRB5_PRINCIPAL_UNPARSE_DISPLAY
An ENTERPRISE principal should result in 'administrator@S4XDOM.BASE'
instead of 'administrator\@S4XDOM.BASE'.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11142
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
2015-06-17 17:41:27 -05:00
Stefan Metzmacher
2e6318f09a heimdal:lib/krb5: allow enterprise principals in verify_logonname()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=11142
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
2015-06-17 17:41:26 -05:00
Jeffrey Altman
5f138a16ef libkrb5: Add missing KRB5_LIB_FUNCTION/KRB5_LIB_CALL
KRB5_LIB_FUNCTION and KRB5_LIB_CALL are necessary even on private
functions that are exported.

Change-Id: Iccd0cfe87ff0a9d851e29890e9cb55b3ae517ce1
2013-06-22 21:17:32 -04:00
Harald Barth
3f52037382 Better error messages when UTF8 conversion fails
Signed-off-by: Love Hornquist Astrand <lha@h5l.org>
2013-04-26 00:05:53 -07:00
Viktor Dukhovni
435c02fa26 Compare pac timestamp to unix timestamp right when neither are set
Signed-off-by: Love Hornquist Astrand <lha@h5l.org>
2013-04-24 16:28:26 -07:00
Stefan Metzmacher
7ecbac23f6 lib/krb5: add utf8 support to build_logon_name() for the PAC
Pair-Programmed-With: Arvid Requate <requate@univention.de>

metze

Signed-off-by: Love Hornquist Astrand <lha@h5l.org>
2011-11-16 19:42:45 -08:00
Love Hornquist Astrand
0879b9831a remove trailing whitespace 2011-05-21 11:57:31 -07:00
Love Hornquist Astrand
f5f9014c90 Warning fixes from Christos Zoulas
- shadowed variables
- signed/unsigned confusion
- const lossage
- incomplete structure initializations
- unused code
2011-04-29 20:25:05 -07:00
Love Hornquist Astrand
03806492d9 prefix symbols that are _krb5_ structures 2011-03-12 13:45:09 -08:00
Love Hornquist Astrand
ea2534a55f export krb5_enomem 2010-11-24 14:35:56 -08:00
Asanka C. Herath
0f853405fe Add missing export and calling convention annotations 2010-11-24 15:32:49 -05:00