Change-Id: I55dc3d87a9571901abd2bbaf268871a482cf3bc5
Reviewed-on: https://boringssl-review.googlesource.com/4983
Reviewed-by: David Benjamin <davidben@chromium.org>
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>
Add expectResumeRejected to note cases where we expect a resumption
handshake to be rejected. (This was previously done by adding a flag,
which is a little less clear.)
Also, save the result of crypto/tls.Conn.ConnectionState() rather than
repeat that a lot.
Change-Id: I963945eda5ce1f3040b655e2441174b918b216b3
Reviewed-on: https://boringssl-review.googlesource.com/4980
Reviewed-by: David Benjamin <davidben@chromium.org>
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>
win8sdk got renamed to win_sdk. Also minor fixes from upstream, mostly pylint.
Upstream also no longer keeps the toolchain hash in a separate file.
Change-Id: Iefc8bb6a487f0cdb13fcf3131b0fba317ca6548b
Reviewed-on: https://boringssl-review.googlesource.com/4982
Reviewed-by: Adam Langley <agl@google.com>
Use when giving -DANDROID_ABI="armeabi-v6 with VFP" to android-cmake
Change-Id: Ifb053bcf4788e34fb54e20c27d4e519416ca9a11
Reviewed-on: https://boringssl-review.googlesource.com/4945
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
2cfbdd71dd.
Change-Id: I4f8dadc7d5d1599d0b75ecdef06f2fc6a5cd8003
Reviewed-on: https://boringssl-review.googlesource.com/4962
Reviewed-by: Adam Langley <agl@google.com>
With SSL2 gone, there's no need for this split between the abstract
cipher framework and ciphers. Put the cipher suite table in ssl_cipher.c
and move other SSL_CIPHER logic there. With that gone, prune the
cipher-related hooks in SSL_PROTOCOL_METHOD.
BUG=468889
Change-Id: I48579de8bc4c0ea52781ba1b7b57bc5b4919d21c
Reviewed-on: https://boringssl-review.googlesource.com/4961
Reviewed-by: Adam Langley <agl@google.com>
Make sure we don't break that on accident.
Change-Id: I22d58d35170d43375622fe61e4a588d1d626a054
Reviewed-on: https://boringssl-review.googlesource.com/4960
Reviewed-by: Adam Langley <agl@google.com>
They're redundant with each other.
Change-Id: I17e7ff8c4e0b1486986dd866fd99673fa2aaa494
Reviewed-on: https://boringssl-review.googlesource.com/4959
Reviewed-by: Adam Langley <agl@google.com>
All ciphers are implemented by an EVP_AEAD.
Change-Id: Ifa754599a34e16bf97e1a4b84a271c6d45462c7c
Reviewed-on: https://boringssl-review.googlesource.com/4958
Reviewed-by: Adam Langley <agl@google.com>
The ctrl hooks are left alone since they should just go away.
Simplifying the cipher story will happen in the next CL.
BUG=468889
Change-Id: I979971c90f59c55cd5d17554f1253158b114f18b
Reviewed-on: https://boringssl-review.googlesource.com/4957
Reviewed-by: Adam Langley <agl@google.com>
This still needs significant work, especially the close_notify half, but
clarify the interface and get *_read_bytes out of SSL_PROTOCOL_METHOD.
read_bytes is an implementation detail of those two and get_message
rather than both an implementation detail of get_message for handshake
and a (wholly inappropriate) exposed interface for the other two.
BUG=468889
Change-Id: I7dd23869e0b7c3532ceb2e9dd31ca25ea31128e7
Reviewed-on: https://boringssl-review.googlesource.com/4956
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>
The old upstream logic actually didn't do this, but 1.1.0's new code does.
Given that the version has never changed and even unknown fields were rejected
by the old code, this seems a safe and prudent thing to do.
Change-Id: I09071585e5183993b358c10ad36fc206f8bceeda
Reviewed-on: https://boringssl-review.googlesource.com/4942
Reviewed-by: Adam Langley <agl@google.com>
The original OpenSSL implementation did the same. M_ASN1_D2I_Finish checks
this. Forwards compatibility with future sessions with unknown fields is
probably not desirable.
Change-Id: I116a8c482cbcc47c3fcc31515c4a3718f66cf268
Reviewed-on: https://boringssl-review.googlesource.com/4941
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>
9a41d1b946 broke handling of multiple records in
a single packet. If |extend| is true, not all of the previous packet should be
consumed, only up to the record length.
Add a test which stresses the DTLS stack's handling of multiple handshake
fragments in a handshake record and multiple handshake records in a packet.
Change-Id: I96571098ad9001e96440501c4730325227b155b8
Reviewed-on: https://boringssl-review.googlesource.com/4950
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>
These defines are part of the the locking callbacks which have been
removed. However, code that still tries to provide locking callbacks
will need these values to compile.
The locking callback that such code tries to install will be ignored,
but that's harmless since BoringSSL handles locking itself now.
Change-Id: Ic84da8b52020ccd3ecc8913b4e41d366690c7649
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 efee575ad4. 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 344c271eb3. 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>
See also upstream's dab18ab596. This allows us to
add an assertion to the finished computation that the handshake buffer has
already been released.
BUG=492371
Change-Id: I8f15c618c8b2c70bfe583c81644d9dbea95519d4
Reviewed-on: https://boringssl-review.googlesource.com/4887
Reviewed-by: Adam Langley <agl@google.com>
This allows us to merge two of the ssl3_digest_cached_records calls which were
almost, but not completely, redundant. Also catches a missing case: the buffer
may be discarded if doing session resumption but otherwise enabling client
authentication.
BUG=492371
Change-Id: I78e9a4a9cca665e89899ef97b815454c6f5c7e02
Reviewed-on: https://boringssl-review.googlesource.com/4885
Reviewed-by: Adam Langley <agl@google.com>
Servers can no longer renegotiate.
Change-Id: Id79d5753562e29d2872871f4f571552a019215fa
Reviewed-on: https://boringssl-review.googlesource.com/4884
Reviewed-by: Adam Langley <agl@google.com>
This is documented as "Only request a client certificate on the initial TLS/SSL
handshake. Do not ask for a client certificate again in case of a
renegotiation." Server-side renegotiation is gone.
I'm not sure this flag has ever worked anyway, dating all the way back to
SSLeay 0.8.1b. ssl_get_new_session overwrites s->session, so the old
session->peer is lost.
Change-Id: Ie173243e189c63272c368a55167b8596494fd59c
Reviewed-on: https://boringssl-review.googlesource.com/4883
Reviewed-by: Adam Langley <agl@google.com>
(obj_dat.h and obj_mac.h are generated from the objects.txt change.)
See upstream's 3c161d081e. Also see upstream's
12048657a9 to give zlib a better comment.
Change-Id: I86937f037f8e0f6179ba8072ccd972eca773c7ce
Reviewed-on: https://boringssl-review.googlesource.com/4882
Reviewed-by: Adam Langley <agl@google.com>
Never send the time as a client. Always send it as a server.
Change-Id: I20c55078cfe199d53dc002f6ee5dd57060b086d5
Reviewed-on: https://boringssl-review.googlesource.com/4829
Reviewed-by: Adam Langley <agl@google.com>
Adam Langley
Adam Langley
Matt Braithwaite
David Benjamin
Joel Klinghed