I believe that case was the only way that X509_check_purpose could
return anything other than zero or one. Thus eliminate the last use of
X509_V_FLAG_X509_STRICT.
Change-Id: If2f071dfa934b924491db2b615ec17390564e7de
Reviewed-on: https://boringssl-review.googlesource.com/30344
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
OpenSSL 1.0.2 (and thus BoringSSL) accepts keyUsage certSign or a
Netscape CA certificate-type in lieu of basicConstraints in an
intermediate certificate (unless X509_V_FLAG_X509_STRICT) is set.
Update-Note: This change tightens the code so that basicConstraints is required for intermediate certificates when verifying chains. This was previously only enabled if X509_V_FLAG_X509_STRICT was set, but that flag also has other effects.
Change-Id: I9e41f4c567084cf30ed08f015a744959982940af
Reviewed-on: https://boringssl-review.googlesource.com/30185
Reviewed-by: Matt Braithwaite <mab@google.com>
This change adds a new flag, X509_V_FLAG_REQUIRE_CA_BASIC_CONSTRAINTS,
which causes basicConstraints with isCA to be required for intermediate
CA certificates. Without this, intermediates are also acceptable if
they're missing basicConstraints, but include either a certSign
keyUsage, or a CA Netscape certificate type.
This is a short-term change for patching. I'll undo a lot of it and make
this the default in the next change.
Change-Id: I7f42ffd76c57de3037f054108951e230c1b4e415
Reviewed-on: https://boringssl-review.googlesource.com/30184
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
The new binary, called |handshaker|, allows split-handshakes to be
tested using shim and handshaker binaries built at different
revisions.
The shim now proxies traffic to the handshaker during the split
handshake. The handoff and handback steps serialize additional state
about the test being performed, and its results.
The proxy and handshaker make heavy use of Unix-isms, and so
split-handshake tests are now restricted to Linux.
Change-Id: I048f0540c3978a31b3e573e00da17caf41a8059e
Reviewed-on: https://boringssl-review.googlesource.com/29348
Reviewed-by: Adam Langley <agl@google.com>
This change adds a function so that an ECDH and the hashing of the
resulting 'x' coordinate can occur inside the FIPS boundary.
Change-Id: If93c20a70dc9dcbca49056f10915d3ce064f641f
Reviewed-on: https://boringssl-review.googlesource.com/30104
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I33c5259f066693c912ba751dff0205ae240f4a92
Reviewed-on: https://boringssl-review.googlesource.com/29964
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Mostly in comments, but there is one special-case around renegotiation_info
that can now be removed.
Change-Id: I2a9114cbff05e0cfff95fe93270fe42379728012
Reviewed-on: https://boringssl-review.googlesource.com/29824
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Callers who use SSL_get0_certificate_types today will find an empty list
in TLS 1.3, which removed it. To provide feature parity, add an accessor
for the signature algorithms list. SSL_get_signature_algorithm_key_type
can be used to map it to a key type.
"Peer signature algorithms" was already taken in the public API by
SSL_get_peer_signature_algorithm to refer to which the peer selected, so
I named this matching SSL_CTX_set_verify_algorithm_prefs.
Change-Id: I12d411d7350e744ed9f88c610df48e0d9fc13256
Reviewed-on: https://boringssl-review.googlesource.com/29684
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Vartanian <flooey@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: I436cc772eb975ad989035ee154a2e050c65e2961
Reviewed-on: https://boringssl-review.googlesource.com/29664
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It's 2018, but passing STL objects across the API boundary turns out to
still be more bother than it's worth. Since we're dropping UniquePtr in
the API anyway, go the whole way and make it a plain-C API.
Change-Id: Ic0202012e5d81afe62d71b3fb57e6a27a8f63c65
Update-note: this will need corresponding changes to the internal use of SSL_CTX_add_cert_compression_alg.
Reviewed-on: https://boringssl-review.googlesource.com/29564
Reviewed-by: David Benjamin <davidben@google.com>
lh_FOO_retrieve is often called with a dummy instance of FOO that has
only a few fields filled in. This works fine for C, but a C++
SSL_SESSION with destructors is a bit more of a nuisance here.
Instead, teach LHASH to allow queries by some external key type. This
avoids stack-allocating SSL_SESSION. Along the way, fix the
make_macros.sh script.
Change-Id: Ie0b482d4ffe1027049d49db63274c7c17f9398fa
Reviewed-on: https://boringssl-review.googlesource.com/29586
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
bssl::UniquePtr and FOO_up_ref do not play well together. Add a helper
to simplify this. This allows us to write things like:
foo->cert = UpRef(bar->cert);
instead of:
if (bar->cert) {
X509_up_ref(bar->cert.get());
}
foo->cert.reset(bar->cert.get());
This also plays well with PushToStack. To append something to a stack
while taking a reference, it's just:
PushToStack(certs, UpRef(cert))
Change-Id: I99ae8de22b837588a2d8ffb58f86edc1d03ed46a
Reviewed-on: https://boringssl-review.googlesource.com/29584
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This was changed in draft-ietf-quic-tls-13 to use a codepoint from the
reserved range.
Change-Id: Ia3cda249a3f37bc244d5c8a7765ec34a5708c9ae
Reviewed-on: https://boringssl-review.googlesource.com/29464
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Update-Note: SSL_CTX_set_min_proto_version(SSL3_VERSION) now fails.
SSL_OP_NO_SSLv3 is now zero. Internal SSL3-specific "AEAD"s are gone.
Change-Id: I34edb160be40a5eea3e2e0fdea562c6e2adda229
Reviewed-on: https://boringssl-review.googlesource.com/29444
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
The full library is a bit much, but this is enough to appease most of
cryptography.io.
Change-Id: I1bb0d83744c4550d5fe23c5c98cfd7e36b17fcc9
Reviewed-on: https://boringssl-review.googlesource.com/29365
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Right now we're inconsistent about it. If the OPTIONAL container is
missing, we report an error, but if the container is empty, we happily
return nothing. The latter behavior is more convenient for emulating
OpenSSL's PKCS#7 functions.
These are our own functions, so we have some leeway here. Looking
through callers, they appear to handle this fine.
Update-Note: This is a behavior change.
Change-Id: I1321025a64df3054d380003c90e57d9eb95e610f
Reviewed-on: https://boringssl-review.googlesource.com/29364
Reviewed-by: Adam Langley <agl@google.com>
CBS_asn1_ber_to_der was a little cumbersome to use. While it, in theory,
allowed callers to consistently advance past the element, no caller
actually did so consistently. Instead they would advance if conversion
happened, and not if it was already DER. For the PKCS7_* functions, this
was even caller-exposed.
Change-Id: I658d265df899bace9ba6616cb465f19c9e6c3534
Reviewed-on: https://boringssl-review.googlesource.com/29304
Reviewed-by: Adam Langley <agl@google.com>
This change adds an AES-GCM AEAD that enforces nonce uniqueness inside
the FIPS module, like we have for TLS 1.2. While TLS 1.3 has not yet
been mentioned in the FIPS 140 IG, we expect it to be in the next ~12
months and so are preparing for that.
Change-Id: I65a7d8196b08dc0033bdde5c844a73059da13d9e
Reviewed-on: https://boringssl-review.googlesource.com/29224
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
cryptography.io gets offended if the library supports some OFB sizes but
not others.
Change-Id: I7fc7b12e7820547a82aae84d9418457389a482fe
Reviewed-on: https://boringssl-review.googlesource.com/29204
Reviewed-by: Adam Langley <agl@google.com>
The DSA code is deprecated and will, hopefully, be removed in the future.
Nonetheless, this is easy enough to fix. It's the analog of the work we'd
already done for ECDSA.
- Document more clearly that we don't care about the DSA code.
- Use the existing constant-time modular addition function rather than
the ad-hoc code.
- Reduce the digest to satisfy modular operations' invariants. (The
underlying algorithms could accept looser bounds, but we reduce for
simplicity.) There's no particular reason to do this in constant time,
but we have the code for it, so we may as well.
- This additionally adds a missing check that num_bits(q) is a multiple
of 8. We otherwise don't compute the right answer. Verification
already rejected all 160-, 224-, and 256-bit keys, and we only
generate DSA parameters where the length of q matches some hash
function's length, so this is unlikely to cause anyone trouble.
- Use Montgomery reduction to perform the modular multiplication. This
could be optimized to save a couple Montgomery reductions as in ECDSA,
but DSA is deprecated, so I haven't bothered optimizing this.
- The reduction from g^k (mod p) to r = g^k (mod p) (mod q) is left
in variable time, but reversing it would require a discrete log
anyway. (The corresponding ECDSA operation is much easier to make
constant-time due to Hasse's theorem, though that's actually still a
TODO. I need to finish lifting EC_FELEM up the stack.)
Thanks to Keegan Ryan from NCC Group for reporting the modular addition issue
(CVE-2018-0495). The remainder is stuff I noticed along the way.
Update-Note: See the num_bits(q) change.
Change-Id: I4f032b041e2aeb09f9737a39f178c24e6a7fa1cb
Reviewed-on: https://boringssl-review.googlesource.com/29145
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The last libssl struct is now opaque! (Promote the SSL_MAX_* constants
as folks use them pretty frequently.)
Update-Note: SSL_SESSION is now opaque. I believe everything handles
this now.
Bug: 6
Change-Id: I8cd29d16173e4370f3341c0e6f0a56e00ea188e9
Reviewed-on: https://boringssl-review.googlesource.com/28964
Reviewed-by: Adam Langley <agl@google.com>
Update-Note: This tweaks the SSL_shutdown behavior. OpenSSL's original
SSL_shutdown behavior was an incoherent mix of discarding the record and
rejecting it (it would return SSL_ERROR_SYSCALL but retrying the
operation would discard it). SSLeay appears to have intended to discard
it, so we previously "fixed" it actually discard.
However, this behavior is somewhat bizarre and means we skip over
unbounded data, which we typically try to avoid. If you are trying to
cleanly shutdown the TLS portion of your protocol, surely it is at a
point where additional data is a syntax error. I suspect I originally
did not realize that, because the discarded record did not properly
continue the loop, SSL_shutdown would appear as if it rejected the data,
and so it's unlikely anyone was relying on that behavior.
Discussion in https://github.com/openssl/openssl/pull/6340 suggests
(some of) upstream also prefers rejecting.
Change-Id: Icde419049306ed17eb06ce1a7e1ff587901166f3
Reviewed-on: https://boringssl-review.googlesource.com/28864
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This change adds server-side support for compressed certificates.
(Although some definitions for client-side support are included in the
headers, there's no code behind them yet.)
Change-Id: I0f98abf0b782b7337ddd014c58e19e6b8cc5a3c2
Reviewed-on: https://boringssl-review.googlesource.com/27964
Reviewed-by: David Benjamin <davidben@google.com>
This matches the OpenSSL 1.1.0 spelling. I'd thought we could hide
SSL_SESSION this pass, but I missed one test that messed with session
IDs!
Bug: 6
Change-Id: I84ea113353eb0eaa2b06b68dec71cb9061c047ca
Reviewed-on: https://boringssl-review.googlesource.com/28866
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
In neither OpenSSL nor BoringSSL can this function actually fail, but
OpenSSL makes it return one anyway. Match them for compatibility.
Change-Id: I497437321ad9ccc5da738f06cd5b19c467167575
Reviewed-on: https://boringssl-review.googlesource.com/28784
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
It would be nice to restrict these, limiting the incorrect sizes to a
separate EVP_AEAD, but start by documenting this.
Bug: 34
Change-Id: I09845882f76a53a010355ceefd168d4fc10a0681
Reviewed-on: https://boringssl-review.googlesource.com/28745
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
cryptography.io wants things exposed out of EVP_get_cipherby* including,
sadly, ECB mode.
Change-Id: I9bac46f8ffad1a79d190cee3b0c0686bf540298e
Reviewed-on: https://boringssl-review.googlesource.com/28464
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
cryptography.io wants RSA_R_BLOCK_TYPE_IS_NOT_02, only used by the
ancient RSA_padding_check_SSLv23 function. Define it but never emit it.
Additionally, it's rather finicky about RSA_R_TOO_LARGE* errors. We
merged them in BoringSSL because having RSA_R_TOO_LARGE,
RSA_R_TOO_LARGE_FOR_MODULUS, and RSA_R_TOO_LARGE_FOR_KEY_SIZE is a
little silly. But since we don't expect well-behaved code to condition
on error codes anyway, perhaps that wasn't worth it. Split them back
up.
Looking through OpenSSL, there is a vague semantic difference:
RSA_R_DIGEST_TOO_BIG_FOR_RSA_KEY - Specifically emitted if a digest is
too big for PKCS#1 signing with this key.
RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE - You asked me to sign or encrypt a
digest/plaintext, but it's too big for this key.
RSA_R_DATA_TOO_LARGE_FOR_MODULUS - You gave me an RSA ciphertext or
signature and it is not fully reduced modulo N.
-OR-
The padding functions produced something that isn't reduced, but I
believe this is unreachable outside of RSA_NO_PADDING.
RSA_R_DATA_TOO_LARGE - Some low-level padding function was told to copy
a digest/plaintext into some buffer, but the buffer was too small. I
think this is basically unreachable.
-OR-
You asked me to verify a PSS signature, but I didn't need to bother
because the digest/salt parameters you picked were too big.
Update-Note: This depends on cl/196566462.
Change-Id: I2e539e075eff8bfcd52ccde365e975ebcee72567
Reviewed-on: https://boringssl-review.googlesource.com/28547
Reviewed-by: Adam Langley <agl@google.com>
PyOpenSSL's tests expect all of the outputs to be distinct. OpenSSL also
tends to prefix the return values with strings like "compiler:", so do
something similar.
Change-Id: Ic411c95a276b477641ebad803ac309b3035c1b13
Reviewed-on: https://boringssl-review.googlesource.com/28544
Reviewed-by: Adam Langley <agl@google.com>
This is so Chromium can verify the session before offering it, rather
than doing it after the handshake (at which point it's too late to punt
the session) as we do today. This should, in turn, allow us to finally
verify certificates off a callback and order it correctly relative to
CertificateRequest in TLS 1.3.
(It will also order "correctly" in TLS 1.2, but this is useless. TLS 1.2
does not bind the CertificateRequest to the certificate at the point the
client needs to act on it.)
Bug: chromium:347402
Change-Id: I0daac2868c97b820aead6c3a7e4dc30d8ba44dc4
Reviewed-on: https://boringssl-review.googlesource.com/28405
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Previously, we'd omitted OpenSSL's OCSP APIs because they depend on a
complex OCSP mechanism and encourage the the unreliable server behavior
that hampers using OCSP stapling to fix revocation today. (OCSP
responses should not be fetched on-demand on a callback. They should be
managed like other server credentials and refreshed eagerly, so
temporary CA outage does not translate to loss of OCSP.)
But most of the APIs are byte-oriented anyway, so they're easy to
support. Intentionally omit the one that takes a bunch of OCSP_RESPIDs.
The callback is benign on the client (an artifact of OpenSSL reading
OCSP and verifying certificates in the wrong order). On the server, it
encourages unreliability, but pyOpenSSL/cryptography.io depends on this.
Dcument that this is only for compatibility with legacy software.
Also tweak a few things for compatilibility. cryptography.io expects
SSL_CTX_set_read_ahead to return something, SSL_get_server_tmp_key's
signature was wrong, and cryptography.io tries to redefine
SSL_get_server_tmp_key if SSL_CTRL_GET_SERVER_TMP_KEY is missing.
Change-Id: I2f99711783456bfb7324e9ad972510be8a95e845
Reviewed-on: https://boringssl-review.googlesource.com/28404
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
These were added in OpenSSL 1.1.0.
Change-Id: I261e0e0ccf82544883c4a2ef5c5dc4a651c0c756
Reviewed-on: https://boringssl-review.googlesource.com/28329
Reviewed-by: Adam Langley <agl@google.com>
PyOpenSSL calls this function these days. Tested by roundtripping with
ourselves and also manually confirming our output interoperates with
OpenSSL. (For anyone repeating this experiment, the OpenSSL
command-line tool has a bug and does not correctly output friendlyName
attributes with non-ASCII characters. I'll send them a PR to fix this
shortly.)
Between this and the UTF-8 logic earlier, the theme of this patch series
seems to be "implement in C something I last implemented in
JavaScript"...
Change-Id: I258d563498d82998c6bffc6789efeaba36fe3a5e
Reviewed-on: https://boringssl-review.googlesource.com/28328
Reviewed-by: Adam Langley <agl@google.com>
This is not very useful without PKCS12_create, which a follow-up change
will implement.
Change-Id: I355ccd22a165830911ae189871ab90a6101f42ae
Reviewed-on: https://boringssl-review.googlesource.com/28327
Reviewed-by: Adam Langley <agl@google.com>
This aligns with OpenSSL 1.1.0's behavior, which deviated from OpenSSL
1.0.2. OpenSSL 1.0.2 effectively assumed input passwords were always
Latin-1.
Update-Note: If anyone was using PKCS#12 passwords with non-ASCII
characters, this changes them from being encoding-confused to hopefully
interpretting "correctly". If this breaks anything, we can add a
fallback to PKCS12_get_key_and_certs/PKCS12_parse, but OpenSSL 1.1.0
does not have such behavior. It only implements a fallback in the
command-line tool, not the APIs.
Change-Id: I0aa92db26077b07a40f85b89f4d3e0f6b0d7be87
Reviewed-on: https://boringssl-review.googlesource.com/28326
Reviewed-by: Adam Langley <agl@google.com>
Update-Note: This changes causes BoringSSL to be stricter about handling
Unicode strings:
· Reject code points outside of Unicode
· Reject surrogate values
· Don't allow invalid UTF-8 to pass through when the source claims to
be UTF-8 already.
· Drop byte-order marks.
Previously, for example, a UniversalString could contain a large-valued
code point that would cause the UTF-8 encoder to emit invalid UTF-8.
Change-Id: I94d9db7796b70491b04494be84249907ff8fb46c
Reviewed-on: https://boringssl-review.googlesource.com/28325
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Callers should not mutate these.
Update-Note: I believe I've fixed up everything. If I missed one, the
fix should be straightforward.
Change-Id: Ifbce4961204822f57502a0de33aaa5a2a08b026d
Reviewed-on: https://boringssl-review.googlesource.com/28266
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Update-Note: Enabling TLS 1.3 now enables both draft-23 and draft-28
by default, in preparation for cycling all to draft-28.
Change-Id: I9405f39081f2e5f7049aaae8a9c85399f21df047
Reviewed-on: https://boringssl-review.googlesource.com/28304
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Hopefully this is the last of it before we can hide the struct. We're
missing peer_sha256 accessors, and some test wants to mutate the ticket
in a test client.
Change-Id: I1a30fcc0a1e866d42acbc07a776014c9257f7c86
Reviewed-on: https://boringssl-review.googlesource.com/28268
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Some third-party projects include it for some inexplicable reason.
Change-Id: I57c406d77d82a4a9ba6b54519023f2b02f2eb5e2
Reviewed-on: https://boringssl-review.googlesource.com/28225
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
