Commit Graph

17 Commits

Author SHA1 Message Date
Adam Langley
faa539f877 Remove static output buffers for hash & HMAC functions.
These static output buffers are a legacy from a time before processes
had threads. This change drops support and callers who were depending on
this (of which there are hopefully none) will crash.

Change-Id: I7b8eb3440def507f92543e55465f821dfa02c7da
Reviewed-on: https://boringssl-review.googlesource.com/14528
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-04-02 17:53:17 +00:00
David Benjamin
17cf2cb1d2 Work around language and compiler bug in memcpy, etc.
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html

Add OPENSSL_memcpy, etc., wrappers which avoid this problem.

BUG=23

Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-21 20:34:47 +00:00
David Benjamin
54091230cd Use C99 for size_t loops.
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left
everything in crypto/x509 and friends alone.

There's some instances in gcm.c that are non-trivial and pulled into a
separate CL for ease of review.

Change-Id: I6515804e3097f7e90855f1e7610868ee87117223
Reviewed-on: https://boringssl-review.googlesource.com/10801
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-12 19:44:24 +00:00
David Benjamin
582d2847ed Reimplement PKCS#12 key derivation.
This is avoids pulling in BIGNUM for doing a straight-forward addition on a
block-sized value, and avoids a ton of mallocs. It's also -Wconversion-clean,
unlike the old one.

In doing so, this replaces the HMAC_MAX_MD_CBLOCK with EVP_MAX_MD_BLOCK_SIZE.
By having the maximum block size available, most of the temporary values in the
key derivation don't need to be malloc'd.

BUG=22

Change-Id: I940a62bba4ea32bf82b1190098f3bf185d4cc7fe
Reviewed-on: https://boringssl-review.googlesource.com/7688
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-19 18:16:38 +00:00
Piotr Sikora
d7421ebf6c Remove condition which always evaluates to true (size_t >= 0).
Found with -Wtype-limits.

Change-Id: I5580f179425bc6b09ff2a8559fce121b0cc8ae14
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/6463
Reviewed-by: Adam Langley <agl@google.com>
2015-11-11 22:20:19 +00:00
Adam Langley
c4737331a3 Fix typo in |HMAC_CTX_cleanup|.
This was part of https://boringssl-review.googlesource.com/#/c/2710,
but that got lost because I was gumption-trapped by the number of
changes in x509/ that I didn't really want to make.

Change-Id: Iaf5bc8bcc2e3cfbb1b37aa477462ee8f824135db
Reviewed-on: https://boringssl-review.googlesource.com/5440
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 20:38:54 +00:00
David Benjamin
af8731f7a5 Remove HMAC_CTX_set_flags.
It's never called externally and for good reason; the only flag to set is
EVP_MD_CTX_FLAG_NO_INIT which is an implementation detail of EVP_PKEY_HMAC
(hopefully to be removed eventually). Indeed, only EVP_PKEY_HMAC ever calls
this function. Except there's no need to because the HMAC_CTX has already been
initialized at that point. (And were it not initialized, that call would not
bode well for the poor HMAC_CTX.)

The legacy EVP_PKEY_HMAC API has test coverage and still works after this
change.

Change-Id: I2fb0bede3c24ad1519f9433f957606de15ba86c7
Reviewed-on: https://boringssl-review.googlesource.com/4970
Reviewed-by: Adam Langley <agl@google.com>
2015-06-02 01:07:07 +00:00
David Benjamin
2c3679210e EVP_Digest*Update, EVP_DigestFinal, and HMAC_Update can never fail.
Enough code fails to check their return codes anyway. We ought to make
it official.

