The error queue should only take ownership of the data if ERR_get_* is called,
not ERR_peek_*. Add a test for ERR_peek_error_line_data.
Change-Id: I976fc90fb54437dff723418ef3afd94f1c967922
Reviewed-on: https://boringssl-review.googlesource.com/2237
Reviewed-by: Adam Langley <agl@google.com>
Saves doing it ad-hoc all the time.
Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8
Reviewed-on: https://boringssl-review.googlesource.com/2241
Reviewed-by: Adam Langley <agl@google.com>
r and s are scalars, not EC coordinates.
Change-Id: I46a20215d3c602559c18c74a1da9a91543ea73ca
Reviewed-on: https://boringssl-review.googlesource.com/2240
Reviewed-by: Adam Langley <agl@google.com>
If psk_len were 0, it would already have been an error earlier. The PSK cipher
suites don't lose the other_secret || psk construction if the PSK happens to be
empty.
Change-Id: I1917236720d0862658562bc8f014cb827ee9aed5
Reviewed-on: https://boringssl-review.googlesource.com/2233
Reviewed-by: Adam Langley <agl@google.com>
Server-side support was removed in 77a942b7fe,
but client-side support was retained as it appeared NSS supported this.
However, this is not the case: ssl3_HandleServerKeyExchange only allows a
ServerKeyExchange message if hs.ws is in an appropriate state.
ssl3_AuthCertificate only sets it to allow ServerKeyExchange if it is a key
exchange that normally uses it or if is_limited is set. is_limited is only set
for the export cipher suites.
Thus we can safely remove this without waiting on gathering UMA data.
BUG=chromium:400587
Change-Id: I9aefb742dbb2d99c13340ab48017e1ceee04bc2f
Reviewed-on: https://boringssl-review.googlesource.com/2230
Reviewed-by: Adam Langley <agl@google.com>
This was added in upstream's 82e610e2cfbbb5fd29c09785b6909a91e606f347. The
commit message cites draft-ietf-tls-renegotiation which was on
draft-ietf-tls-renegotiation-01 at the time. The text in question (6.2 Server
Considerations) is no longer in RFC 5746. The RFC now recommends terminating
the connection which is much simpler.
It also was wrong anyway as it checked s->ctx->options instead of s->options
for SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION.
Removing that block will result in the connection being terminated in
ssl_scan_clienthello_tlsext.
Change-Id: Ie222c78babd3654c5023ad07ac0d8e0adde68698
Reviewed-on: https://boringssl-review.googlesource.com/2235
Reviewed-by: Adam Langley <agl@google.com>
Parameters like these should not change between 32-bit and 64-bit. 64 is also
the value recommended in RFC 6347, section 4.1.2.6. Document those fields while
I'm here.
Change-Id: I8481ee0765ff3d261a96a2e1a53b6ad6695b2d42
Reviewed-on: https://boringssl-review.googlesource.com/2222
Reviewed-by: Adam Langley <agl@google.com>
This was added in http://rt.openssl.org/Ticket/Display.html?id=2033 to support
a mode where a DTLS socket would statelessly perform the ClientHello /
HelloVerifyRequest portion of the handshake, to be handed off to a socket
specific to this peer address.
This is not used by WebRTC or other current consumers. If we need to support
something like this, it would be cleaner to do the listen portion (cookieless
ClientHello + HelloVerifyRequest) externally and then spin up an SSL instance
on receipt of a cookied ClientHello. This would require a slightly more complex
BIO to replay the second ClientHello but would avoid peppering the DTLS
handshake state with a special short-circuiting mode.
Change-Id: I7a413932edfb62f8b9368912a9a0621d4155f1aa
Reviewed-on: https://boringssl-review.googlesource.com/2220
Reviewed-by: Adam Langley <agl@google.com>
One of them was never implemented upstream or downstream. The other no longer
works in BoringSSL. They're not used within BoringSSL (this still compiles),
even in X509_INFO, and do not appear to be used by consumers. If they were, we
would like to know via a compile failure.
This removes the last consumer within BoringSSL of the ASN.1 parsing macros.
Change-Id: Ifb72b1fcd0a4f7b3e6b081486f8638110872334b
Reviewed-on: https://boringssl-review.googlesource.com/2203
Reviewed-by: Adam Langley <agl@google.com>
(Imported form upstream's 455b65dfab0de51c9f67b3c909311770f2b3f801 and
0d6a11a91f4de238ce533c40bd9507fe5d95f288)
Change-Id: Ia195c7fe753cfa3a7f8c91d2d7b2cd40a547be43
Imported from upstream's 9bed73adaa6f834177f29e478d9a2247a6577c04.
Upstream's commit appears to have been based on BoringSSL's commits to
improve the constant-time behaviour of RSA padding checks and thus I've
not tried to import those bits of the change.
Change-Id: I0ea5775b0f1e18741bbbc9f792a6af0d3d2a4caf
When calling X509_set_version to set v1 certificate, that should mean
that the version number field is omitted.
(Imported from upstream's 8c0d19d8577c9a96b65622bfa92d0affd6bbb4ac)
Change-Id: If433fda7b6ccbd899f3379a38581c351cf4a82da
Can't really happen, but the flow of control isn't obvious. Add an
initializer.
(Imported from upstream's fa2ae04c40510262d198131c758acd8aa5a9b4ce)
Change-Id: If393687bca9f505b825feffaf2a63895a0ea5b6a
Pull constant-time methods out to a separate header, add tests.
(Imported from upstream's 9a9b0c0401cae443f115ff19921d347b20aa396b and
27739e92659d38cdefa21e51b7f52b81a7ac3388)
Change-Id: Id570f5c531aca791112929e6258989f43c8a78d7
The old code implicitly relies on the ASN.1 code returning a \0-prefixed
buffer when the buffer length is 0. Change this to verify explicitly
that the ASN.1 string has positive length.
(Imported from upstream's 7f7c05ca638c3cc6d261961fae439cd91e3c1d27)
Change-Id: Icc6c44b874bdcb02374016a36d209830d6162a8a
When d2i_ECPrivateKey reads a private key with a missing (optional)
public key, generate one automatically from the group and private key.
(Imported from upstream's 2083f7c465d07867dd9867b8742bb71c03d1f203)
Change-Id: I9e5090de87cf846ab92e4be5b6bf64e6091d02e4
State hanging off the SSL gets freed in two places.
Change-Id: I41a8d2a7cab35f0098396006e1f6380038ec471a
Reviewed-on: https://boringssl-review.googlesource.com/2212
Reviewed-by: Adam Langley <agl@google.com>
Remove the existing md5_test and sha1_test. They now are all covered by
digest_test. For good measure, test the one-shot functions too.
Change-Id: I8e144cc563fb8817144e26cbd2e10c15642464ba
Reviewed-on: https://boringssl-review.googlesource.com/2211
Reviewed-by: Adam Langley <agl@google.com>
Also add a few other assertions.
Change-Id: Iae0c65802f4d05c7585e2790be5295f478e1f614
Reviewed-on: https://boringssl-review.googlesource.com/2210
Reviewed-by: Adam Langley <agl@google.com>
Two leaks can happen: if idx is -1, the newly allocated entry may not be freed.
Also, for X509_PURPOSE_add, if only one BUF_strdup succeeds, it will leak.
Restructure both so that the allocations happen ahead of time and are properly
cleaned up. This avoids leaving an existing entry in a half-broken state.
Found (sort of) by scan-build; because of all the indirections and DYNAMIC
flags, it doesn't actually realize the leak's been fixed.
Change-Id: I5521889bd14e007b3f62b6a4906d7c346698b48c
Reviewed-on: https://boringssl-review.googlesource.com/2209
Reviewed-by: Adam Langley <agl@google.com>
If it fails to be added to the list, the input should be freed. Found (sort of)
by scan-build; because of all the indirections and DYNAMIC flags, it doesn't
actually realize the leak's been fixed.
Change-Id: Idca10964e1ffb2ace1cea7f88d94693205d70d5b
Reviewed-on: https://boringssl-review.googlesource.com/2208
Reviewed-by: Adam Langley <agl@google.com>
s->s3 is never NULL if an ssl3_* function is called, and we'll crash later
anyway. (This also makes scan-build stop believing it can be NULL.)
Change-Id: Ibf8433bd4d945f9bf5416d72946102a9e50d2787
Reviewed-on: https://boringssl-review.googlesource.com/2206
Reviewed-by: Adam Langley <agl@google.com>
Appease clang scan-build a bit. I'm not sure it's actually worth silencing all
of them because some of them look like preserving invariants between local
variables, but some are clearly pointless or can be restructured slightly.
Change-Id: I0bc81e2589bb402ff3ef0182d7a8921e31b85052
Reviewed-on: https://boringssl-review.googlesource.com/2205
Reviewed-by: Adam Langley <agl@google.com>
Check the return value while we're here. This avoids some arithmetic and
appease scan-build's dead assignment flagger.
Change-Id: If3615076e091eb44b9e3e9d50cd64f80e645337e
Reviewed-on: https://boringssl-review.googlesource.com/2204
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's ef908777218bd4a362dbe9cebb8e18fa8ab384cf.)
Change-Id: Id9b288d230cc9d8ab308690a18e687e2132e3293
Reviewed-on: https://boringssl-review.googlesource.com/2168
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 267e6f3cc0ef78dea4e5cf93907a71556a45f008)
Change-Id: I99cfd0196b9625449c50494562c44f57f09fed17
Reviewed-on: https://boringssl-review.googlesource.com/2167
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This stopped being a sample program a bit ago.
Change-Id: I23301fd71a373f995847dcfd64346bd262811f39
Reviewed-on: https://boringssl-review.googlesource.com/2131
Reviewed-by: Adam Langley <agl@google.com>
This got lost in the initial commit. Add a test for d2i_AutoPrivateKey.
BUG=crbug.com/428671
Change-Id: Ib4f6114b03536edcfe3b1720a513f57f748e81d2
Reviewed-on: https://boringssl-review.googlesource.com/2130
Reviewed-by: Adam Langley <agl@google.com>
Without SSL_MODE_AUTO_RETRY, even blocking mode will return
SSL_ERROR_WANT_{READ|WRITE} in the event of a renegotiation.
The comments in the code speak only of "nasty problems" unless this is
done. The original commit that added SSL_MODE_AUTO_RETRY
(54f10e6adce56eb2e59936e32216162aadc5d050) gives a little more detail:
The [...] behaviour is needed by applications such as s_client and
s_server that use select() to determine when to use SSL_read.
Without the -nbio flag, s_client will use select() to find when the
socket is readable and then call SSL_read with a blocking socket.
However, this will still block in the event of an incomplete record, so
the delay is already unbounded. This it's very unclear what the point of
this behaviour ever was.
Perhaps if the read and write paths were different sockets where the
read socket was non-blocking but the write socket was blocking. But that
seems like an implausible situation to worry too much about.
Change-Id: I9d9f2526afc2e0fd0e5440e9a047f419a2d61afa
Reviewed-on: https://boringssl-review.googlesource.com/2140
Reviewed-by: Adam Langley <agl@google.com>
The only alias, EVP_PKEY_RSA2, is handled programmatically. ASN1_PKEY_ALIAS and
ASN1_PKEY_DYNAMIC are then unused and unexported and can be removed.
Change-Id: I990650636bac3b802c8b439257c67ce7a3f8bc70
Reviewed-on: https://boringssl-review.googlesource.com/2124
Reviewed-by: Adam Langley <agl@google.com>
It's never used, upstream or downstream. The 64-bit value is wrong anyway for
LLP64 platforms.
Change-Id: I56afc51f4c17ed3f1c30959b574034f181b5b0c7
Reviewed-on: https://boringssl-review.googlesource.com/2123
Reviewed-by: Adam Langley <agl@google.com>
Some archaeology: it was added in upstream's
ee1d9ec019a7584482bd95891404f1cad66a4a0a. This seems to come from upstream's
arrangement where an EVP_MD can specify both the signing algorithm and the
message digest. (Most of the usual hash algorithms were tied to RSA.)
The flag is set on EVP_MDs that should use the EVP_PKEY's method table in
EVP_Sign* rather than the one attached to the EVP_MD (there's also
required_pkey_type to filter on EVP_PKEY to prevent a mismatch). Without the
flag, the old codepath is hit where they're tied together.
Interestingly, EVP_md5 does not have the flag, but I suppose this is because no
one would sign ECDSA + MD5. EVP_DigestSign* also postdates this and doesn't use
the legacy mechanism anyway. Upstream also has, e.g., EVP_ecdsa(). Although
those too have since also gained the flag in
bce1af776247fee153223ea156228810779483ce.
Let's get rid of these TODOs. We don't have the old codepath. It's unclear if
upstream really does either at this point.
Note: EVP_PKEY_RSA_method in upstream is actually a macro that expands to three
fields, which is why it's so difficult to figure out what's going on with those
structs.
Change-Id: I1aea4d3f79f1eb1755063bb96c1c65276c6e3643
Reviewed-on: https://boringssl-review.googlesource.com/2122
Reviewed-by: Adam Langley <agl@google.com>
This code isn't compiled in. It seems there was some half-baked logic for a
7-byte alert that includes more information about handshake messages
retransmit.
No such alert exists, and the code had a FIXME anyway. If it gets resurrected
in DTLS 1.3 or some extension, we can deal with it then.
Change-Id: I8784ea8ee44bb8da4b0fe5d5d507997526557432
Reviewed-on: https://boringssl-review.googlesource.com/2121
Reviewed-by: Adam Langley <agl@google.com>
