Commit Graph

23 Commits

Author SHA1 Message Date
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
4f57074bfa Check input length to pkey_rsa_verify and add initial tests.
This is imported from upstream's
71bbc79b7d3b1195a7a7dd5f547d52ddce32d6f0 and test vectors taken
initially from 2d7bbd6c9fb6865e0df480602c3612652189e182 (with a handful
more added).

The tests are a little odd because OpenSSL supports this "salt length
recovery" mode and they go through that codepath for all verifications.

Change-Id: I220104fe87e2a1a1458c99656f9791d8abfbbb98
Reviewed-on: https://boringssl-review.googlesource.com/12822
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-16 17:16:57 +00:00
David Benjamin
981936791e Remove some easy obj.h dependencies.
A lot of consumers of obj.h only want the NID values. Others didn't need
it at all. This also removes some OBJ_nid2sn and OBJ_nid2ln calls in EVP
error paths which isn't worth pulling a large table in for.

BUG=chromium:499653

Change-Id: Id6dff578f993012e35b740a13b8e4f9c2edc0744
Reviewed-on: https://boringssl-review.googlesource.com/7563
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 20:50:33 +00:00
David Benjamin
719594e512 Un-const EVP_PKEY_CTX_set0_rsa_oaep_label and fix overflow check.
It takes ownership of the buffer, so it's not actually const. The
const-ness gets dropped once it transits through EVP_PKEY_CTX_ctrl.

Also compare against INT_MAX explicitly for the overflow check. I'm not sure
whether the casting version is undefined, but comparing against INT_MAX matches
the rest of the codebase when transiting in and out of signed ints.

Change-Id: I131165a4b5f0ebe02c6db3e7e3e0d1af5b771710
Reviewed-on: https://boringssl-review.googlesource.com/6850
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:34:38 +00:00
David Benjamin
8ac35f0274 Remove unused EVP_PKEY_METHOD hooks.
foo_init hooks are never implemented. Even upstream never uses them. The
flags member is also never used. We also don't expose paramgen, so
remove it.

Change-Id: I51d9439316c5163520ab7168693c457f33e59417
Reviewed-on: https://boringssl-review.googlesource.com/6846
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:23:46 +00:00
Adam Langley
ef7dba6ac7 Fix error in ce9d85ee.
I used a size_t out of habit, but |RSA_public_decrypt| is an old-style
function.

Change-Id: Ibd94d03743fe0099d61578ec15c19fa5333127db
2016-01-26 15:31:23 -08:00
Adam Langley
ce9d85eedd Tweaks for node.js
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.

Changes:
  ∙ Support verify_recover. This is very obscure and the motivation
    appears to be https://github.com/nodejs/node/issues/477 – where it's
    not clear that anyone understands what it means :(
  ∙ Add a few, no-op #defines
  ∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
    reach into these structs in the future.
  ∙ Add EC_get_builtin_curves.
  ∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
    decrepit.

Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:42 +00:00
David Benjamin
e13263d5e4 Resolve a few old TODOs.
A lot of commented-out code we haven't had to put them back, so these
can go now. Also remove the TODO about OAEP having a weird API. The API
is wrong, but upstream's shipped it with the wrong API, so that's what
it is now.

Change-Id: I7da607cf2d877cbede41ccdada31380f812f6dfa
Reviewed-on: https://boringssl-review.googlesource.com/6763
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:14:35 +00:00
David Benjamin
3570d73bf1 Remove the func parameter to OPENSSL_PUT_ERROR.
Much of this was done automatically with
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/'
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+,  ([a-zA-Z_0-9]+\);)/\1\2/'

BUG=468039

Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8
Reviewed-on: https://boringssl-review.googlesource.com/5276
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:37 +00:00
David Benjamin
65ee9b7ce9 Remove EVP_PKEY_HMAC.
This removes EVP_PKEY_HMAC and all the support code around it. EVP_MD requires
a lot of extra glue to support HMAC. This lets us prune it all away.

As a bonus, it removes a (minor) dependency from EVP to the legacy ASN.1 stack.

Change-Id: I5a9e3e39f518429828dbf13d14647fb37d9dc35a
Reviewed-on: https://boringssl-review.googlesource.com/5120
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 00:03:02 +00:00
David Benjamin
cca4ba7611 Remove unnecessary NULL checks, part 3.
Finish up the e's.

Change-Id: Iabb8da000fbca6efee541edb469b90896f60d54b
Reviewed-on: https://boringssl-review.googlesource.com/4516
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:12:04 +00:00
David Benjamin
7cc29ab833 Take advantage of normalized return values.
Now that much of EVP has been normalized to 0/1, a lot of code can just use
boolean operators. (As can some code which was already using them...)

