It can be folded into dtls1_read_app_data. This code, since it still takes an
output pointer, does not yet process records atomically. (Though, being DTLS,
it probably should...)
Change-Id: I57d60785c9c1dd13b5b2ed158a08a8f5a518db4f
Reviewed-on: https://boringssl-review.googlesource.com/8177
Reviewed-by: David Benjamin <davidben@google.com>
This was probably the worst offender of them all as read_bytes is the wrong
abstraction to begin with. Note this is a slight change in how processing a
record works. Rather than reading one fragment at a time, we process all
fragments in a record and return. The intent here is so that all records are
processed atomically since the connection eventually will not be able to retain
a buffer holding the record.
This loses a ton of (though not quite all yet) those a2b macros.
Change-Id: Ibe4bbcc33c496328de08d272457d2282c411b38b
Reviewed-on: https://boringssl-review.googlesource.com/8176
Reviewed-by: David Benjamin <davidben@google.com>
read_close_notify is a very straight-forward hook and doesn't need much.
Change-Id: I7407d842321ea1bcb47838424a0d8f7550ad71ca
Reviewed-on: https://boringssl-review.googlesource.com/8174
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The business with ssl_record_prefix_len is rather a hassle. Instead, have
tls_open_record always decrypt in-place and give back a CBS to where the body
is.
This way the caller doesn't need to do an extra check all to avoid creating an
invalid pointer and underflow in subtraction.
Change-Id: I4e12b25a760870d8f8a503673ab00a2d774fc9ee
Reviewed-on: https://boringssl-review.googlesource.com/8173
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Alert handling is more-or-less identical across all contexts. Push it down from
read_bytes into the low-level record functions. This also deduplicates the code
shared between TLS and DTLS.
Now the only type mismatch managed by read_bytes is if we get handshake data in
read_app_data.
Change-Id: Ia8331897b304566e66d901899cfbf31d2870194e
Reviewed-on: https://boringssl-review.googlesource.com/8124
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is getting a little repetitive.
Change-Id: Ib0fa8ab10149557c2d728b88648381b9368221d9
Reviewed-on: https://boringssl-review.googlesource.com/8126
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Move this logic out of dtls1_read_bytes and into dtls1_get_record. Only trigger
it when reading from the buffer fails. The other one shouldn't be necessary.
This exists to handle the blocking BIO case when the
BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT signal triggers, so we only need to do it when
timeouts actually trigger.
There also doesn't seem to be a need for most of the machinery. The
BIO_set_flags call seems to be working around a deficiency in the underlying
BIO. There also shouldn't be a need to check the handshake state as there
wouldn't be a timer to restart otherwise.
Change-Id: Ic901ccfb5b82aeb409d16a9d32c04741410ad6d7
Reviewed-on: https://boringssl-review.googlesource.com/8122
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
The two modes are quite different. One of them requires the BIO honor an
extra BIO_ctrl. Also add an explanation at the top of
addDTLSRetransmitTests for how these tests work. The description is
scattered across many different places.
BUG=63
Change-Id: Iff4cdd1fbf4f4439ae0c293f565eb6780c7c84f9
Reviewed-on: https://boringssl-review.googlesource.com/8121
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's f792c663048f19347a1bb72125e535e4fb2ecf39.)
Change-Id: If9bbb10de3ea858076bd9587d21ec331e837dd53
Reviewed-on: https://boringssl-review.googlesource.com/8171
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Operations in the DSA signing algorithm should run in constant time in
order to avoid side channel attacks. A flaw in the OpenSSL DSA
implementation means that a non-constant time codepath is followed for
certain operations. This has been demonstrated through a cache-timing
attack to be sufficient for an attacker to recover the private DSA key.
CVE-2016-2178
(Imported from upstream's 621eaf49a289bfac26d4cbcdb7396e796784c534 and
b7d0f2834e139a20560d64c73e2565e93715ce2b.)
We should eventually not depend on BN_FLG_CONSTTIME since it's a mess (seeing
as the original fix was wrong until we reported b7d0f2834e to them), but, for
now, go with the simplest fix.
Change-Id: I9ea15c1d1cc3a7e21ef5b591e1879ec97a179718
Reviewed-on: https://boringssl-review.googlesource.com/8172
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
We've got it in entry points. That should be sufficient. (Do we even need it
there?)
Change-Id: I39b245a08fcde7b57e61b0bfc595c6ff4ce2a07a
Reviewed-on: https://boringssl-review.googlesource.com/8127
Reviewed-by: David Benjamin <davidben@google.com>
This cannot happen.
Change-Id: Ib1b473aa91d6479eeff43f7eaf94906d0b2c2a8f
Reviewed-on: https://boringssl-review.googlesource.com/8123
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
ssl->cert is never NULL. It gets created in SSL_new unconditionally.
Change-Id: I5c54c9c73e281e61a554820d61421226d763d33a
Reviewed-on: https://boringssl-review.googlesource.com/8125
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
More spring-cleaning of unnecessary incompatibilities. Since
OpenSSL_add_all_algorithms_conf doesn't specify a configuration file, it's
perfectly sound to have such a function.
Dear BoringSSL, please add all algorithms.
Uh, sure. They were already all there, but I have added them!
PS: Could you also load all your configuration files while you're at it.
...I don't have any. Fine. I have loaded all configuration files which I
recognize. *mutters under breath* why does everyone ask all these strange
questions...
Change-Id: I57f956933d9e519445bf22f89853bd5f56904172
Reviewed-on: https://boringssl-review.googlesource.com/8160
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Some files were named 𝑥_test.txt and some 𝑥_tests.txt. This change
unifies around the latter.
Change-Id: Id6f29bad8b998f3c3466655097ef593f7f18f82f
Reviewed-on: https://boringssl-review.googlesource.com/8150
Reviewed-by: David Benjamin <davidben@google.com>
The fake numbers collide with other numbers defined below. Also PUSH and POP
are actually used. DUP legitimately isn't though.
Change-Id: Iaa15a065d846b89b9b7958b78068393cfee2bd6f
Reviewed-on: https://boringssl-review.googlesource.com/8143
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Some OpenSSL consumers use them, so provide no-op versions to make porting code
easier.
Change-Id: I4348568c1cb08d2b2c0a9ec9a17e2c0449260965
Reviewed-on: https://boringssl-review.googlesource.com/8142
Reviewed-by: David Benjamin <davidben@google.com>
Make building against software that expects OpenSSL easier.
Change-Id: I1af090ae8208218d6e226ee0baf51053699d85cc
Reviewed-on: https://boringssl-review.googlesource.com/8141
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Windows is, not unreasonably, complaining that taking abs() of an unsigned is
ridiculous. But these values actually are signed and fit very easily in an int
anyway.
Change-Id: I34fecaaa3616732112e3eea105a7c84bd9cd0bae
Reviewed-on: https://boringssl-review.googlesource.com/8144
Reviewed-by: Adam Langley <agl@google.com>
doc.go is still a little unhappy.
Change-Id: I5a8f3da91dabb45d29d0e08f13b7dabdcd521c38
Reviewed-on: https://boringssl-review.googlesource.com/8145
Reviewed-by: David Benjamin <davidben@google.com>
Otherwise builds fail with:
crypto/newhope/newhope_statistical_test.cc:136:27: error: format specifies type 'long' but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
Change-Id: I85d5816c1d7ee71eef362bffe983b2781ce310a4
One of these tests the distribution of noise polynomials; the other
tests that that agreed-upon keys (prior to whitening) have roughly equal
numbers of 0s and 1s.
Along the way, expose a few more API bits.
Change-Id: I6b04708d41590de45d82ea95bae1033cfccd5d67
Reviewed-on: https://boringssl-review.googlesource.com/8130
Reviewed-by: Adam Langley <agl@google.com>
In TLS 1.3, the actual record type is hidden within the encrypted data
and the record layer defaults to using a TLS 1.0 {3, 1} record version
for compatibility. Additionally the record layer no longer checks the
minor version of the record layer to maintain compatibility with the
TLS 1.3 spec.
Change-Id: If2c08e48baab170c1658e0715c33929d36c9be3a
Reviewed-on: https://boringssl-review.googlesource.com/8091
Reviewed-by: David Benjamin <davidben@google.com>
They match the new style not the old EAY style now. They're also not
likely to be reformatted. It's just the legacy ASN.1 stuff now and we're
intentionally not doing much with those. (The old text was written back
before the SSL stack had been reformatted.)
Change-Id: I4852761b013e8c2688ebc7eaf4970afbdc69e858
Reviewed-on: https://boringssl-review.googlesource.com/8129
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Assembly code for X25519 wasn't included on OS X when built with
build systems other than CMake, which lead to a SIGTRAP due to a
missing x25519_x86_64.
Reported by Gurgen Hrachyan.
Change-Id: Ib6026f31cce0405ec3e75d8a52bf0940e57c62c8
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/8111
Reviewed-by: David Benjamin <davidben@google.com>
This commit adds coverage of the "offer" (first) step, as well as
testing all outputs of the "accept" (second) step, not just the shared
key.
Change-Id: Id11fe24029abc302442484a6c01fa496a1578b3a
Reviewed-on: https://boringssl-review.googlesource.com/8100
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL was actually super-buggy here (though known bugs on our end have been
fixed), but pyOpenSSL was confused and incorrectly documented that callers call
SSL_read after SSL_shutdown to do bidi shutdown, so we should probably support
this. Add a test that it works.
Change-Id: I2b6d012161330aeb4cf894bae3a0b6a55d53c70d
Reviewed-on: https://boringssl-review.googlesource.com/8093
Reviewed-by: Adam Langley <agl@google.com>
This keeps the naming convention in line with the actual spec.
Change-Id: I34673f78dbc29c1659b4da0e49677ebe9b79636b
Reviewed-on: https://boringssl-review.googlesource.com/8090
Reviewed-by: David Benjamin <davidben@google.com>
The test vectors are taken from the reference implementation, modified
to output the results of its random-number generator, and the results of
key generation prior to SHA3. This allows the interoperability of the
two implementations to be tested somewhat.
To accomplish the testing, this commit creates a new, lower-level API
that leaves the generation of random numbers and all wire encoding and
decoding up to the caller.
Change-Id: Ifae3517696dde4be4a0b7c1998bdefb789bac599
Reviewed-on: https://boringssl-review.googlesource.com/8070
Reviewed-by: Adam Langley <agl@google.com>
Now that we no longer support Windows XP, this is available.
Unfortunately, the public header version of CRYPTO_MUTEX means we
still can't easily merge CRYPTO_MUTEX and CRYPTO_STATIC_MUTEX.
BUG=37
Change-Id: If309de3f06e0854c505083b72fd64d1dbb3f4563
Reviewed-on: https://boringssl-review.googlesource.com/8081
Reviewed-by: Adam Langley <agl@google.com>
Windows SRWLOCK requires you call different functions here. Split
them up in preparation for switching Windows from CRITICAL_SECTION.
BUG=37
Change-Id: I7b5c6a98eab9ae5bb0734b805cfa1ff334918f35
Reviewed-on: https://boringssl-review.googlesource.com/8080
Reviewed-by: Adam Langley <agl@google.com>
This is easier to deploy, and more obvious. This commit reverts a few
pieces of e25775bc, but keeps most of it.
Change-Id: If8d657a4221c665349c06041bb12fffca1527a2c
Reviewed-on: https://boringssl-review.googlesource.com/8061
Reviewed-by: Adam Langley <agl@google.com>
This code has caused a long history of problems. This change rewrites it
completely with something that is, hopefully, much simplier and robust
and adds more testing.
Change-Id: Ibeef51f9386afd95d5b73316e451eb3a2d7ec4e0
Reviewed-on: https://boringssl-review.googlesource.com/8033
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit c7eae5a326. pyOpenSSL
expects to be able to call |SSL_read| after a shutdown and get EOF.
Change-Id: Icc5faa09d644ec29aac99b181dac0db197f283e3
Reviewed-on: https://boringssl-review.googlesource.com/8060
Reviewed-by: Adam Langley <agl@google.com>
The TLS 1.3 spec has an explicit nonce construction for AEADs that
requires xoring the IV and sequence number.
Change-Id: I77145e12f7946ffb35ebeeb9b2947aa51058cbe9
Reviewed-on: https://boringssl-review.googlesource.com/8042
Reviewed-by: Adam Langley <agl@google.com>
Constants representing TLS 1.3 are added to allow for future work to be
flagged on TLS1_3_VERSION. To prevent BoringSSL from negotiating the
non-existent TLS 1.3 version, it is explicitly disabled using
SSL_OP_NO_TLSv1_3.
Change-Id: Ie5258a916f4c19ef21646c4073d5b4a7974d6f3f
Reviewed-on: https://boringssl-review.googlesource.com/8041
Reviewed-by: Adam Langley <agl@google.com>
This renames the Channel ID EncryptedExtensions message to allow for
compatibility with TLS 1.3 EncryptedExtensions.
Change-Id: I5b67d00d548518045554becb1b7213fba86731f2
Reviewed-on: https://boringssl-review.googlesource.com/8040
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL's bbio logic is kind of crazy. It would be good to eventually do the
buffering in a better way (notably, bbio is fragile, if not outright broken,
for DTLS). In the meantime, this fixes a number of bugs where the existence of
bbio was leaked in the public API and broke things.
- SSL_get_wbio returned the bbio during the handshake. It must always return
the BIO the consumer configured. In doing so, internal accesses of
SSL_get_wbio should be switched to ssl->wbio since those want to see bbio.
For consistency, do the same with rbio.
- The logic in SSL_set_rfd, etc. (which I doubt is quite right since
SSL_set_bio's lifetime is unclear) would get confused once wbio got wrapped.
Those want to compare to SSL_get_wbio.
- If SSL_set_bio was called mid-handshake, bbio would get disconnected and lose
state. It forgets to reattach the bbio afterwards. Unfortunately, Conscrypt
does this a lot. It just never ended up calling it at a point where the bbio
would cause problems.
- Make more explicit the invariant that any bbio's which exist are always
attached. Simplify a few things as part of that.
Change-Id: Ia02d6bdfb9aeb1e3021a8f82dcbd0629f5c7fb8d
Reviewed-on: https://boringssl-review.googlesource.com/8023
Reviewed-by: Kenny Root <kroot@google.com>
Reviewed-by: Adam Langley <agl@google.com>
GetConfigPtr was a silly name. GetTestConfig matches the type and GetTestState.
Change-Id: I9998437a7be35dbdaab6e460954acf1b95375de0
Reviewed-on: https://boringssl-review.googlesource.com/8024
Reviewed-by: Adam Langley <agl@google.com>
The 'elliptic_curves' extension is being renamed to 'supported_groups'
in the TLS 1.3 draft, and most of the curve-specific methods are
generalized to groups/group IDs.
Change-Id: Icd1a1cf7365c8a4a64ae601993dc4273802610fb
Reviewed-on: https://boringssl-review.googlesource.com/7955
Reviewed-by: David Benjamin <davidben@google.com>