Change-Id: Ie646360fd7073ea943036f5e21bed13df7e1b77a
Reviewed-on: https://boringssl-review.googlesource.com/4954
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 22:17:10 +00:00
Brian Smith
8983a1da7b Remove gotos from HMAC code.
Change-Id: Ic17257e65207ada658f781f4b35ec0cf75bb5474
Reviewed-on: https://boringssl-review.googlesource.com/4151
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:54:26 +00:00
David Benjamin
23721e3705 Forbid reusing HMAC key without reusing the hash function.
There's no good reason to do this, and it doesn't work; HMAC checks the length
of the key and runs it through the hash function if too long. The reuse occurs
after this check.

This allows us to shave 132 bytes off HMAC_CTX as this was the only reason it
ever stored the original key. It also slightly simplifies HMAC_Init_ex's
logic.

Change-Id: Ib56aabc3630b7178f1ee7c38ef6370c9638efbab
Reviewed-on: https://boringssl-review.googlesource.com/3733
Reviewed-by: Adam Langley <agl@google.com>
2015-03-02 23:16:12 +00:00
David Benjamin
cc239d3903 Use HMAC_Init_ex, not HMAC_Init, in HMAC.
We've already initialized the context, HMAC_Init has questionable behavior
around NULL keys, and this avoids a size_t truncation.

Change-Id: Iab6bfc24fe22d46ca4c01be6129efe0630d553e6
Reviewed-on: https://boringssl-review.googlesource.com/3732
Reviewed-by: Adam Langley <agl@google.com>
2015-03-02 23:15:20 +00:00
Adam Langley
2b2d66d409 Remove string.h from base.h.
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.

This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.

Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-02 19:14:15 +00:00
David Benjamin
ba90d7c56e Test HMAC_CTX initial state and remove now unneccessary code.
The special-case in HMAC is no longer needed. Test that HMAC_CTX is initialized
with the zero key.

Change-Id: I4ee2b495047760765c7d7fdfb4ccb510723aa263
Reviewed-on: https://boringssl-review.googlesource.com/3121
Reviewed-by: Adam Langley <agl@google.com>
2015-01-29 20:24:49 +00:00
Eric Roman
2fe7f2d0d9 Initialize HMAC keys to zero.
In an attempt to assign a zero-length HMAC key, consumers might
incorrectly call:

   HMAC_Init_ex(key=NULL, key_len=0)

This does not work as expected since |key==NULL| has special semantics.
This bug may consequently result in uninitialized memory being used for
the HMAC key data.

This workaround doesn't fix all the problems associated with this
pattern, however by defaulting to a zero key the results are more
predictable than before.

BUG=http://crbug.com/449409

Change-Id: I777276d57c61f1c0cce80b18e28a9b063784733f
Reviewed-on: https://boringssl-review.googlesource.com/3040
Reviewed-by: Adam Langley <agl@google.com>
2015-01-28 01:27:25 +00:00
David Benjamin
238510a679 Introduce HMAC_CTX_copy_ex and deprecate HMAC_CTX_copy.
HMAC_CTX_copy's documentation is off. It actually follows the old copy
functions which call FOO_init on dest first. Notably this means that they leak
memory if dest is currently in use.

Add HMAC_CTX_copy_ex as an analog of EVP_MD_CTX_copy and deprecate
HMAC_CTX_copy. (EVP_CIPHER_CTX_copy, in contrast, was correct from the start.)

Change-Id: I48566c858663d3f659bd356200cf862e196576c9
Reviewed-on: https://boringssl-review.googlesource.com/2694
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 20:35:47 +00:00
Nick Harper
d3bcf13165 Properly handle key_len=0 for HMAC
The expectation when calling HMAC with key=NULL and keylen=0 is to compute
HMAC on the provided data with a key of length 0 instead of using the
"previous" key, which in the case of HMAC() is whatever bytes happen to be
left on the stack when the HMAC_CTX struct is allocated.

Change-Id: I52a95e262ee4e15f1af3136cb9c07f42f40ce122
Reviewed-on: https://boringssl-review.googlesource.com/2660
Reviewed-by: Adam Langley <agl@google.com>
2014-12-19 18:18:08 +00:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00