Change-Id: I6bb17edfd6f67050bf1706d59d8f37df57535faa
Reviewed-on: https://boringssl-review.googlesource.com/3875
Reviewed-by: Adam Langley <agl@google.com>
2015-03-13 19:40:42 +00:00
David Benjamin
e0ba4dddf6 Normalize all EVP_PKEY_CTX_ctrl return values.
No code within BoringSSL or Google (grep for EVP_PKEY_CTX_(ctrl|get|set)) is
sensitive to the various failure cases. Normalize it all to 0/1 for simplicity.

This does carry a slight risk: any new ctrl hooks we import from upstream that,
like EVP_PKEY_CTX_get_rsa_oaep_md, return something other than success/failure
cannot be called directly via EVP_PKEY_CTX_ctrl. They instead need to
internally be routed through a struct like CBS and only called through the
wrappers. To that end, unexport EVP_PKEY_CTX_ctrl and require that callers use
the wrappers. No code in Google uses it directly and, if need be, switching to
the wrapper would be an incredibly upstreamable patch.

Change-Id: I3fd4e5a1a0f3d4d1c4122c52d4c74a5105b99cd5
Reviewed-on: https://boringssl-review.googlesource.com/3874
Reviewed-by: Adam Langley <agl@google.com>
2015-03-13 19:39:42 +00:00
David Benjamin
bc5d8ee0d4 Route EVP_PKEY_CTRL_GET_RSA_OAEP_LABEL through a CBS.
This is the only EVP_PKEY ctrl hook which returns something other than a
boolean.

Change-Id: Ic226aef168abdf72e5d30e8264a559ed5039a055
Reviewed-on: https://boringssl-review.googlesource.com/3873
Reviewed-by: Adam Langley <agl@google.com>
2015-03-13 19:19:59 +00:00
David Benjamin
e60202797d Move EVP_R_COMMAND_NOT_SUPPORTED into individual EVP_PKEY ctrl hooks.
This removes another place where we're internally sensitive to the
success/failure conditions.

Change-Id: I18fecf6457e841ba0afb718397b9b5fd3bbdfe4c
Reviewed-on: https://boringssl-review.googlesource.com/3872
Reviewed-by: Adam Langley <agl@google.com>
2015-03-13 19:18:39 +00:00
David Benjamin
6eb000dbee Add in missing curly braces part 3.
Everything else.

Change-Id: Iac02b144465b4e7b6d69ea22ff2aaf52695ae732
2015-02-11 15:14:46 -08: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
5db5d4e2b0 RSA_encrypt and RSA_sign_raw aren't the same.
Change-Id: Ie09ab12baf59b8c1d628396424ce6cadf43bf0e1
Reviewed-on: https://boringssl-review.googlesource.com/1913
Reviewed-by: Adam Langley <agl@google.com>
2014-10-09 21:04:07 +00:00
Adam Langley
5129e2d695 Align EVP return values with BoringSSL convention.
Where possible, functions should return one for success and zero for
error. The use of additional negative values to indicate an error is,
itself, error prone.

This change fixes many EVP functions to remove the possibility of
negative return values. Existing code that is testing for <= 0 will
continue to function, although there is the possibility that some code
was differentiating between negative values (error) and zero (invalid
signature) for the verify functions and will now show the wrong error
message.

Change-Id: I982512596bb18a82df65861394dbd7487783bd3d
Reviewed-on: https://boringssl-review.googlesource.com/1333
Reviewed-by: Adam Langley <agl@google.com>
2014-07-29 21:47:51 +00:00
David Benjamin
e14dcc45e8 Remove RSA_SSLV23_PADDING.
It's unused with SSLv2 gone. Also, being a decryption padding check, it really
should be constant-time and isn't.

Change-Id: I96be02cb50f9bf0229b9174eccd80fa338bf8e3e
Reviewed-on: https://boringssl-review.googlesource.com/1254
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 19:23:51 +00:00
David Benjamin
9b561e69b6 Fix EVP_PKEY_FLAG_AUTOARGLEN behavior.
Converting check_autoarg from a macro to a function lost the behavior. Instead,
just move the logic into p_rsa.c which was the only EVP_PKEY implementation
that even needed the flag.

Also document this behavior on each of the functions. Make note of the out =
NULL case only returning the maximum output size, and not necessarily the
actual size.

For testing, update example_sign to determine the signature size using the NULL
behavior rather than querying the RSA key.

Change-Id: Iec6c2862028a5cfdefe8faa0e8c471755070898a
Reviewed-on: https://boringssl-review.googlesource.com/1121
Reviewed-by: Adam Langley <agl@google.com>
2014-07-10 02:10:47 +00:00
Adam Langley
6887edb917 Improvements in constant-time OAEP decoding.
This change adds a new function, BN_bn2bin_padded, that attempts, as
much as possible, to serialise a BIGNUM in constant time.

This is used to avoid some timing leaks in RSA decryption.
2014-06-20 13:17:37 -07: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