Commit Graph

2367 Commits

Author SHA1 Message Date
Joseph Sutton
2a4210b7e9 gsskrb5: CVE-2022-3437 Pass correct length to _gssapi_verify_pad()
We later subtract 8 when calculating the length of the output message
buffer. If padlength is excessively high, this calculation can underflow
and result in a very large positive value.

Now we properly constrain the value of padlength so underflow shouldn't
be possible.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
22749e918f gsskrb5: CVE-2022-3437 Check for overflow in _gsskrb5_get_mech()
If len_len is equal to total_len - 1 (i.e. the input consists only of a
0x60 byte and a length), the expression 'total_len - 1 - len_len - 1',
used as the 'len' parameter to der_get_length(), will overflow to
SIZE_MAX. Then der_get_length() will proceed to read, unconstrained,
whatever data follows in memory. Add a check to ensure that doesn't
happen.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
6a48779651 gsskrb5: CVE-2022-3437 Check buffer length against overflow for DES{,3} unwrap
Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
4aca82c7d0 gsskrb5: CVE-2022-3437 Check the result of _gsskrb5_get_mech()
We should make sure that the result of 'total_len - mech_len' won't
overflow, and that we don't memcmp() past the end of the buffer.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
ce6d8bbdbb gsskrb5: CVE-2022-3437 Avoid undefined behaviour in _gssapi_verify_pad()
By decrementing 'pad' only when we know it's safe, we ensure we can't
stray backwards past the start of a buffer, which would be undefined
behaviour.

In the previous version of the loop, 'i' is the number of bytes left to
check, and 'pad' is the current byte we're checking. 'pad' was
decremented at the end of each loop iteration. If 'i' was 1 (so we
checked the final byte), 'pad' could potentially be pointing to the
first byte of the input buffer, and the decrement would put it one
byte behind the buffer.

That would be undefined behaviour.

