Commit Graph

691 Commits

Author SHA1 Message Date
David Benjamin
ca6c82643a Add DTLS-SRTP tests.
Just the negotiation portion as everything else is external. This feature is
used in WebRTC.

Change-Id: Iccc3983ea99e7d054b59010182f9a56a8099e116
Reviewed-on: https://boringssl-review.googlesource.com/2310
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:16:53 +00:00
Håvard Molland
ce5be4bd5c Add zero copy read and write api for bio pairs.
Also add functionality for setting external buffers to give the
caller better control of the buffers. This is typical needed if OS
sockets can outlive the bio pair.

Change-Id: I500f0c522011ce76e9a9bce5d7b43c93d9d11457
2014-11-18 14:06:46 -08:00
David Benjamin
5e4f6e9247 Remove some remnants of SSLv2.
Change-Id: Id294821162c4c9ea6f2fce2a0be65bafcb616068
Reviewed-on: https://boringssl-review.googlesource.com/2311
Reviewed-by: Adam Langley <agl@google.com>
2014-11-17 20:27:13 +00:00
David Benjamin
3087f6e594 Fix garbage free on malloc failure in ec_wNAF_mul.
PR#3595

(Imported from upstream's e04d426bf98ebb22abf0f15b6f09d333a6e8b2ad.)

Change-Id: I01a9d9bef7e911b3fb1565f8a582f5d6cc7d5537
Reviewed-on: https://boringssl-review.googlesource.com/2290
Reviewed-by: Adam Langley <agl@google.com>
2014-11-14 18:47:41 +00:00
Alex Chernyakhovsky
4cd8c43e73 Remove support for processing fragmented alerts
Prior to this change, BoringSSL maintained a 2-byte buffer for alerts,
and would support reassembly of fragmented alerts.

NSS does not support fragmented alerts, nor would any reasonable
implementation produce them. Remove fragmented alert handling and
produce an error if a fragmented alert has ever been encountered.

Change-Id: I31530ac372e8a90b47cf89404630c1c207cfb048
Reviewed-on: https://boringssl-review.googlesource.com/2125
Reviewed-by: Adam Langley <agl@google.com>
2014-11-13 22:58:30 +00:00
David Benjamin
bdf5e72f50 Don't resume sessions if the negotiated version doesn't match.
All of NSS, upstream OpenSSL, SChannel, and Secure Transport require, on the
client, that the ServerHello version match the session's version on resumption.
OpenSSL's current behavior is incompatible with all of these. Fall back to a
full handshake on the server instead of mismatch.

Add a comment on the client for why we are, as of
30ddb434bf, not currently enforcing the same in
the client.

Change-Id: I60aec972d81368c4ec30e2fd515dabd69401d175
Reviewed-on: https://boringssl-review.googlesource.com/2244
Reviewed-by: Adam Langley <agl@google.com>
2014-11-13 22:05:12 +00:00
David Benjamin
2f3ba910a2 Fix ec_test build on Windows.
No need to include unistd.h. (Though it probably should include string.h for
memcmp and strcmp.)

Change-Id: Ib09d2da4f7079c9d87338df75ec3560f4f203764
Reviewed-on: https://boringssl-review.googlesource.com/2260
Reviewed-by: Adam Langley <agl@google.com>
2014-11-12 19:39:55 +00:00
David Benjamin
e18d821dfc runner: Refuse to resume sessions on mismatching versions.
Clients all consistently reject mismatches. If a different version was
negotiated, a server should ignore the resumption. This doesn't actually affect
current tests.  We really want to be making this change in BoringSSL (and then
upstream), but get the Go half into shape first.

Change-Id: Ieee7e141331d9e08573592e661889bd756dccfa9
Reviewed-on: https://boringssl-review.googlesource.com/2243
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 18:25:28 +00:00
David Benjamin
d0297db108 Remove TODO about DTLS cookie lengths.
The limit increased from 32 to 255 between DTLS 1.0 and DTLS 1.2.

Change-Id: I329a59f9ba2bccc70282e2b47679c57b67e5ed43
Reviewed-on: https://boringssl-review.googlesource.com/2242
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 18:24:23 +00:00
David Benjamin
ca6554b133 Add tests for client-initiated renegotiation.
These'll get removed once most of renego support is gone, but this is to prove
removing the warning alert from the previous commit still prevents legacy
renegotiations.

Change-Id: I7d9d95e1d4c5d23d3b6d170938a5499a65f2d5ea
Reviewed-on: https://boringssl-review.googlesource.com/2236
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 00:00:49 +00:00
David Benjamin
9114fae39e Add a test for RSA ServerKeyExchange.
Ensure that the client rejects it with UNEXPECTED_MESSAGE, not by attempting to
decode it.

Change-Id: Ifc5613cf1152e0f7dcbee73e05df1ef367dfbfd5
Reviewed-on: https://boringssl-review.googlesource.com/2232
Reviewed-by: Adam Langley <agl@google.com>
2014-11-11 00:00:39 +00:00
David Benjamin
688d8dfe48 Remove psk_identity_hint from SSL_SESSION.
There's not much point in retaining the identity hint in the SSL_SESSION. This
avoids the complexity around setting psk_identity hint on either the SSL or the
SSL_SESSION. Introduce a peer_psk_identity_hint for the client to store the one
received from the server.

This changes the semantics of SSL_get_psk_identity_hint; it now only returns
the value configured for the server. The client learns the hint through the
callback. This is compatible with the one use of this API in conscrypt (it
pulls the hint back out to pass to a callback).

Change-Id: I6d9131636b47f13ac5800b4451436a057021054a
Reviewed-on: https://boringssl-review.googlesource.com/2213
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:59:47 +00:00
David Benjamin
e1b20a0136 Remove SSL3_FLAGS_POP_BUFFER.
This is an experimental flag that dates back to SSLeay 0.8.1b or earlier. It's
never set internally and never set in consumers.

Change-Id: I922583635c9f3d8d93f08f1707531ad22a26ae6a
Reviewed-on: https://boringssl-review.googlesource.com/2214
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:59:13 +00:00
David Benjamin
5e961c1ff1 Add DTLS replay tests.
At the record layer, DTLS maintains a window of seen sequence numbers to detect
replays. Add tests to cover that case. Test both repeated sequence numbers
within the window and sequence numbers past the window's left edge. Also test
receiving sequence numbers far past the window's right edge.

Change-Id: If6a7a24869db37fdd8fb3c4b3521b730e31f8f86
Reviewed-on: https://boringssl-review.googlesource.com/2221
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:58:56 +00:00
David Benjamin
0d82482e47 Fix ownership of error data in ERR_peek_*.
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>
2014-11-10 23:09:59 +00:00
David Benjamin
f2f3cfedb7 Use BN_bn2bin_padded rather than doing math to figure out leading zeros.
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>
2014-11-10 23:04:52 +00:00
David Benjamin
b145c8140b Compare r and s sizes to the order, not the degree.
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>
2014-11-10 23:02:41 +00:00
David Benjamin
1f10d9c8e1 Remove redundant PSK length check.
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>
2014-11-10 23:02:11 +00:00
David Benjamin
1df112448b Fix memory leak in ssl3_send_client_key_exchange error handling.
Change-Id: I0f0d7a3d4cb6448582ae4945e732611bb9bf5d9f
Reviewed-on: https://boringssl-review.googlesource.com/2231
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:01:07 +00:00
David Benjamin
525a0fe315 Remove client-side support for ServerKeyExchange in the RSA key exchange.
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>
2014-11-10 23:00:09 +00:00
David Benjamin
b4188f0c9d Don't be lenient if the client attempts unsafe renego.
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>
2014-11-10 22:46:17 +00:00
David Benjamin
6867f4854e Remove #if 0'd code documenting an old bug.
It was a bug anyway.

Change-Id: I59d680ce3615a4b24e72a9b6fa16939d83cc15ac
Reviewed-on: https://boringssl-review.googlesource.com/2234
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 22:45:17 +00:00
David Benjamin
d8138e91d0 Keep retransmit window size architecture-independent.
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>
2014-11-10 22:44:56 +00:00
David Benjamin
60e7992764 Remove DTLSv1_listen.
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>
2014-11-10 22:39:24 +00:00
David Benjamin
b044020f84 Remove i2d_X509_PKEY and d2i_X509_PKEY.
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>
2014-11-10 22:28:23 +00:00
Adam Langley
9ed9dae18e Update constant-time operations.
(Based on upstream's 42af669ff2754dfbe1dd55a0ab56664f82284dc4)

Change-Id: I4d3954fea7471e274c626483a0dfb9d7b3250b74
2014-11-10 13:48:30 -08:00
Adam Langley
a0b74eb241 aesni-x86_64.pl: make ECB subroutine Windows ABI compliant (RT 3552).
(Imported from upstream's 3e3cc471c2682f5cb40523f3997f75974de1755e.)

Change-Id: I8b6468b104091a609140a035739c09a031de90b4
2014-11-10 13:45:33 -08:00
Adam Langley
af6e45bde5 Simplify constant-time RSA padding check.
(Imported form upstream's 455b65dfab0de51c9f67b3c909311770f2b3f801 and
0d6a11a91f4de238ce533c40bd9507fe5d95f288)

Change-Id: Ia195c7fe753cfa3a7f8c91d2d7b2cd40a547be43
2014-11-10 13:45:33 -08:00
Adam Langley
a952d96656 Add a few more constant-time utility functions.
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
2014-11-10 13:45:32 -08:00
Adam Langley
7530e3031d perlasm/x86_64-xlate.pl: handle inter-bank movd.
(Imported from upstream's f7835e1c20836f286f00d6bcc69f154493e01475)

Change-Id: I9ebf2b90a008ff88bfce0a55566f0d0977391c1e
2014-11-10 13:45:32 -08:00
Adam Langley
fba236fa88 Omit version for v1 certificates.
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
2014-11-10 13:45:32 -08:00
Adam Langley
920e69658e Extra return in check_issued.
(Imported from upstream's b8d687bb561384bb3d52027cbf637fa4852c4225)

Change-Id: I5155b8fa165fbb83a0ba6790571fec28e22cd45c
2014-11-10 13:45:32 -08:00
Adam Langley
0e7f89f96c Remove pkey_ctrl.
It only included ASN1_PKEY_CTRL_DEFAULT_MD_NID and that's unused in
BoringSSL.

Change-Id: Idfcbd0f26f6448ce307c53ddef334f2e63c85a64
2014-11-10 13:45:32 -08:00
Adam Langley
08d6fe2fdb Possibly-uninit variable in pem_lib.c.
Can't really happen, but the flow of control isn't obvious. Add an
initializer.

(Imported from upstream's fa2ae04c40510262d198131c758acd8aa5a9b4ce)

Change-Id: If393687bca9f505b825feffaf2a63895a0ea5b6a
2014-11-10 13:45:32 -08:00
Adam Langley
e1cf3f1579 md5-x86_64.pl: work around warning.
(Imported from upstream's 30b7d5e1d83a2f5cb862361c180f7ba1f31a20a5.)

Change-Id: Icbcdb03bcaf8eddda1a7dfecc4f182ce6d93caeb
2014-11-10 13:45:32 -08:00
Adam Langley
3dfbcc1f25 x86[_64] assembly pack: add Silvermont performance data.
(Imported from upstream's 9dd6240201fdd9a9a0ce2aa66df04c174d08cf99)

Change-Id: Ie0f6f876e06ac28c717ec949565f6b0126166b30
2014-11-10 13:45:32 -08:00
Adam Langley
b15d8132c7 Constant-time utilities.
Pull constant-time methods out to a separate header, add tests.

(Imported from upstream's 9a9b0c0401cae443f115ff19921d347b20aa396b and
27739e92659d38cdefa21e51b7f52b81a7ac3388)

Change-Id: Id570f5c531aca791112929e6258989f43c8a78d7
2014-11-10 13:45:32 -08:00
Adam Langley
655038e7db ASN1_STRING_to_UTF8 missing initializer.
(Imported from upstream's b85d461cdf92b1d76d903c245ec01fd7398179d8)

Change-Id: I85f620a5a924c3fe73aa394b39ab8ab343c4c7eb
2014-11-10 13:45:32 -08:00
Adam Langley
9398f168f9 Explicitly check for empty ASN.1 strings in d2i_ECPrivateKey.
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
2014-11-10 13:45:32 -08:00
Adam Langley
9f5a314d35 Automatically generate a missing EC public key.
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
2014-11-10 13:45:31 -08:00
David Benjamin
1a8b549098 Fix memory leak in calling SSL_clear.
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>
2014-11-06 02:03:05 +00:00
David Benjamin
9da9035b50 Add digest_test with tests for all existing EVP_MDs.
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>
2014-11-06 01:49:03 +00:00
David Benjamin
23586e1e0e Test insertion of duplicates in pqueue_test.
Also add a few other assertions.

Change-Id: Iae0c65802f4d05c7585e2790be5295f478e1f614
Reviewed-on: https://boringssl-review.googlesource.com/2210
Reviewed-by: Adam Langley <agl@google.com>
2014-11-06 01:46:57 +00:00
David Benjamin
a85093f5bb Fix error handling in X509_PURPOSE_add and X509_TRUST_add.
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>
2014-11-06 01:44:43 +00:00
David Benjamin
ab2815eaff Fix leak in X509V3_EXT_add on error.
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>
2014-11-06 01:40:58 +00:00
David Benjamin
4aa86f1cdf Don't leak in err_set_error_data if error queue is empty.
Caught by clang scan-build.

Change-Id: I16496bfc469e809348199057adfb24c33c426ccb
Reviewed-on: https://boringssl-review.googlesource.com/2207
Reviewed-by: Adam Langley <agl@google.com>
2014-11-06 01:35:26 +00:00
David Benjamin
e2793a7189 Remove redundant s->s3 check.
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>
2014-11-06 01:34:52 +00:00
David Benjamin
9ecafa5c78 Shush some dead assignments.
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>
2014-11-06 01:34:33 +00:00
David Benjamin
f2b32a2de2 Switch ssl3_send_channel_id to BN_bn2bin_padded.
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>
2014-11-06 01:32:27 +00:00
David Benjamin
626fb8b75c Fix leaks in and de-macro X509_PKEY_new.
Caught by clang scan-build.

Change-Id: I075ae0423ec72493f21608ab5704e2984ca3e86c
Reviewed-on: https://boringssl-review.googlesource.com/2202
Reviewed-by: Adam Langley <agl@google.com>
2014-11-06 01:30:59 +00:00