Also implement it without reference to crypto/asn1 or fake ASN1_INTEGERs and
add a test. Some platform crypto APIs only give back the key size, and not the
encoded signature length. No sense in implementing it twice.
BUG=347404,499653
Change-Id: I9aa27d52674375f8b036e57bb5850f091c9b25dd
Reviewed-on: https://boringssl-review.googlesource.com/5080
Reviewed-by: Adam Langley <agl@google.com>
Platform crypto APIs for PKCS#1 RSA signatures vary between expecting the
caller to prepend the DigestInfo prefix (RSA_sign_raw) and prepending it
internally (RSA_sign). Currently, Chromium implements sign or sign_raw as
appropriate. To avoid needing both variants, the new asynchronous methods will
only expose the higher-level one, sign.
To satisfy ports which previously implemented sign_raw, expose the DigestInfo
prefix as a utility function.
BUG=347404
Change-Id: I04c397b5e9502b2942f6698ecf81662a3c9282e6
Reviewed-on: https://boringssl-review.googlesource.com/4940
Reviewed-by: Adam Langley <agl@google.com>
Also tighten X509_cmp_time to reject more than three fractional
seconds in the time; and to reject trailing garbage after the offset.
CVE-2015-1789
(Imported from upstream's 9bc3665ac9e3c36f7762acd3691e1115d250b030)
Change-Id: I2091b2d1b691c177d58dc7960e2e7eb4c97b1f69
Reviewed-on: https://boringssl-review.googlesource.com/5124
Reviewed-by: Adam Langley <agl@google.com>
If gdb is attached, it's convenient to be able to continue running.
Change-Id: I3bbb2634d05a08f6bad5425f71da2210dbb80cfe
Reviewed-on: https://boringssl-review.googlesource.com/5125
Reviewed-by: Adam Langley <agl@google.com>
Mirrors SSL_SESSION_to_bytes. It avoids having to deal with object-reuse, the
non-size_t length parameter, and trailing data. Both it and the object-reuse
variant back onto an unexposed SSL_SESSION_parse which reads a CBS.
Note that this changes the object reuse story slightly. It's now merely an
optional output pointer that frees its old contents. No d2i_SSL_SESSION
consumer in Google that's built does reuse, much less reuse with the assumption
that the top-level object won't be overridden.
Change-Id: I5cb8522f96909bb222cab0f342423f2dd7814282
Reviewed-on: https://boringssl-review.googlesource.com/5121
Reviewed-by: Adam Langley <agl@google.com>
The binaries for these already have underscores in, best to have the
source files match.
Change-Id: I32a419f32ec7786fe2537d061eb0706a7bc73f4a
Reviewed-on: https://boringssl-review.googlesource.com/5101
Reviewed-by: Adam Langley <agl@google.com>
Per malloc(3):
The UNIX 98 standard requires malloc(), calloc(), and realloc() to set
errno to ENOMEM upon failure. Glibc assumes that this is done (and the
glibc versions of these routines do this); if you use a private malloc
implementation that does not set errno, then certain library routines may
fail without having a reason in errno.
Notably, thread_test otherwise fails an assertion deep in glibc.
Change-Id: Ia2c0ab306987476e7d6570d4bbf04a2641398925
Reviewed-on: https://boringssl-review.googlesource.com/5111
Reviewed-by: Adam Langley <agl@google.com>
BN_copy can fail on malloc failure. The case in crypto/rsa was causing the
malloc tests in all_tests.go to infinite loop.
Change-Id: Id5900512013fba9960444d78a8c056aa4314fb2d
Reviewed-on: https://boringssl-review.googlesource.com/5110
Reviewed-by: Adam Langley <agl@google.com>
When cross-compiling for Windows on a Linux system, the filesystem is
case sensitive and so #includes with uppercase characters cause errors.
Change-Id: I6fe6d677887ce84ba8b2c3136cf2c70998d96e81
Reviewed-on: https://boringssl-review.googlesource.com/5060
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
In doing so, check for malloc failures and use scopers as appropriate.
This should clear rsa_test for use with malloc tests. Also replace the
SetKey macro and exploded RSA keys with a DER RSAPrivateKey structure.
Much less tedious.
Change-Id: I3ce092ef67e7ac2af74f509abbdf84b7f2b6d45d
Reviewed-on: https://boringssl-review.googlesource.com/5043
Reviewed-by: Adam Langley <agl@google.com>
We have need of it internally.
Change-Id: I564af468728b22245e8eab384ea7018b7e88cc86
Reviewed-on: https://boringssl-review.googlesource.com/5022
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Mac wants a stdlib.h. Windows wants a void and doesn't like static const in
array declarations.
Change-Id: If1e8fb141e41200cf8a7348c6918c2f22465f5fe
Reviewed-on: https://boringssl-review.googlesource.com/5030
Reviewed-by: Adam Langley <agl@google.com>
This change makes |CBS_get_any_asn1_element| only handle DER elements.
Another function, |CBS_get_any_ber_asn1_element| is exposed internally
for the cases where we need to process BER data.
Change-Id: I544141a1a3d7913986352a8fd9a6d00b9f282652
Reviewed-on: https://boringssl-review.googlesource.com/4994
Reviewed-by: Adam Langley <agl@google.com>
The documentation for |CBS_get_any_asn1_element| says that
|out_header_len| may be NULL, but in the case of an indefinite-length
element it would be written unconditionally.
Thanks to Doug Hogan for noticing this.
Change-Id: I17609b3465df73d42dd9efd75e783159aa99a59b
Reviewed-on: https://boringssl-review.googlesource.com/4993
Reviewed-by: Adam Langley <agl@google.com>
The client and server both have to decide on behaviour when resuming a
session where the EMS state of the session doesn't match the EMS state
as exchanged in the handshake.
Original handshake
| No Yes
------+--------------------------------------------------------------
|
R | Server: ok [1] Server: abort [3]
e No | Client: ok [2] Client: abort [4]
s |
u |
m |
e |
Yes | Server: don't resume No problem
| Client: abort; server
| shouldn't have resumed
[1] Servers want to accept legacy clients. The draft[5] says that
resumptions SHOULD be rejected so that Triple-Handshake can't be done,
but we'll rather enforce that EMS was used when using tls-unique etc.
[2] The draft[5] says that even the initial handshake should be aborted
if the server doesn't support EMS, but we need to be able to talk to the
world.
[3] This is a very weird case where a client has regressed without
flushing the session cache. Hopefully we can be strict and reject these.
[4] This can happen when a server-farm shares a session cache but
frontends are not all updated at once. If Chrome is strict here then
hopefully we can prevent any servers from existing that will try to
resume an EMS session that they don't understand. OpenSSL appears to be
ok here: https://www.ietf.org/mail-archive/web/tls/current/msg16570.html
[5] https://tools.ietf.org/html/draft-ietf-tls-session-hash-05#section-5.2
BUG=492200
Change-Id: Ie1225a3960d49117b05eefa5a36263d8e556e467
Reviewed-on: https://boringssl-review.googlesource.com/4981
Reviewed-by: Adam Langley <agl@google.com>
This implementation does not prompt for a password. It's just enough
to ensure that the many functions that take a tuple of
|pem_password_cb| and a |void *| to a password work in a reasonable
way when the latter is non-NULL.
Change-Id: Ic6bfc484630c67b5ede25277e14eb3b00c2024f0
Reviewed-on: https://boringssl-review.googlesource.com/4990
Reviewed-by: Adam Langley <agl@google.com>
It would be nice if assert(x) reduced to ((void) x) when NDEBUG was
defined, but it doesn't. Because of this, locally define CHECK, which
does. This avoids warnings with Clang.
Change-Id: I70882741da4984a025bcfaac1969032387f369de
Reviewed-on: https://boringssl-review.googlesource.com/4991
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
bsaes-armv7.S implements bsaes_cbc_encrypt if #if __ARM_MAX_ARCH__ >= 7
but e_aes.c instead used #if __ARM_ARCH >= 7 causing duplicate symbols
for linkers that care about that
Change-Id: I10ad8e24be75fdc03b0670869a53078b0477950b
Reviewed-on: https://boringssl-review.googlesource.com/4943
Reviewed-by: Adam Langley <agl@google.com>
The only flag is EVP_MD_CTX_FLAG_NO_INIT and no good can possibly come of
anyone outside EVP_PKEY_HMAC calling it. (And indeed no one calls it.
EVP_MD_CTX_set_flags has a caller in wpa_supplicant, but it uses
EVP_MD_CTX_FLAG_NON_FIPS_ALLOW which we don't define. The call is guarded by a
pair of ifdefs for some FIPS mode wpa_supplicant.)
Change-Id: I70ab8ffa646f3f75dfa4d37c96b9e82448ff1e40
Reviewed-on: https://boringssl-review.googlesource.com/4971
Reviewed-by: Adam Langley <agl@google.com>
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>
Not terribly important given that we already have NIST vectors, but may as
well. These tests come from upstream's
2cfbdd71dde0c3ddf4597eb20cc3e3fb8485fc15.
Change-Id: I4f8dadc7d5d1599d0b75ecdef06f2fc6a5cd8003
Reviewed-on: https://boringssl-review.googlesource.com/4962
Reviewed-by: Adam Langley <agl@google.com>
The SSL_PROTOCOL_METHOD table needs work, but this makes it clearer
exactly what the shared interface between the upper later and TLS/DTLS
is.
BUG=468889
Change-Id: I38931c484aa4ab3f77964d708d38bfd349fac293
Reviewed-on: https://boringssl-review.googlesource.com/4955
Reviewed-by: Adam Langley <agl@google.com>
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>
The SHA-2 family has some exceptions, but they're all programmer errors
and should be documented as such. (Are the failure cases even
necessary?)
Change-Id: I00bd0a9450cff78d8caac479817fbd8d3de872b8
Reviewed-on: https://boringssl-review.googlesource.com/4953
Reviewed-by: Adam Langley <agl@google.com>
Use sized integer types rather than unsigned char/int/long. The latter
two are especially a mess as they're both used in lieu of uint32_t.
Sometimes the code just blindly uses unsigned long and sometimes it uses
unsigned int when an LP64 architecture would notice.
Change-Id: I4c5c6aaf82cfe9fe523435588d286726a7c43056
Reviewed-on: https://boringssl-review.googlesource.com/4952
Reviewed-by: Adam Langley <agl@google.com>
At some point we might need to make this defined by the consumer.
BUG=495146
Change-Id: Iedac305f234cb383799a5afc14046cd10fb3256a
Reviewed-on: https://boringssl-review.googlesource.com/4963
Reviewed-by: Adam Langley <agl@google.com>
bn_test's output is meant to be piped to bc, but this got broken somewhat:
- OpenSSL uses uppercase hex rather than BoringSSL's lowercase. bc only accepts
uppercase. Document that this needs some shell pipeline until we replace
them with better tests because this is all ridiculous.
- Some stderr outputs moved to stdout to avoid cluttering stdout. Just remove
them. The operations are fast enough to not need progress.
- To cut down on noise, only write the bc transcript given a command-line flag.
Also remove the -results flag since it's pointless. (It writes only the
results and not the inputs.)
Change-Id: I08f87cac1e03fab461f0dc40b9d4285bd877807d
Reviewed-on: https://boringssl-review.googlesource.com/4896
Reviewed-by: Adam Langley <agl@google.com>
While this isn't really an issue, don't use the a - b comparator pattern since
it doesn't account for overflows. (They'll also break silently if that field
ever becomes unsigned as it should be.)
Change-Id: I613d19df6e4a785efd4cffd46e8b03dbc95b98e2
Reviewed-on: https://boringssl-review.googlesource.com/4890
Reviewed-by: Adam Langley <agl@google.com>
Android needs to be able to read a PKCS#7 blob from a Java
InputStream. This change adds |BIO_read_asn1| which reads a single
ASN.1 object from the start of a BIO without overreading.
Change-Id: I74776e686529c8e58af1c26a4909f9bd4e87b707
If BN_rand is called with |bits| set to 1 and |top| set to 1 then a 1 byte
buffer overflow can occur.
See also upstream's efee575ad464bfb60bf72dcb73f9b51768f4b1a1. But rather than
making |BN_rand| fail, be consistent with the |bits| = 0 case and just don't
set the bits that don't exist. Add tests to ensure the degenerate cases behave.
Change-Id: I5e9fbe6fd8f7f7b2e011a680f2fbe6d7ed4dab65
Reviewed-on: https://boringssl-review.googlesource.com/4893
Reviewed-by: Adam Langley <agl@google.com>
The functions BN_rshift and BN_lshift shift their arguments to the right or
left by a specified number of bits. Unpredicatable results (including
crashes) can occur if a negative number is supplied for the shift value.
Thanks to Mateusz Kocielski (LogicalTrust), Marek Kroemeke and Filip Palian
for discovering and reporting this issue.
(Imported from upstream's 7cc18d8158b5fc2676393d99b51c30c135502107.)
Change-Id: Ib9f5e410a46df3d7f02a61374807fba209612bd3
Reviewed-on: https://boringssl-review.googlesource.com/4892
Reviewed-by: Adam Langley <agl@google.com>
See upstream's 344c271eb339fc2982e9a3584a94e51112d84584. We had the error check
already. But, for consistency with the rest of that function's error paths,
pushing an error on the error queue would be prudent.
Change-Id: I8b702abc679dc94dffa79c19a9b7c3d0adc0638b
Reviewed-on: https://boringssl-review.googlesource.com/4889
Reviewed-by: Adam Langley <agl@google.com>
(obj_dat.h and obj_mac.h are generated from the objects.txt change.)
See upstream's 3c161d081e2d30549e787437d05ffa08122a5114. Also see upstream's
12048657a91b12e499d03ec9ff406b42aba67366 to give zlib a better comment.
Change-Id: I86937f037f8e0f6179ba8072ccd972eca773c7ce
Reviewed-on: https://boringssl-review.googlesource.com/4882
Reviewed-by: Adam Langley <agl@google.com>
Support is spotty enough with compiler/library mismatches, and this doesn't
leak to public headers. It's probably simplest to just have consumers supply
it as a build flag.
BUG=491808
Change-Id: I0576a0514a266ee90d644317ae0f49cdddbafd1d
Reviewed-on: https://boringssl-review.googlesource.com/4880
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
With NO_ASM defined, the recent AEAD changes broke the tests. The
problem is that the generic CBC mode code tests whether in != out and
omits to save the IV, assuming that it'll be able to read the old
ciphertext block.
However, consider the case where out = in - 16:
1 2 3 4
|-------|-------|------|-------|
^ ^
| |
out in
First time around, 1 = decrypt(2) ^ iv and everything is fine, because
the IV was preconfigured. However, the next iteration of the loop sets
2 = decrypt(3) and tries to XOR it with the contents of the previous
ciphertext block… from 2.
Change-Id: Ibabff430704fad246de132b4d6d514f6a0362734
This isn't exhaustive. There are still failures in some tests which probably
ought to get C++'d first.
Change-Id: Iac58df9d98cdfd94603d54374a531b2559df64c3
Reviewed-on: https://boringssl-review.googlesource.com/4795
Reviewed-by: Adam Langley <agl@google.com>
Currently far from passing and I haven't even tried with a leak checker yet.
Also bn_test is slow.
Change-Id: I4fe2783aa5f7897839ca846062ae7e4a367d2469
Reviewed-on: https://boringssl-review.googlesource.com/4794
Reviewed-by: Adam Langley <agl@google.com>
tls1_enc is now SSL_AEAD_CTX_{open,seal}. This starts tidying up a bit
of the record-layer logic. This removes rr->input, as encrypting and
decrypting records no longer refers to various globals. It also removes
wrec altogether. SSL3_RECORD is now only used to maintain state about
the current incoming record. Outgoing records go straight to the write
buffer.
This also removes the outgoing alignment memcpy and simply calls
SSL_AEAD_CTX_seal with the parameters as appropriate. From bssl speed
tests, this seems to be faster on non-ARM and a bit of a wash on ARM.
Later it may be worth recasting these open/seal functions to write into
a CBB (tweaked so it can be malloc-averse), but for now they take an
out/out_len/max_out trio like their EVP_AEAD counterparts.
BUG=468889
Change-Id: Ie9266a818cc053f695d35ef611fd74c5d4def6c3
Reviewed-on: https://boringssl-review.googlesource.com/4792
Reviewed-by: Adam Langley <agl@google.com>