The patch changes it so that 'pad' is the byte we previously checked,
which allows us to ensure that we only decrement it when we know we
have a byte to check.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
cc9af5194a gsskrb5: CVE-2022-3437 Don't pass NULL pointers to memcpy() in DES unwrap
Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
e407e0ead6 gsskrb5: CVE-2022-3437 Use constant-time memcmp() in unwrap_des3()
The surrounding checks all use ct_memcmp(), so this one was presumably
meant to as well.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Joseph Sutton
e18b8f111f gsskrb5: CVE-2022-3437 Use constant-time memcmp() for arcfour unwrap
Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15134

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2022-11-15 17:51:45 -06:00
Nicolas Williams
bad07f7738 gss: Fix UB 2022-11-01 16:10:57 -05:00
Nicolas Williams
ea4b822af7 gss: Fix gss-token success exit code 2022-10-06 16:42:33 -05:00
Sergei Trofimovich
e7efa7783a gssapi: add dependency on gkrb5_err.h
Noticed missing target directory dependency as a build failure in
`make --shuffle` mode (added in https://savannah.gnu.org/bugs/index.php?62100):

      CC       test_common.o
    In file included from test_common.c:34:
    krb5/gsskrb5_locl.h:42:10: fatal error: gkrb5_err.h: No such file or directory
       42 | #include <gkrb5_err.h>
          |          ^~~~~~~~~~~~~
    compilation terminated.
    make[3]: *** [Makefile:2347: test_common.o] Error 1 shuffle=1656680590

The change moves gkrb5_err.h and friends to BUILT_SOURCES
to guarantee their presence when main build starts.
2022-09-16 16:13:50 -04:00
Daria Phoebe Brashear
133f517482 rewrite fallthrough to HEIM_FALLTHROUGH to deal with new Apple SDKs
Apple clang version 14.0.0 (clang-1400.0.17.3.1) fails the build
because stds.h defines `fallthrough` as a macro which is then
expanded when base.h evaluates

  # if __has_attribute(fallthrough) && __clang_major__ >= 5

The macOS SDK defines `DISPATCH_FALLTHROUGH` as the macro instead
of `fallthrough`.

This change replaces the use of `fallthrough` in the tree with
`HEIM_FALLTHROUGH` and updates the declaration in configure logic
to define `HEIM_FALLTHROUGH` based upon existing definitions
(if any) of `fallthrough` or `DISPATCH_FALLTHROUGH`.
2022-09-16 15:58:45 -04:00
Joseph Sutton
b19633f9b9 Use constant-time memcmp when comparing sensitive buffers
This helps to avoid timing attacks.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-04-30 13:35:52 -04:00
Luke Howard
69973757ce gss: remove gss_get_instance() 2022-01-30 14:20:05 -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
Jeffrey Altman
d35c9b2d67 lib/gssapi/ntlm: _gss_ntlm_inquire_cred dead code removal
do not check 'cred_handle' for GSS-C_NO_CREDENTIAL twice.

Change-Id: I3629aa49b2d20d3444c6ede46715d65b6072484f
2022-01-23 23:11:46 -05:00
Jeffrey Altman
ca4ff365f8 lib/gssapi/mech: gss_mech_switch do not leak 'm'
If there is a memory allocation failure after 'm'
is allocated, 'm' will be leaked; free it.

Change-Id: I625273634af207fac7c489df166cebde4d467cbc
2022-01-23 23:07:36 -05:00
Roland C. Dowdeswell
8dcd05ed4d _gss_ntlm_init_sec_context() mem leaks 2022-01-22 21:54:20 -05:00
Roland C. Dowdeswell
e87fca8091 _gss_ntlm_delete_sec_context() mem leaks 2022-01-22 21:54:20 -05:00
Roland C. Dowdeswell
8526b4c627 fix memory leak near NTLM type2 response 2022-01-22 21:54:20 -05:00
Jeffrey Altman
04527412e3 Follow the Linux kernel's lead on "fallthrough"
The pseudo keyword 'fallthrough' is defined such that case statement
blocks must end with any of these keywords:
 * break;
 * fallthrough;
 * continue;
 * goto <label>;
 * return [expression];
 *
 *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes

The macro is defined either as

  __attribute__((__fallthrough__))

or as

  do {} while (0)  /* fallthrough */

not including the semicolon.

This change implements the Linux kernel style and updates several locations
where "/*fallthrough*/ and /* FALLTHROUGH */ were not previously replaced.

Externally imported code such as libedit, libtommath and sqlite are
restored to their unaltered state.

Change-Id: I69db8167b0d5884f55d96d72de3059a0235a1ba3
2022-01-21 10:39:47 -05:00
Nicolas Williams
681708f416 gsskrb5: Fix coverity issue 2022-01-20 13:28:57 -06:00
Nicolas Williams
8c5030bcf7 gss: test_context: Fix leak 2022-01-19 12:35:08 -06:00
Nicolas Williams
26054d835c gss: Fix leak in gss-token 2022-01-19 12:33:11 -06:00
Nicolas Williams
f26bc69ded gss: Fix name attr leak in test_context 2022-01-18 12:35:26 -06:00
Nicolas Williams
0b137e3287 gss: Workaround valgrind "lifetime not equal" issue 2022-01-18 12:35:26 -06:00
Joseph Sutton
1c93a6ff26 heimdal: Avoid overflow when performing bitwise shift operations
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
2022-01-18 00:31:45 -05:00
Nicolas Williams
00dd104b96 gsskrb5: Fix dead code issues in deleg cred path 2022-01-17 18:10:08 -06:00
Nicolas Williams
82b8c906e9 gsskrb5: Fix incorrect allocation failure check 2022-01-17 18:00:51 -06:00
Nicolas Williams
77c048db9b gss: Check add_builtin() errors 2022-01-17 17:59:10 -06:00
Nicolas Williams
7fe5799090 gsskrb5: Fix dead code in get_transited() 2022-01-17 11:50:16 -06:00
Nicolas Williams
fe63ddc487 gsskrb5: Remove dead code in split_attr() 2022-01-17 11:01:19 -06:00
Luke Howard
dca1048e96 gss: use memset rather than {0} to initialize channel bindings 2022-01-17 22:18:44 +11:00
Luke Howard
7f2cf34b1d gss: _gss_negoex_accept make error const
krb5_get_error_message() returns a const char *, not a char *. Amends
Change-Id I464b3c5.
2022-01-17 15:46:45 +11:00
Luke Howard
ce0ba125d9 gss: _gss_negoex_init make error const
krb5_get_error_message() returns a const char *, not a char *. Amends
Change-Id I870ed0b.
2022-01-17 15:46:41 +11:00
Jeffrey Altman
ff18c32ae4 lib/gssapi/ntlm: from_file do not leak 'f' on error
Change-Id: Ica774bc3c156c384a2cf7084259d31f445d24a7d
2022-01-16 23:10:33 -05:00
Jeffrey Altman
6e8ab0c204 lib/gssapi/spnego: _gss_negoex_accept do not leak error message
Change-Id: I464b3c5e5b96b36da2cda71b1dacc8ad971fda35
2022-01-16 23:04:54 -05:00
Jeffrey Altman
ac53ce5c99 lib/gssapi/spnego: _gss_negoex_init do not leak error message
Change-Id: I870ed0bd8de7bc6ab5b8cf7c6d3816d04de354fa
2022-01-16 23:02:38 -05:00
Jeffrey Altman
8254c01ae7 lib/gssapi/spnego: accept_sec_context free 'supported_mechs'
If no preferred mechanism was found 'supported_mechs' was leaked.

Change-Id: I2982f94d7e9569461f562987609ff7bff57b3f88
2022-01-16 22:57:15 -05:00
Jeffrey Altman
c822b9bc96 gssapi/krb5: _gsskrb5_inquire_name init 'major'
If no attributes are indicated 'major' is unset.  Default to
GSS_S_UNAVAILABLE.

Change-Id: I277ebdebab0fb0322b702638c57548d1f4c4be3d
2022-01-16 15:54:47 -05:00
Jeffrey Altman
7ae24732c7 clang-format generated hdb, spnego and krb5 asn1.c files
Alphabetically sorted the $(spnego_files), $(gssapi_files), and $(gen_files_hdb)
lists.

Added rules to execute clang-format when available on the included files.

Change-Id: If3cde862f3237bc7cd100bc82d4fbbf568f1a354
2022-01-16 15:11:22 -05:00
Nicolas Williams
febdcd4cbd cf: Make clang-format style common makefile macro 2022-01-16 14:07:03 -06:00
Nicolas Williams
940aea6653 gss: clang-format ASN.1 compiler outputs 2022-01-16 14:07:03 -06:00
Jeffrey Altman
f341fa7721 prevent unintended sign extension errors
When an unsigned char is shifted << 24 bits its type will be
promoted to signed 32-bits.   If the value is then assigned to
an unsigned 64-bit value sign extension will occur.

Prevent the unwanted sign extension by explicitly casting the
value to unsigned long before shifting.

Change-Id: Iabeac0f17dc3229a2dc89abe71960a8ffbf523f8
2022-01-16 00:23:05 -05:00
Jeffrey Altman
7145a8e908 gssapi/mech: mech_locl.h roken.h must be included earlier
If included roken.h should be immediately following config.h.
Doing so ensures that all platform specific headers are
included in the proper order and avoids unnecessary includes
of headers managed by roken.h.

Change-Id: I27f11b42300b6ebcfbcc8d2c53915e96b6eec1d9
2022-01-15 21:24:10 -05: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
Jeffrey Altman
9f3004bfd5 gssapi/krb5: _gsskrb5_export_sec_context copy/paste error
80f3194a76
("gssapi/krb5/{export,import}_sec_context: make smaller tokens.")
stored the source principal when it should have stored the target
principal.

Change-Id: Ife6b137f9fe8f63cdb78b4212f74d502080ec2a2
2022-01-14 23:01:30 -05:00
Nicolas Williams
f076ed57cc Fix make dist 2022-01-14 20:10:19 -06:00
Nicolas Williams
55fa5bf7d2 gsskrb5: Fix warnings 2022-01-14 17:39:05 -06:00
Nicolas Williams
96b7ea671d gss: Fix warnings 2022-01-14 17:39:05 -06:00