These were already documented, though some of the documentation was
expanded on slightly.
Change-Id: I04c6276a83a64a03ab9cce9b9c94d4dea9ddf638
Reviewed-on: https://boringssl-review.googlesource.com/5896
Reviewed-by: Adam Langley <agl@google.com>
SCT and OCSP are part of the session data and as such shouldn't be sent
again to the client when resuming.
Change-Id: Iaee3a3c4c167ea34b91504929e38aadee37da572
Reviewed-on: https://boringssl-review.googlesource.com/5900
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
ssl.h should be first. Also two lines after includes and the rest of the
file.
Change-Id: Icb7586e00a3e64170082c96cf3f8bfbb2b7e1611
Reviewed-on: https://boringssl-review.googlesource.com/5892
Reviewed-by: Adam Langley <agl@google.com>
Forgot to fix these when I fixed the headers.
Change-Id: Ie45e624abc993e16e2d5a872ef00dba9029a38df
Reviewed-on: https://boringssl-review.googlesource.com/5891
Reviewed-by: Adam Langley <agl@google.com>
This gets the documentation into the ssl.h documentation, and removes
one of the circularly-dependent headers hanging off ssl.h. Also fixes
some typos; there were a few instances of "SSL *ctx".
Change-Id: I2a41c6f518f4780af84d468ed220fe7b0b8eb0d3
Reviewed-on: https://boringssl-review.googlesource.com/5883
Reviewed-by: Adam Langley <agl@google.com>
Also switch to the new variable names (SSL_CTX *ctx, SSL *ssl,
SSL_SESSION *session) for all documented functions.
Change-Id: I15e15a703b96af1727601108223c7ce3b0691f1d
Reviewed-on: https://boringssl-review.googlesource.com/5882
Reviewed-by: Adam Langley <agl@google.com>
This is arguably more commonly queried connection information than the
tls-unique.
Change-Id: I1f080536153ba9f178af8e92cb43b03df37110b5
Reviewed-on: https://boringssl-review.googlesource.com/5874
Reviewed-by: Adam Langley <agl@google.com>
Just the stuff that has been pulled out into sections already.
Change-Id: I3da6bc61d79ccfe2b18d888075dc32026a656464
Reviewed-on: https://boringssl-review.googlesource.com/5873
Reviewed-by: Adam Langley <agl@google.com>
Unfortunately, these are also some of the worst APIs in the SSL stack.
I've tried to capture all the things they expose to the caller. 0 vs -1
is intentionally left unexpanded on for now. Upstream's documentation
says 0 means transport EOF, which is a nice idea but isn't true. (A lot
of random functions return 0 on error and pass it up to the caller.)
https://crbug.com/466303 tracks fixing that.
SSL_set_bio is intentionally documented to NOT be usable when they're
already configured. The function tries to behave in this case and even
with additional cases when |rbio| and/or |wbio| are unchanged, but this
is buggy. For instance, this will explode:
SSL_set_bio(ssl, bio1, bio1);
SSL_set_bio(ssl, bio2, SSL_get_wbio(ssl));
As will this, though it's less clear this is part of the API contract
due to SSL taking ownership.
SSL_set_bio(ssl, bio1, bio2);
SSL_set_bio(ssl, bio2, bio1);
It also tries to handle ssl->bbio already existing, but I doubt it quite
works. Hopefully we can drop ssl->bbio eventually. (Why is this so
complicated...)
Change-Id: I5f9f3043915bffc67e2ebd282813e04afbe076e6
Reviewed-on: https://boringssl-review.googlesource.com/5872
Reviewed-by: Adam Langley <agl@google.com>
Last set of changes didn't do that.
Change-Id: Iae24e75103529ce4d50099c5cbfbcef0e10ba663
Reviewed-on: https://boringssl-review.googlesource.com/5871
Reviewed-by: Adam Langley <agl@google.com>
clang scan-build found a memory leak if the overflow codepath in
dtls1_hm_fragment is hit. Along the way, tidy up that function.
Change-Id: I3c4b88916ee56ab3ab63f93d4a967ceae381d187
Reviewed-on: https://boringssl-review.googlesource.com/5870
Reviewed-by: Adam Langley <agl@google.com>
SSL_get_client_CA_list is one of those dreaded functions which may query either
configuration state or handshake state. Moreover, it does so based on
|ssl->server|, which may not be configured until later. Also check
|ssl->handshake_func| to make sure |ssl| is not in an indeterminate state.
This also fixes a bug where SSL_get_client_CA_list wouldn't work in DTLS due to
the incorrect |ssl->version| check.
Change-Id: Ie564dbfeecd2c8257fd6bcb148bc5db827390c77
Reviewed-on: https://boringssl-review.googlesource.com/5827
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: Ifa44fef160fc9d67771eed165f8fc277f28a0222
Reviewed-on: https://boringssl-review.googlesource.com/5840
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It was checking algorithm_mac rather than algorithm_enc.
(Coincidentally, it gave the right answer if you compiled out the
ChaCha20 ciphers since SSL_AES128GCM and SSL_AEAD shared a value.)
Change-Id: I17047425ef7fabb98969144965d8db9528ef8497
Reviewed-on: https://boringssl-review.googlesource.com/5850
Reviewed-by: Adam Langley <agl@google.com>
The size of the stack caused by this object is problematic for systems
that have smaller stacks because they expect many threads.
Change-Id: Ib8f03741f9dd96bf474126f001947f879e50a781
Reviewed-on: https://boringssl-review.googlesource.com/5831
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
There's not enough in that file to really justify its own file now.
Change-Id: I6130cfce6c40fe9d46aa83dd83e6f38d87fdcf64
Reviewed-on: https://boringssl-review.googlesource.com/5823
Reviewed-by: Adam Langley <agl@google.com>
Quite a lot of consumers of the SSL stack will never need to touch files from
the SSL stack, but enough do that we can't just ditch them. Toss that all into
their own file so a static linker can drop it.
Change-Id: Ia07de939889eb09e3ab16aebcc1b6869ca8b75a0
Reviewed-on: https://boringssl-review.googlesource.com/5820
Reviewed-by: Adam Langley <agl@google.com>
Applications may require the stapled OCSP response in order to verify
the certificate within the verification callback.
Change-Id: I8002e527f90c3ce7b6a66e3203c0a68371aac5ec
Reviewed-on: https://boringssl-review.googlesource.com/5730
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change adds the ability to configure ciphers specifically for
TLS ≥ 1.0. This compliments the existing ability to specify ciphers
for TLS ≥ 1.1.
This is useful because TLS 1.0 is the first version not to suffer from
POODLE. (Assuming that it's implemented correctly[1].) Thus one might
wish to reserve RC4 solely for SSLv3.
[1] https://www.imperialviolet.org/2014/12/08/poodleagain.html
Change-Id: I774d5336fead48f03d8a0a3cf80c369692ee60df
Reviewed-on: https://boringssl-review.googlesource.com/5793
Reviewed-by: Adam Langley <agl@google.com>
SSL_CTX gets memset to zero, so many of the values needn't be explicitly
initialized.
Change-Id: I0e707a0dcc052cd6f0a5dc8d037400170bd75594
Reviewed-on: https://boringssl-review.googlesource.com/5812
Reviewed-by: Adam Langley <agl@google.com>
setup_key_block is called when the first CCS resolves, but for resumptions this
is the incoming CCS (see ssl3_do_change_cipher_spec). Rather than set
need_record_splitting there, it should be set in the write case of
tls1_change_cipher_state.
This fixes a crash from the new record layer code in resumption when
record-splitting is enabled. Tweak the record-splitting tests to cover this
case.
This also fixes a bug where renego from a cipher which does require record
splitting to one which doesn't continues splitting. Since version switches are
not allowed, this can only happen after a renego from CBC to RC4.
Change-Id: Ie4e1b91282b10f13887b51d1199f76be4fbf09ad
Reviewed-on: https://boringssl-review.googlesource.com/5787
Reviewed-by: Adam Langley <agl@google.com>
Note that DTLS treats oversized ciphertexts different from everything else.
Change-Id: I71cba69ebce0debdfc96a7fdeb2666252e8d28ed
Reviewed-on: https://boringssl-review.googlesource.com/5786
Reviewed-by: Adam Langley <agl@google.com>
If the two extensions select different next protocols (quite possible since one
is server-selected and the other is client-selected), things will break. This
matches the behavior of NSS (Firefox) and Go.
Change-Id: Ie1da97bf062b91a370c85c12bc61423220a22f36
Reviewed-on: https://boringssl-review.googlesource.com/5780
Reviewed-by: Adam Langley <agl@google.com>
The maximum buffer size computation wasn't quite done right in
ssl_buffer.c, so we were failing with BUFFER_TOO_SMALL for sufficiently
large records. Fix this and, as penance, add 103 tests.
(Test that we can receive maximum-size records in all cipher suites.
Also test SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER while I'm here.)
BUG=526998
Change-Id: I714c16dda2ed13f49d8e6cd1b48adc5a8491f43c
Reviewed-on: https://boringssl-review.googlesource.com/5785
Reviewed-by: Adam Langley <agl@google.com>
The bidi shutdown code uses type = 0 as a special signal value, but code
elsewhere doesn't account for this.
BUG=526437
Change-Id: I090cee421633d70ef3b84f4daa811608031b9ed9
Reviewed-on: https://boringssl-review.googlesource.com/5771
Reviewed-by: Adam Langley <agl@google.com>
Bidirectional shutdown doesn't make sense over DTLS; you can't reuse the
underlying channel after receiving close_notify because the channel is
unordered. This removes one caller of dtls1_read_bytes.
Really close_notify makes no sense in DTLS. It can't even protect
against some kind of truncation because it's all unordered. But continue
to send it in case anything is (unreliably since the channel is lossy)
relying on close_notify to signal some kind of session end. This only
makes SSL_shutdown stop trying to wait for one once we've already
decided to shut down the connection.
BUG=526437
Change-Id: I6afad7cb7209c4aba0b96f9246b04c81d90987a9
Reviewed-on: https://boringssl-review.googlesource.com/5770
Reviewed-by: Adam Langley <agl@google.com>
Now that it even works at all (type = 0 bug aside), add tests for it.
Test both close_notify being received before and after SSL_shutdown is
called. In the latter case, have the peer send some junk to be ignored
to test that works.
Also test that SSL_shutdown fails on unclean shutdown and that quiet
shutdowns ignore it.
BUG=526437
Change-Id: Iff13b08feb03e82f21ecab0c66d5f85aec256137
Reviewed-on: https://boringssl-review.googlesource.com/5769
Reviewed-by: Adam Langley <agl@google.com>
When discarding a record, it's important to start reading the next one,
or the state machine retry signaling doesn't work.
BUG=526437
Change-Id: I5e4a5155310d097c0033cdf5d06712410a01ee08
Reviewed-on: https://boringssl-review.googlesource.com/5768
Reviewed-by: Adam Langley <agl@google.com>
The handshake state machine is still rather messy (we should switch to CBB,
split the key exchanges apart, and also pull reading and writing out), but this
version makes it more obvious to the compiler that |p| and |sig_len| are
initialized. The old logic created a synchronous-only state which, if enterred
directly, resulted in some variables being uninitialized.
Change-Id: Ia3ac9397d523fe299c50a95dc82a9b26304cea96
Reviewed-on: https://boringssl-review.googlesource.com/5765
Reviewed-by: Adam Langley <agl@google.com>
Our tests shouldn't panic if the program misbehaves.
Change-Id: I113e050222bcf48e5f25883f860dbc1c5c77e77e
Reviewed-on: https://boringssl-review.googlesource.com/5764
Reviewed-by: Adam Langley <agl@google.com>
Move cert_chain to the SSL_SESSION. Now everything on an SSL_SESSION is
properly serialized. The cert_chain field is, unfortunately, messed up
since it means different things between client and server.
There exists code which calls SSL_get_peer_cert_chain as both client and
server and assumes the existing semantics for each. Since that function
doesn't return a newly-allocated STACK_OF(X509), normalizing between the
two formats is a nuisance (we'd either need to store both cert_chain and
cert_chain_full on the SSL_SESSION or create one of the two variants
on-demand and stash it into the SSL).
This CL does not resolve this and retains the client/server difference
in SSL_SESSION. The SSL_SESSION serialization is a little inefficient
(two copies of the leaf certificate) for a client, but clients don't
typically serialize sessions. Should we wish to resolve it in the
future, we can use a different tag number. Because this was historically
unserialized, existing code must already allow for cert_chain not being
preserved across i2d/d2i.
In keeping with the semantics of retain_only_sha256_of_client_certs,
cert_chain is not retained when that flag is set.
Change-Id: Ieb72fc62c3076dd59750219e550902f1ad039651
Reviewed-on: https://boringssl-review.googlesource.com/5759
Reviewed-by: Adam Langley <agl@google.com>
The difference of two pointers is signed, even though it's always
non-negative here, so MSVC is complaining about signedness mismatch.
Change-Id: I5a042d06ed348540706b93310af3f60f3ab5f303
Reviewed-on: https://boringssl-review.googlesource.com/5766
Reviewed-by: Adam Langley <agl@google.com>
Rather than parse the fields in two passes, group the code relating to
one field together. Somewhat less annoying to add new fields. To keep
this from getting too unwieldy, add a few more helper functions for the
common field types.
Change-Id: Ia86c6bbca9dd212d5c35029363ea4d6b6426164a
Reviewed-on: https://boringssl-review.googlesource.com/5758
Reviewed-by: Adam Langley <agl@google.com>
It's completely redundant with the copy in the SSL_SESSION except it
isn't serialized.
Change-Id: I1d95a14cae064c599e4bab576df1dd156da4b81c
Reviewed-on: https://boringssl-review.googlesource.com/5757
Reviewed-by: Adam Langley <agl@google.com>
Gets another field out of the SSL_SESSION.
Change-Id: I9a27255533f8e43e152808427466ec1306cfcc60
Reviewed-on: https://boringssl-review.googlesource.com/5756
Reviewed-by: Adam Langley <agl@google.com>
The old empty record logic discarded the records at a very low-level.
Let the error bubble up to ssl3_read_bytes so the type mismatch logic
may kick in before the empty record is skipped.
Add tests for when the record in question is application data, before
before the handshake and post ChangeCipherSpec.
BUG=521840
Change-Id: I47dff389cda65d6672b9be39d7d89490331063fa
Reviewed-on: https://boringssl-review.googlesource.com/5754
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>
Apparently we left those as zero. Oh well. This only affects
SSL_CIPHER_get_bits, but so long as we have the field, it ought to be correct.
Change-Id: I2878ec22c2f5a6263f805e04d9fd8448994639b7
Reviewed-on: https://boringssl-review.googlesource.com/5752
Reviewed-by: Adam Langley <agl@google.com>
This made sense when the cipher might have been standardized as-is, so a
DHE_RSA variant could appease the IETF. Since the standardized variant is going
to have some nonce tweaks anyway, there's no sense in keeping this around. Get
rid of one non-standard cipher suite value early. (Even if they were to be
standardized as-is, it's not clear we should implement new DHE cipher suites at
this point.)
Chrome UMA, unsurprisingly, shows that it's unused.
Change-Id: Id83d73a4294b470ec2e94d5308fba135d6eeb228
Reviewed-on: https://boringssl-review.googlesource.com/5750
Reviewed-by: Adam Langley <agl@google.com>
This is a simpler implementation than OpenSSL's, lacking responder IDs
and request extensions support. This mirrors the client implementation
already present.
Change-Id: I54592b60e0a708bfb003d491c9250401403c9e69
Reviewed-on: https://boringssl-review.googlesource.com/5700
Reviewed-by: Adam Langley <agl@google.com>
They get initialized in SSL_new and SSL_CTX_new, respectively.
Change-Id: Ib484108987a99f654d1a77fc473103f5cb393bd7
Reviewed-on: https://boringssl-review.googlesource.com/5676
Reviewed-by: Adam Langley <agl@google.com>
They're not called (new in 1.0.2). We actually may well need to
configure these later to strike ECDSA from the list on Chrome/XP
depending on what TLS 1.3 does, but for now striking it from the cipher
suite list is both necessary and sufficient. I think we're better off
removing these for now and adding new APIs later if we need them.
(This API is weird. You pass in an array of NIDs that must be even
length and alternating between hash and signature NID. We'd also need a
way to query the configured set of sigalgs to filter away. Those used to
exist but were removed in
https://boringssl-review.googlesource.com/#/c/5347/. SSL_get_sigalgs is
an even uglier API and doesn't act on the SSL_CTX.)
And with that, SSL_ctrl and SSL_CTX_ctrl can *finally* be dropped. Don't
leave no-op wrappers; anything calling SSL_ctrl and SSL_CTX_ctrl should
instead switch to the wrapper macros.
BUG=404754
Change-Id: I5d465cd27eef30d108eeb6de075330c9ef5c05e8
Reviewed-on: https://boringssl-review.googlesource.com/5675
Reviewed-by: Adam Langley <agl@google.com>
I'm not sure why one would ever want to externally know the curve list
supported by the server. The API is new as of 1.0.2 and has no callers.
Configuring curves will be much more useful when Curve25519 exists and the API
isn't terribly crazy, so keep that API around and promote it to a real
function.
BUG=404754
Change-Id: Ibd5858791d3dfb30d53dd680cb75b0caddcbb7df
Reviewed-on: https://boringssl-review.googlesource.com/5674
Reviewed-by: Adam Langley <agl@google.com>
This change stores the size of the group/modulus (for RSA/DHE) or curve
ID (for ECDHE) in the |SSL_SESSION|. This makes it available for UIs
where desired.
Change-Id: I354141da432a08f71704c9683f298b361362483d
Reviewed-on: https://boringssl-review.googlesource.com/5280
Reviewed-by: Adam Langley <agl@google.com>
This guarantees that we never read beyond the first record, even if the
first record is empty. Between removing SSL_set_read_ahead and DTLS
enforcing record boundaries, this means the buffer need never memmove
data.
The memmove isn't really much of a burden and we can probably just put
SSL_set_read_ahead back after the cleanup if desired. But while the
non-existant read_ahead is off, we should avoid reading more than needed.
(Also the current memmove logic is completely wrong for TLS. Checking
align != 0 doesn't make sense. The real reason to memmove is that the
next record may still be full size. So now line 209 of s3_pkt.c should
*actually* be unreachable.)
SSL_R_HTTPS_PROXY_REQUEST detection is now slightly less accurate, but
OpenSSL was already not parsing HTTP completely. We could asynchronously
read the extra 3 bytes once the first 5 match, but that seems
unnecessary. (Shall we just get rid of all these HTTP detectors? The
only consumer of those error codes is some diagnostics logic.)
BUG=468889
Change-Id: Ie3bf148ae7274795e1d048d78282d1d8063278ea
Reviewed-on: https://boringssl-review.googlesource.com/5714
Reviewed-by: Adam Langley <agl@google.com>
I'm not sure why I made a separate one. (Not quite how the V2ClientHello
code will look in the buffer-free API yet. Probably the future
refactored SSL_HANDSHAKE gadget will need separate entry points to
consume a handshake message or V2ClientHello and the driver deals with
framing.)
This also means that ssl3_setup_read_buffer is never called external to
ssl3_read_n.
BUG=468889
Change-Id: I872f1188270968bf53ee9d0488a761c772a11e9e
Reviewed-on: https://boringssl-review.googlesource.com/5713
Reviewed-by: Adam Langley <agl@google.com>
Rather than sniff for ClientHello, just fall through to standard logic
once weird cases are resolved.
This means that garbage will now read as WRONG_VERSION rather than
UNKNOWN_PROTOCOL, but the rules here were slightly odd anyway. This also
means we'll now accept empty records before the ClientHello (up to the
empty record limit), and process records of the wrong type with the
usual codepath during the handshake.
This shouldn't be any more risk as it just makes the ClientHello more
consistent with the rest of the protocol. A TLS implementation that
doesn't parse V2ClientHello would do the same unless it still
special-cased the first record. All newly-exposed states are reachable
by fragmenting ClientHello by one byte and then sending the record in
question.
BUG=468889
Change-Id: Ib701ae5d8adb663e158c391639b232a9d9cd1c6e
Reviewed-on: https://boringssl-review.googlesource.com/5712
Reviewed-by: Adam Langley <agl@google.com>
This isn't called and, with the fixed-DH client cert types removed, is
only useful if a server wishes to not accept ECDSA certificates or
something.
BUG=404754
Change-Id: I21d8e1a71aedf446ce974fbeadc62f311ae086db
Reviewed-on: https://boringssl-review.googlesource.com/5673
Reviewed-by: Adam Langley <agl@google.com>
These are unused (new as of 1.0.2). Although being able to separate the
two stores is a reasonable thing to do, we hope to remove the
auto-chaining feature eventually. Given that, SSL_CTX_set_cert_store
should suffice. This gets rid of two more ctrl macros.
BUG=404754,486295
Change-Id: Id84de95d7b2ad5a14fc68a62bb2394f01fa67bb4
Reviewed-on: https://boringssl-review.googlesource.com/5672
Reviewed-by: Adam Langley <agl@google.com>
Due to a typo, when a server sent an unknown extension, the extension
number would be taken from a NULL structure rather than from the
variable of the same name that's in the local scope.
BUG=517935
Change-Id: I29d5eb3c56cded40f6155a81556199f12439ae06
Reviewed-on: https://boringssl-review.googlesource.com/5650
Reviewed-by: Adam Langley <agl@google.com>
Rather than support arbitrarily many handshake hashes in the general
case (which the PRF logic assumes is capped at two), special-case the
MD5/SHA1 two-hash combination and otherwise maintain a single rolling
hash.
Change-Id: Ide9475565b158f6839bb10b8b22f324f89399f92
Reviewed-on: https://boringssl-review.googlesource.com/5618
Reviewed-by: Adam Langley <agl@google.com>
Rather than iterate over handshake_dgsts itself, it can just call
tls1_handshake_digest.
Change-Id: Ia518da540e47e65b13367eb1af184c0885908488
Reviewed-on: https://boringssl-review.googlesource.com/5617
Reviewed-by: Adam Langley <agl@google.com>
A memory BIO is internally a BUF_MEM anyway. There's no need to bring
BIO_write into the mix. BUF_MEM is size_t clean.
Change-Id: I4ec6e4d22c72696bf47c95861771013483f75cab
Reviewed-on: https://boringssl-review.googlesource.com/5616
Reviewed-by: Adam Langley <agl@google.com>
The handshake hash is initialized from the buffer as soon as the cipher
is known. When adding a message to the transcript, independently update
the buffer and rolling hash, whichever is active. This avoids the
complications around dont_free_handshake_buffer and EMS.
BUG=492371
Change-Id: I3b1065796a50fd1be5d42ead7210c2f253ef0aca
Reviewed-on: https://boringssl-review.googlesource.com/5615
Reviewed-by: Adam Langley <agl@google.com>
It's purely the PRF function now, although it's still different from the
rest due to the _DEFAULT field being weird.
Change-Id: Iaea7a99cccdc8be4cd60f6c1503df5be2a63c4c5
Reviewed-on: https://boringssl-review.googlesource.com/5614
Reviewed-by: Adam Langley <agl@google.com>
It's a property of just algorithm_enc and hopefully AES-GCM will
continue to be the only true AEAD that requires this. Simpler to just
keep it in ssl_aead_ctx.c.
Change-Id: Ib7c060a3de2fa8590b2dc36c23a5d5fabff43b07
Reviewed-on: https://boringssl-review.googlesource.com/5613
Reviewed-by: Adam Langley <agl@google.com>
Take the sequence number as a parameter. Also replace satsub64be with
the boring thing: convert to uint64_t and subtract normally.
BUG=468889
Change-Id: Icab75f872b5e55cf4e9d68b66934ec91afeb198b
Reviewed-on: https://boringssl-review.googlesource.com/5558
Reviewed-by: Adam Langley <agl@google.com>
The split was only needed for buffering records. Likewise, the extra
seq_num field is now unnecessary.
This also fixes a bug where dtls1_process_record will push an error on
the queue if the decrypted record is too large, which dtls1_get_record
will ignore but fail to clear, leaving garbage on the error queue. The
error is now treated as fatal; the reason DTLS silently drops invalid
packets is worrying about ease of DoS, but after SSL_AEAD_CTX_open, the
packet has been authenticated. (Unless it's the null cipher, but that's
during the handshake and the handshake is already DoS-able by breaking
handshake reassembly state.)
The function is still rather a mess. Later changes will clean this up.
BUG=468889
Change-Id: I96a54afe0755d43c34456f76e77fc4ee52ad01e3
Reviewed-on: https://boringssl-review.googlesource.com/5557
Reviewed-by: Adam Langley <agl@google.com>
The existing tests only went monotonic. Allow an arbitrary mapping
function. Also test by sending more app data. The handshake is fairly
resilient to replayed packets, whereas our test code intentionally
isn't.
Change-Id: I0fb74bbacc260c65ec5f6a1ca8f3cb23b4192855
Reviewed-on: https://boringssl-review.googlesource.com/5556
Reviewed-by: Adam Langley <agl@google.com>
The only point format that we ever support is uncompressed, which the
RFC says implementations MUST support. The TLS 1.3 and Curve25519
forecast is that point format negotiation is gone. Each curve has just
one point format and it's labeled, for historial reasons, as
"uncompressed".
Change-Id: I8ffc8556bed1127cf288d2a29671abe3c9b3c585
Reviewed-on: https://boringssl-review.googlesource.com/5542
Reviewed-by: Adam Langley <agl@google.com>
I don't think we had coverage for this check.
Change-Id: I5e454e69c1ee9f1b9760d2ef1431170d76f78d63
Reviewed-on: https://boringssl-review.googlesource.com/5544
Reviewed-by: Adam Langley <agl@google.com>
That got out of sync at some point.
Change-Id: I5a45f50f330ceb65053181afc916053a80aa2c5d
Reviewed-on: https://boringssl-review.googlesource.com/5541
Reviewed-by: Adam Langley <agl@google.com>
It's never called anywhere and doesn't return anything interesting.
Change-Id: I68e7e9cd7b74a72f61092ac5d2b5d2390e55a228
Reviewed-on: https://boringssl-review.googlesource.com/5540
Reviewed-by: Adam Langley <agl@google.com>
The RSA key exchange needs decryption and is still unsupported.
Change-Id: I8c13b74e25a5424356afbe6e97b5f700a56de41f
Reviewed-on: https://boringssl-review.googlesource.com/5467
Reviewed-by: Adam Langley <agl@google.com>
This change mirrors upstream's custom extension API because we have some
internal users that depend on it.
Change-Id: I408e442de0a55df7b05c872c953ff048cd406513
Reviewed-on: https://boringssl-review.googlesource.com/5471
Reviewed-by: Adam Langley <agl@google.com>
These are not in upstream and were probably introduced on accident by stray vim
keystrokes.
Change-Id: I35f51f81fc37e75702e7d8ffc6f040ce71321b54
Reviewed-on: https://boringssl-review.googlesource.com/5490
Reviewed-by: Adam Langley <agl@google.com>
With the fastradio stuff gone, the padding computation is slightly more
straight-forward.
Change-Id: I67ede92fdf5f34c265c7a44e4cdc1a5ce5416df2
Reviewed-on: https://boringssl-review.googlesource.com/5482
Reviewed-by: Adam Langley <agl@google.com>
This sort of test is more suitable for ssl_test than runner. This should
stress all the various cases around padding. Use tickets rather than
hostnames to inflate the ClientHello because there's a fairly tight
maximum hostname length.
Change-Id: Ibd43aaa7acb9bf5fa00a9d2548d2101e5bb147d3
Reviewed-on: https://boringssl-review.googlesource.com/5480
Reviewed-by: Adam Langley <agl@google.com>
These were used in the upstream Go code to fuzz-test the handshake
marshal/unmarshal functions. But we don't do that there so best to
remove them.
(The ClientHello equals function is still used, however, to test DTLS
retransmission.)
Change-Id: I950bdf4f7eefa2bca13c10f5328d2e6c586604e2
Reviewed-on: https://boringssl-review.googlesource.com/5470
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Fastradio was a trick where the ClientHello was padding to at least 1024
bytes in order to trick some mobile radios into entering high-power mode
immediately. After experimentation, the feature is being dropped.
This change also tidies up a bit of the extensions code now that
everything is using the new system.
Change-Id: Icf7892e0ac1fbe5d66a5d7b405ec455c6850a41c
Reviewed-on: https://boringssl-review.googlesource.com/5466
Reviewed-by: Adam Langley <agl@google.com>
This also removes support for the “old” Channel ID extension.
Change-Id: I1168efb9365c274db6b9d7e32013336e4404ff54
Reviewed-on: https://boringssl-review.googlesource.com/5462
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I5777b73f485da6534b407e6c531f8293898b9c06
Reviewed-on: https://boringssl-review.googlesource.com/5461
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I3b085cd13295e83c578c549763f0de82f39499a2
Reviewed-on: https://boringssl-review.googlesource.com/5460
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
In the ancient times, before ex_data and OpenSSL, SSLeay supported a
single app_data slot in various types. Later app_data begat ex_data, and
app_data was replaced by compatibility macros to ex_data index zero.
Today, app_data is still in use, but ex_data never reserved index zero
for app_data. This causes some danger where, if the first ex_data
registration did not use NULL callbacks, the registration's callbacks
would collide with app_data.
Instead, add an option to the types with app_data to reserve index zero.
Also switch SSL_get_ex_data_X509_STORE_CTX_idx to always return zero
rather than allocate a new one. It used to be that you used
X509_STORE_CTX_get_app_data. I only found one consumer that we probably
don't care about, but, to be safe and since it's easy, go with the
conservative option. (Although SSL_get_ex_data_X509_STORE_CTX_idx wasn't
guaranteed to alias app_data, in practice it always did. No consumer
ever calls X509_STORE_CTX_get_ex_new_index.)
Change-Id: Ie75b279d60aefd003ffef103f99021c5d696a5e9
Reviewed-on: https://boringssl-review.googlesource.com/5313
Reviewed-by: Adam Langley <agl@google.com>
Chromium's NaCl build has _POSIX_SOURCE already defined, so #undef it first.
The compiler used also dislikes static asserts with the same name.
Change-Id: I0283fbad1a2ccf98cdb0ca2a7965b15441806308
Reviewed-on: https://boringssl-review.googlesource.com/5430
Reviewed-by: Adam Langley <agl@google.com>
