Also add an assert to that effect.
Change-Id: I1bd0571e3889f1cba968fd99041121ac42ee9e89
Reviewed-on: https://boringssl-review.googlesource.com/5990
Reviewed-by: Adam Langley <agl@google.com>
This is useful to skip an optional element, and mirrors the behaviour of
CBS_get_optional_asn1_octet_string.
Change-Id: Icb538c5e99a1d4e46412cae3c438184a94fab339
Reviewed-on: https://boringssl-review.googlesource.com/5800
Reviewed-by: Adam Langley <agl@google.com>
This begins decoupling the transport from the SSL state machine. The buffering
logic is hidden behind an opaque API. Fields like ssl->packet and
ssl->packet_length are gone.
ssl3_get_record and dtls1_get_record now call low-level tls_open_record and
dtls_open_record functions that unpack a single record independent of who owns
the buffer. Both may be called in-place. This removes ssl->rstate which was
redundant with the buffer length.
Future work will push the buffer up the stack until it is above the handshake.
Then we can expose SSL_open and SSL_seal APIs which act like *_open_record but
return a slightly larger enum due to other events being possible. Likewise the
handshake state machine will be detached from its buffer. The existing
SSL_read, SSL_write, etc., APIs will be implemented on top of SSL_open, etc.,
combined with ssl_read_buffer_* and ssl_write_buffer_*. (Which is why
ssl_read_buffer_extend still tries to abstract between TLS's and DTLS's fairly
different needs.)
The new buffering logic does not support read-ahead (removed previously) since
it lacks a memmove on ssl_read_buffer_discard for TLS, but this could be added
if desired. The old buffering logic wasn't quite right anyway; it tried to
avoid the memmove in some cases and could get stuck too far into the buffer and
not accept records. (The only time the memmove is optional is in DTLS or if
enough of the record header is available to know that the entire next record
would fit in the buffer.)
The new logic also now actually decrypts the ciphertext in-place again, rather
than almost in-place when there's an explicit nonce/IV. (That accidentally
switched in https://boringssl-review.googlesource.com/#/c/4792/; see
3d59e04bce96474099ba76786a2337e99ae14505.)
BUG=468889
Change-Id: I403c1626253c46897f47c7ae93aeab1064b767b2
Reviewed-on: https://boringssl-review.googlesource.com/5715
Reviewed-by: Adam Langley <agl@google.com>
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.
It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.
Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
It switched from CBB_remaining to CBB_len partway through review, but
the semantics are still CBB_remaining. Using CBB_len allows the
len_before/len_after logic to continue working even if, in the future,
handshake messages are built on a non-fixed CBB.
Change-Id: Id466bb341a14dbbafcdb26e4c940a04181f2787d
Reviewed-on: https://boringssl-review.googlesource.com/5371
Reviewed-by: Adam Langley <agl@google.com>
One tedious thing about using CBB is that you can't safely CBB_cleanup
until CBB_init is successful, which breaks the general 'goto err' style
of cleanup. This makes it possible:
CBB_zero ~ EVP_MD_CTX_init
CBB_init ~ EVP_DigestInit
CBB_cleanup ~ EVP_MD_CTX_cleanup
Change-Id: I085ecc4405715368886dc4de02285a47e7fc4c52
Reviewed-on: https://boringssl-review.googlesource.com/5267
Reviewed-by: Adam Langley <agl@google.com>
kData5 was meant to test lengths that are too long, but the input
gets rejected earlier for not using short-form encoding. Switch it to
testing a badly encoded element of length 128, the shortest element that
uses long-form encoding.
Change-Id: I35f4df89bfa7a681698eda569c525b5871288487
Reviewed-on: https://boringssl-review.googlesource.com/5264
Reviewed-by: Adam Langley <agl@google.com>
Rather than four massive functions that handle every extension,
organise the code by extension with four smaller functions for each.
Change-Id: I876b31dacb05aca9884ed3ae7c48462e6ffe3b49
Reviewed-on: https://boringssl-review.googlesource.com/5142
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>
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>
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>
First batch of the alphabet.
Change-Id: If4e60f4fbb69e04eb4b70aa1b2240e329251bfa5
Reviewed-on: https://boringssl-review.googlesource.com/4514
Reviewed-by: Adam Langley <agl@google.com>
The encoding of an INTEGER should not have leading zeros, except to pad for the
sign bit.
Change-Id: I80d22818cf1d2ca9d27e215620392e1725372aa5
Reviewed-on: https://boringssl-review.googlesource.com/4218
Reviewed-by: Adam Langley <agl@google.com>
Tag number 31 is a long form tag that requires multiple octets. It
cannot be handled by adding a single uint8. Changed CBB_add_asn1()
to return 0 when it is passed in the extension for tag 31.
Change-Id: Ia33936d4f174d1a7176eb11da0b5c7370efb9416
CBB_init_fixed() should not call free because it can lead to use after
free or double free bugs. The caller should be responsible for
creating and destroying the buffer.
In the current code, ssl3_get_v2_client_hello() may free s->init_buf->data
via CBB_init_fixed(). It can also be freed via SSL_free(s) since
ssl3_get_v2_client_hello() doesn't set it to NULL and CBB_init_fixed()
can't set the caller's pointer to NULL.
Change-Id: Ia05a67ae25af7eb4fb04f08f20d50d912b41e38b
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>
Bruce Dawson pointed out that the shadowing of |ret| in |s3_srvr.c|
looked dodgy. It was actually deliberate (we don't want to reset the
default value of the function's |ret| variable with a successful return
from the callback) but it does look dodgy.
This change adds -Wshadow to ban variable shadowing and fixes all
current instances.
Change-Id: I1268f88b9f26245c7d16d6ead5bb9014ea471c01
Reviewed-on: https://boringssl-review.googlesource.com/2520
Reviewed-by: Adam Langley <agl@google.com>
Do away with all those unreadable macros. Also fix many many memory leaks in
the SSL_SESSION reuse case. Add a number of helper functions in CBS to help
with parsing optional fields.
Change-Id: I2ce8fd0d5b060a1b56e7f99f7780997fabc5ce41
Reviewed-on: https://boringssl-review.googlesource.com/1998
Reviewed-by: Adam Langley <agl@google.com>
Zero is encoded as a single zero octet. Per X.690, 8.3.1:
The encoding of an integer value shall be primitive. The contents octets
shall consist of one or more octets.
Change-Id: If4304a2be5117b71446a3a62a2b8a6124f85a202
Reviewed-on: https://boringssl-review.googlesource.com/2010
Reviewed-by: Adam Langley <agl@google.com>
Companion to CBS_get_asn1_uint64. Also add tests for both the parsing and the
serializing.
Change-Id: Ic5e9a0089c88b300f874712d0e9964cb35a8c40b
Reviewed-on: https://boringssl-review.googlesource.com/1999
Reviewed-by: Adam Langley <agl@google.com>
Intended to make parsing ASN.1 structures with OPTIONAL elements easier. (Just
attempting to parse the next tag doesn't distinguish between a malformed CBS
which has now been partially advanced and an optional tag mismatch.)
Change-Id: Idceb3dfd6ec028e87e1bc5aaddcec177b0c32150
Reviewed-on: https://boringssl-review.googlesource.com/1995
Reviewed-by: Adam Langley <agl@google.com>
Chromium does not like static initializers, and the CPU logic uses one to
initialize CPU bits. However, the crypto library lacks an explicit
initialization function, which could complicate (no compile-time errors)
porting existing code which uses crypto/, but not ssl/.
Add an explicit CRYPTO_library_init function, but make it a no-op by default.
It only does anything (and is required) if building with
BORINGSSL_NO_STATIC_INITIALIZER.
Change-Id: I6933bdc3447fb382b1f87c788e5b8142d6f3fe39
Reviewed-on: https://boringssl-review.googlesource.com/1770
Reviewed-by: Adam Langley <agl@google.com>
Useful for parsing things when you don't know what tag to expect (e.g. a
CHOICE). Also allow its operands to be NULL, so that it can be used to skip
arbitrary elements.
Delete CBS_ASN1_ANY which wasn't doing anything.
Change-Id: I56413e68b4f2e13860ea3e55373d5830713d7e5c
Reviewed-on: https://boringssl-review.googlesource.com/1661
Reviewed-by: Adam Langley <agl@google.com>
We don't really want to expose this but the bytestring test calls this
function directly and, when linked against a shared library, thus needs
it to be in the dynamic symbol table.
Change-Id: Ife01da36fe142026a6a3e545cff746512b966ee6
This change removes the previous OpenSSL/NSS hack in PKCS#12 parsing and
limits the hacks purely to the BER->DER conversion function, where they
belong.
PKCS#7 and #12 switch between implicit and explicit tags in different
places and sometimes only implicitly define that they are using implicit
tags. This change fixes a previous confusion where an implicit tag was
thought to be explicit.
Change-Id: Ib68c78cf2a1bfcbf90a296cb98313ab86ed2a1f3
Reviewed-on: https://boringssl-review.googlesource.com/1640
Reviewed-by: Adam Langley <agl@google.com>
Previously, the ASN.1 functions in bytestring were capable of processing
indefinite length elements when the _ber functions were used. That works
well enough for PKCS#3, but NSS goes a bit crazy with BER encoding and
PKCS#12. Rather than complicate the core bytestring functions further,
the BER support is removed from them and moved to a separate function
that converts from BER to DER (if needed).
Change-Id: I2212b28e99bab9fab8c61f80d2012d3e5a3cc2f0
Reviewed-on: https://boringssl-review.googlesource.com/1591
Reviewed-by: Adam Langley <agl@google.com>
Otherwise, in C, it becomes a K&R function declaration which doesn't actually
type-check the number of arguments.
Change-Id: I0731a9fefca46fb1c266bfb1c33d464cf451a22e
Reviewed-on: https://boringssl-review.googlesource.com/1582
Reviewed-by: Adam Langley <agl@google.com>
ssl23_get_client_hello has lots of remnants of SSLv2 support and remnants of an
even older SSL_OP_NON_EXPORT_FIRST option (see upstream's
d92f0bb6e9ed94ac0c3aa0c939f2565f2ed95935) which complicates the logic.
Split it into three states and move V2ClientHello parsing into its own
function. Port it to CBS and CBB to give bounds checks on the V2ClientHello
parse.
This fixes a minor bug where, if the SSL_accept call in ssl23_get_client_hello
failed, cb would not be NULL'd and SSL_CB_ACCEPT_LOOP would get reported an
extra time.
It also unbreaks the invariant between s->packet, s->packet_length,
s->s3->rbuf.buf, and s->s3->rbuf.offset at the point the switch, although this
was of no consequence because the first ssl3_read_n call passes extend = 0
which resets s->packet and s->packet_length.
It also makes us tolerant to major version bumps in the ClientHello. Add tests
for TLS tolerance of both minor and major version bumps as well as the HTTP
request error codes.
Change-Id: I948337f4dc483f4ebe1742d3eba53b045b260257
Reviewed-on: https://boringssl-review.googlesource.com/1455
Reviewed-by: Adam Langley <agl@google.com>
This function serialises a PKCS#7 structure containing a number of
certificates.
Change-Id: Iaf15887e1060d5d201d5a3dd3dca8d51105ee6d6
Reviewed-on: https://boringssl-review.googlesource.com/1431
Reviewed-by: Adam Langley <agl@google.com>
When shifting data because extra ASN.1 length bytes were needed, the
data was moved from the start of the ASN.1 length, not the start of the
ASN.1 data.
Change-Id: Ib13d5e4e878774df2af0505c0297eff6cf781728
Reviewed-on: https://boringssl-review.googlesource.com/1430
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Also fix some DTLS cookie bugs. rcvd_cookie is never referenced after being
saved (and the length isn't saved, so it couldn't be used anyway), and the
cookie verification failed to check the length.
For convenience, add a CBS_mem_equal helper function. Saves a bit of
repetition.
Change-Id: I187137733b069f0ac8d8b1bf151eeb80d388b971
Reviewed-on: https://boringssl-review.googlesource.com/1174
Reviewed-by: Adam Langley <agl@google.com>
Also tidy up some variable names and update RSA_verify call for it no longer
returning -1. Add CBS helper functions for dealing with C strings.
Change-Id: Ibc398d27714744f5d99d4f94ae38210cbc89471a
Reviewed-on: https://boringssl-review.googlesource.com/1164
Reviewed-by: Adam Langley <agl@google.com>
Previously, public headers lived next to the respective code and there
were symlinks from include/openssl to them.
This doesn't work on Windows.
This change moves the headers to live in include/openssl. In cases where
some symlinks pointed to the same header, I've added a file that just
includes the intended target. These cases are all for backwards-compat.
Change-Id: I6e285b74caf621c644b5168a4877db226b07fd92
Reviewed-on: https://boringssl-review.googlesource.com/1180
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This gives us systematic bounds-checking on all the parses. Also adds a
convenience function, CBS_memdup, for saving the current contents of a CBS.
Change-Id: I17dad74575f03121aee3f771037b8806ff99d0c3
Reviewed-on: https://boringssl-review.googlesource.com/1031
Reviewed-by: Adam Langley <agl@google.com>
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
