Commit Graph

498 Commits

Author SHA1 Message Date
Adam Langley
3e6526575a aarch64 support.
This is an initial cut at aarch64 support. I have only qemu to test it
however—hopefully hardware will be coming soon.

This also affects 32-bit ARM in that aarch64 chips can run 32-bit code
and we would like to be able to take advantage of the crypto operations
even in 32-bit mode. AES and GHASH should Just Work in this case: the
-armx.pl files can be built for either 32- or 64-bit mode based on the
flavour argument given to the Perl script.

SHA-1 and SHA-256 don't work like this however because they've never
support for multiple implementations, thus BoringSSL built for 32-bit
won't use the SHA instructions on an aarch64 chip.

No dedicated ChaCha20 or Poly1305 support yet.

Change-Id: Ib275bc4894a365c8ec7c42f4e91af6dba3bd686c
Reviewed-on: https://boringssl-review.googlesource.com/2801
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 23:38:11 +00:00
David Benjamin
bc44c089fb Store SRTP_PROTECTION_PROFILES as const.
They're small, but they should be read-only. This slightly changes public API
and affects downstream WebRTC code.

Hold on landing this until https://webrtc-codereview.appspot.com/34649004/
rolls into Chromium.

Change-Id: I93cbae20f69d55411d6b1cb62ed7d9a81c83b701
Reviewed-on: https://boringssl-review.googlesource.com/2720
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 22:10:08 +00:00
David Benjamin
7ce1c0ca75 Make SSL_load_error_strings a no-op.
SSL_library_init already loads the error strings (unlike upstream). Code which
calls both will end up loading error strings twice. Instead make the second
call a no-op.

Change-Id: Ifd34ab20ed46aabeba14661e58f8dac2bbb29f69
Reviewed-on: https://boringssl-review.googlesource.com/2790
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 22:09:58 +00:00
David Benjamin
e9fc3e547e Remove P-521 from the default supported curves list.
Per review comment on https://boringssl-review.googlesource.com/#/c/2843/.

Change-Id: I84c9320ff908c9f8912e83c6ece89d9b06c32bbf
Reviewed-on: https://boringssl-review.googlesource.com/2860
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:53:23 +00:00
David Benjamin
aa3f6daa86 Tag a number of globals as const.
Change-Id: I6f334911f153395a2e5e26adfd08912a1d8c558b
Reviewed-on: https://boringssl-review.googlesource.com/2847
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:53:00 +00:00
David Benjamin
70bd80a236 Remove constraints on curve ID values.
The under 32 constraint is silly; it's to check for duplicate curves in
library-supplied configuration. That API is new as of 1.0.2. It doesn't seem
worth bothering; if the caller supplies a repeated value, may as well emit a
repeated one and so be it. (Probably no one will ever call that function
outside of maybe test code anyway.)

While I'm here, remove the 0 constraint too. It's not likely to change, but
removing the return value overload seems easier than keeping comments about it
comments about it.

Change-Id: I01d36dba1855873875bb5a0ec84b040199e0e9bc
Reviewed-on: https://boringssl-review.googlesource.com/2844
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:52:28 +00:00
David Benjamin
52e5bacf7c Prune away unimplemented curve IDs.
We only implement four curves (P-224, P-256, P-384, and P-521) and only
advertise the latter three by default. Don't maintain entries corresponding to
all the unimplemented curves.

Change-Id: I1816a10c6f849ca1d9d896bc6f4b64cd6b329481
Reviewed-on: https://boringssl-review.googlesource.com/2843
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:51:44 +00:00
David Benjamin
0cb3f5bc27 Switch OBJ_undef uses to NID_undef.
They both happen to be zero, but OBJ_undef is a type error; OBJ_foo expands to
a comma-separated list of integers.

Change-Id: Ia5907dd3bc83240b7cc98af6456115d2efb48687
Reviewed-on: https://boringssl-review.googlesource.com/2842
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:51:25 +00:00
David Benjamin
6095de8da2 Add tests for certificate mismatch.
Cover another mildly interesting error case.

Change-Id: Ice773af79f5e03f39f0cd2a9e158bae03e065392
Reviewed-on: https://boringssl-review.googlesource.com/2841
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:51:17 +00:00
David Benjamin
d1d7d3d26a Clear existing extension state.
When parsing ClientHello clear any existing extension state from
SRP login and SRTP profile.

(Imported from upstream's 4f605ccb779e32a770093d687e0554e0bbb137d3)

More state that should be systematically reset across handshakes. Add a reset
on the ServerHello end too since that was missed.

Change-Id: Ibb4549acddfd87caf7b6ff853e2adbfa4b7e7856
Reviewed-on: https://boringssl-review.googlesource.com/2838
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:50:40 +00:00
David Benjamin
e3b2eebd04 The dtls1_output_cert_chain function no longer exists so remove it from ssl_locl.h
(Imported from upstream's 789da2c73d875af59b14156b6295aa4bdfc4f424)

Change-Id: Id94877d8d22578e23c63d1f133820a89ceae29ae
Reviewed-on: https://boringssl-review.googlesource.com/2834
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:49:16 +00:00
David Benjamin
710d227daa Fix memory leak in SSL_new if errors occur.
(Imported from upstream's 76e6509085ea96df0ca542568ee2596343711307)

Change-Id: I6319271a1f46b3d36a4eba950cbab60420126175
Reviewed-on: https://boringssl-review.googlesource.com/2833
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:49:04 +00:00
David Benjamin
2adb7ec286 ssl_create_cipher_list: check whether push onto cipherstack succeeds
(Imported from upstream's f5905ba341ad0fa3731469f10f7fba6f92ecd787.)

Change-Id: I92f2f53a127a4f59ce71cf00a9a4aedd0560e586
Reviewed-on: https://boringssl-review.googlesource.com/2832
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:48:55 +00:00
David Benjamin
af19de3101 Fix the test async_bio in datagram mode.
write_quota should only be decremented by 1 in datagram mode, otherwise we'll
underflow and always allow writes through. This does not cause any existing
tests to fail.

(It will be useful once the bug in dtls1_do_write is fixed.)

Change-Id: I42aa001d7264790a3726269890635f679497fb1c
Reviewed-on: https://boringssl-review.googlesource.com/2831
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:48:40 +00:00
David Benjamin
17a5f85cbb Clarify dtls1_do_write's interaction with the buffering BIO.
The existing comments are not very helpful. This code is also quite buggy.
Document two of them as TODOs.

Change-Id: Idfaf93d9c3b8b1ee92f2fb0d292ef513b5f6d824
Reviewed-on: https://boringssl-review.googlesource.com/2830
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:48:31 +00:00
David Benjamin
80cee912de Account for the MTU BIO_ctrls returning negative or overly large numbers.
BIO_ctrls do not have terribly well-defined return values on error. (Though the
existing ones seem to all return 0, not -1, on nonexistant operation.)

Change-Id: I08497f023ce3257c253aa71517a98b2fe73c3f74
Reviewed-on: https://boringssl-review.googlesource.com/2829
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:46:50 +00:00
David Benjamin
a18b671c94 Simplify minimum and default MTUs.
g_probably_mtu and dtls1_guess_mtu is a bunch of logic for guessing the right
MTU, but it only ever returns the maximum (the input is always zero). Trim that
down to only what it actually does.

Change-Id: If3afe3f68ccb36cbf9c4525372564d16a4bbb73f
Reviewed-on: https://boringssl-review.googlesource.com/2828
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:45:57 +00:00
David Benjamin
5a3cc0381b Remove BIO_CTRL_DGRAM_MTU_EXCEEDED retry in dtls1_do_write.
The retry doesn't actually work when we're sending a non-initial fragment; the
s->init_off != 0 block will get re-run each iteration through and continually
prepend headers. It can also infinite loop if the BIO reports
BIO_CTRL_DGRAM_MTU_EXCEEDED but either fails to report an MTU or reports an MTU
that always rounds up to the minimum. See upstream's
d3d9eef31661633f5b003a9e115c1822f79d1870.

WebRTC doesn't participate in any of the MTU logic and inherits the default
MTU, so just remove it for now.

Change-Id: Ib2ed2ba016b7c229811741fb7369c015ba0b551f
Reviewed-on: https://boringssl-review.googlesource.com/2827
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:45:14 +00:00
David Benjamin
7f18b139cc Always SSL_OP_NO_QUERY_MTU before querying the BIO MTU.
That setting means that the MTU is provided externally via SSL_set_mtu.

(Imported from upstream's 001235778a6e9c645dc0507cad6092d99c9af8f5)

Change-Id: I4e5743a9dee734ddd0235f080aefe98a7365aaf6
Reviewed-on: https://boringssl-review.googlesource.com/2826
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:43:36 +00:00
David Benjamin
d9778fb418 Guard against small MTUs from the BIO.
Based in part on upstream's cf75017bfd60333ff65edf9840001cd2c49870a3. This
situation really shouldn't be able to happen, but between no static asserts
that the minimum MTU is always large enough and a bug in reseting the MTU later
(to be fixed be a follow-up import from upstream), check these and return a
useful error code.

Change-Id: Ie853e5d35a6a7bc9c0032e74ae71529d490f4fe2
Reviewed-on: https://boringssl-review.googlesource.com/2825
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:43:07 +00:00
David Benjamin
c67a3ae6ba Drop retransmits in DTLS tests.
BoringSSL currently retransmits non-deterministically on an internal timer
(rather than one supplied externally), so the tests currently fail flakily
depending on timing. Valgrind is a common source for this. We still assume an
in-order and reliable channel, but drop retransmits silently:

- Handshake messages may arrive with old sequence numbers.

- Retransmitted CCS records arrive from the previous epoch.

- We may receive a retransmitted Finished after we believe the handshake has
  completed. (Aside: even in a real implementation, only Finished is possible
  here. Even with out-of-order delivery, retransmitted or reordered messages
  earlier in the handshake come in under a different epoch.)

Note that because DTLS renego and a Finished retransmit are ambiguous at the
record layer[*], this precludes us writing tests for DTLS renego. But DTLS
renego should get removed anyway. As BoringSSL currently implements renego,
this ambiguity is also a source of complexity in the real implementation. (See
the SSL3_MT_FINISHED check in dtls1_read_bytes.)

[*] As a further fun aside, it's also complex if dispatching renego vs Finished
after handshake message reassembly. The spec doesn't directly say the sequence
number is reset across renegos, but it says "The first message each side
transmits in /each/ handshake always has message_seq = 0". This means that such
an implementation needs the handshake message reassembly logic be aware that a
Finished fragment with high sequence number is NOT an out-of-order fragment for
the next handshake.

Change-Id: I35d13560f82bcb5eeda62f4de1571d28c818cc36
Reviewed-on: https://boringssl-review.googlesource.com/2770
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:13:05 +00:00
David Benjamin
f3a8b12ac3 Remove SSL_SESSION::cipher_id.
As of our 82b7da271f, an SSL_SESSION created
externally always has a cipher set. Unknown ciphers are rejected early. Prior
to that, an SSL_SESSION would only have a valid cipher or valid cipher_id
depending on whether it came from an internal or external session cache.

See upstream's 6a8afe2201cd888e472e44225d3c9ca5fae1ca62 and
c566205319beeaa196e247400c7eb0c16388372b for more context.

Since we don't get ourselves into this strange situation and s->cipher is now
always valid for established SSL_SESSION objects (the existence of
unestablished SSL_SESSION objects during a handshake is awkward, but something
to deal with later), do away with s->cipher_id altogether. An application
should be able to handle failing to parse an SSL_SESSION instead of parsing it
successfuly but rejecting all resumptions.

Change-Id: I2f064a815e0db657b109c7c9269ac6c726d1ffed
Reviewed-on: https://boringssl-review.googlesource.com/2703
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:10:55 +00:00
David Benjamin
b8a56f112f Remove dead code from EVP_CIPHER codepaths.
Everything is an AEAD now.

Change-Id: Ib47638e128843fc8299c3dbf9bd60c01eb5afa16
Reviewed-on: https://boringssl-review.googlesource.com/2700
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:05:41 +00:00
David Benjamin
e95d20dcb8 Support EVP_AEAD in DTLS.
This CL removes the last of the EVP_CIPHER codepath in ssl/. The dead code is
intentionally not pruned for ease of review, except in DTLS-only code where
adding new logic to support both, only to remove half, would be cumbersome.

Fixes made:
- dtls1_retransmit_state is taught to retain aead_write_ctx rather than
  enc_write_ctx.
- d1_pkt.c reserves space for the variable-length nonce when echoed into the
  packet.
- dtls1_do_write sizes the MTU based on EVP_AEAD max overhead.
- tls1_change_cipher_state_cipher should not free AEAD write contexts in DTLS.
  This matches the (rather confused) ownership for the EVP_CIPHER contexts.
  I've added a TODO to resolve this craziness.

A follow-up CL will remove all the resultant dead code.

Change-Id: I644557f4db53bbfb182950823ab96d5e4c908866
Reviewed-on: https://boringssl-review.googlesource.com/2699
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 21:03:40 +00:00
David Benjamin
044abb0aaa Implement SSLv3 ciphers with stateful AEADs.
This introduces another knob into SSL_AEAD_CTX to omit the version from the ad
parameter. It also allows us to fold a few more SSL3_ENC_METHOD hooks together.

Change-Id: I6540d410d4722f734093554fb434dab6e5217d4f
Reviewed-on: https://boringssl-review.googlesource.com/2698
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 20:55:58 +00:00
David Benjamin
41ac979211 Add the PRF to SSL3_ENC_METHOD.
This lets us fold away the SSLv3-specific generate_master_secret. Once SSLv3
uses AEADs, others will fold away as well.

Change-Id: I27c1b75741823bc6db920d35f5dd5ce71b6fdbb3
Reviewed-on: https://boringssl-review.googlesource.com/2697
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 20:43:26 +00:00
David Benjamin
31b1d81354 Factor SSLv3 key derivation steps into an ssl3_PRF.
Fix up the generate_master_secret parameter while we're here.

Change-Id: I1c80796d1f481be0c3eefcf3222f2d9fc1de4a51
Reviewed-on: https://boringssl-review.googlesource.com/2696
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 20:43:08 +00:00
David Benjamin
1f5e115ea9 Tidy up tls1_PRF a little.
size_t all the parameters. Also explicitly label label as label. This is in
preparation for pulling the PRF out into SSL3_ENC_METHOD so more of the
SSL3_ENC_METHOD hooks may be shared between SSLv3 and TLS once SSLv3 uses
stateful AEADs.

Also port away from EVP_PKEY_HMAC and use HMAC_CTX directly. The abstraction
doesn't buy much and is different from all the other EVP_DigestSign* functions.
There are few enough users within BoringSSL and Google that we can probably
deprecate and eventually remove it altogether.

Change-Id: I5d4529438c8a2a992fc199388a0c9e73bd6d2e06
Reviewed-on: https://boringssl-review.googlesource.com/2695
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 20:42:33 +00:00
David Benjamin
ea72bd0b60 Implement all TLS ciphers with stateful AEADs.
The EVP_CIPHER codepath should no longer be used with TLS. It still exists for
DTLS and SSLv3. The AEAD construction in TLS does not allow for
variable-overhead AEADs, so stateful AEADs do not include the length in the ad
parameter. Rather the AEADs internally append the unpadded length once it is
known. EVP_aead_rc4_md5_tls is modified to account for this.

Tests are added (and RC4-MD5's regenerated) for each of the new AEADs. The
cipher tests are all moved into crypto/cipher/test because there's now a lot of
them and they clutter the directory listing.

In ssl/, the stateful AEAD logic is also modified to account for stateful AEADs
with a fixed IV component, and for AEADs which use a random nonce (for the
explicit-IV CBC mode ciphers).

The new implementation fixes a bug/quirk in stateless CBC mode ciphers where
the fixed IV portion of the keyblock was generated regardless. This is at the
end, so it's only relevant for EAP-TLS which generates a MSK from the end of
the key block.

Change-Id: I2d8b8aa11deb43bde2fd733f4f90b5d5b8cb1334
Reviewed-on: https://boringssl-review.googlesource.com/2692
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 20:30:26 +00:00
David Benjamin
af032d68b3 Allocate the temporary buffer in tls1_PRF internally.
It's not worth saving the extra mallocs. This is preparation for moving SSLv3
to stateful AEADs; it'll share code TLS's SSL3_ENC_METHOD, but
tls1_generate_key_block is different, so that'll be pulled out into its own
hook.

Change-Id: I3f2136600758465c66ce23736041bb47f74efa6d
Reviewed-on: https://boringssl-review.googlesource.com/2690
Reviewed-by: Adam Langley <agl@google.com>
2015-01-14 19:47:45 +00:00
Adam Langley
a307dfd29f Add (void) to some macros to satisfy compiler.
More modern versions of GCC (at least with aarch64) are warning about an
unused value in these locations. It's incorrect, but I guess that the
macro is confusing it.

Using a (void) tag is a little ugly but solves the problem.

Change-Id: If6ba5083ab6e501c81e7743ae1ed99a89565e57c
Reviewed-on: https://boringssl-review.googlesource.com/2810
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 23:46:03 +00:00
David Benjamin
13be1de469 Add a basic MTU test.
The minimum MTU (not consistently enforced) is just under 256, so it's
difficult to test everything, but this is a basic test. (E.g., without renego,
the only handshake message with encryption is Finished which fits in the MTU.)
It tests the server side because the Certificate message is large enough to
require fragmentation.

Change-Id: Ida11f1057cebae2b800ad13696f98bb3a7fbbc5e
Reviewed-on: https://boringssl-review.googlesource.com/2824
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 22:37:25 +00:00
David Benjamin
dc4b197f0f Remove cookie_len setting in dtls1_new.
This should have been removed with its dtls1_clear cousin in
8c88153465.

Change-Id: Ibf4ee67348f603285b26766568cbb92183b62cee
Reviewed-on: https://boringssl-review.googlesource.com/2823
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 22:36:12 +00:00
David Benjamin
62fd16283a Implement SSL_clear with ssl_new and ssl_free.
State on s3 gets freed in both ssl3_clear and ssl3_free. Considate to just
ssl3_free. This replaces the (SSL,ssl,ssl3)_clear calls in (SSL,ssl,ssl3)_new
with the state that was initialized. This results in a little code duplication
between SSL_new and SSL_clear because state is on the wrong object. I've just
left TODOs for now; some of it will need disentangling.

We're far from it, but going forward, separate state between s and s->s3 as:

- s contains configuration state, DTLS or TLS. It is initialized from SSL_CTX,
  configurable directly afterwards, and preserved across SSL_clear calls.
  (Including when it's implicitly set as part of a handshake callback.)

- Connection state hangs off s->s3 (TLS) and s->d1 (DTLS). It is reset across
  SSL_clear. This should happen naturally out of a ssl_free/ssl_new pair.

The goal is to avoid needing separate initialize and reset code for anything;
the point any particular state is reset is the point its owning context is
destroyed and recreated.

Change-Id: I5d779010778109f8c339c07433a0777feaf94d1f
Reviewed-on: https://boringssl-review.googlesource.com/2822
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 22:35:58 +00:00
David Benjamin
02ddbfdf46 Move Channel ID initialization out of ssl3_new.
Configuration data inherited from the ctx happens in SSL_new. (This also gets
in the way of using ssl3_free/ssl3_new to implement SSL_clear.)

Change-Id: I2773af91abf4e1edc0c1a324bc1e94088d7c2274
Reviewed-on: https://boringssl-review.googlesource.com/2821
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 22:30:04 +00:00
Adam Langley
44e2709cd6 Fix DTLS memory leak.
A memory leak can occur in dtls1_buffer_record if either of the calls to
ssl3_setup_buffers or pqueue_insert fail. The former will fail if there
is a malloc failure, whilst the latter will fail if attempting to add a
duplicate record to the queue. This should never happen because
duplicate records should be detected and dropped before any attempt to
add them to the queue. Unfortunately records that arrive that are for
the next epoch are not being recorded correctly, and therefore replays
are not being detected. Additionally, these "should not happen" failures
that can occur in dtls1_buffer_record are not being treated as fatal and
therefore an attacker could exploit this by sending repeated replay
records for the next epoch, eventually causing a DoS through memory
exhaustion.

Thanks to Chris Mueller for reporting this issue and providing initial
analysis and a patch. Further analysis and the final patch was performed
by Matt Caswell from the OpenSSL development team.

CVE-2015-0206

(Imported from upstream's 7c6a3cf2375f5881ef3f3a58ac0fbd0b4663abd1).

Change-Id: I765fe61c75bc295bcc4ab356b8a5ce88c8964764
Reviewed-on: https://boringssl-review.googlesource.com/2782
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-01-09 19:41:47 +00:00
David Benjamin
780d6dd0fe Treat handshake_failure in response to ClientHello special.
Add a dedicated error code to the queue for a handshake_failure alert in
response to ClientHello. This matches NSS's client behavior and gives a better
error on a (probable) failure to negotiate initial parameters.

BUG=https://crbug.com/446505

Change-Id: I34368712085a6cbf0031902daf2c00393783d96d
Reviewed-on: https://boringssl-review.googlesource.com/2751
Reviewed-by: Adam Langley <agl@google.com>
2015-01-06 18:31:49 +00:00
Nick Harper
4dd053e059 Cast ca_list to (void *) to silence msvc warning 4090
Change-Id: If1fad46f14286ba98b86754605731a7be31de901
Reviewed-on: https://boringssl-review.googlesource.com/2680
Reviewed-by: Adam Langley <agl@google.com>
2015-01-06 01:14:03 +00:00
Adam Langley
fcf25833bc Reformat the rest of ssl/.
Change-Id: I7dc264f7e29b3ba8be4c717583467edf71bf8dd9
2014-12-18 17:43:03 -08:00
Adam Langley
be2900a6a3 Reformat s3_{enc|lib}.c.
Change-Id: I4f2a241ef996952195b9bcdd9ee305e28b2aff5d
2014-12-18 12:09:22 -08:00
Adam Langley
1bea173fd4 Reformatting of s3_{cbc|clnt}.c
Change-Id: Ie873bdf0dd5a66e76e6ebf909b1f1fe29b6fa611
2014-12-17 19:06:57 -08:00
Adam Langley
6e73d62dcc Touch up ssl3_get_message.
The |skip_message| variable was overly complex and, since we have at
least 32-bit ints, we know that a 24-bit value doesn't overflow an int.

Change-Id: I5c16fa979e1716f39cc47882c033bcf5bce3284c
Reviewed-on: https://boringssl-review.googlesource.com/2610
Reviewed-by: Adam Langley <agl@google.com>
2014-12-17 00:16:23 +00:00
David Benjamin
a6d81018f8 Consistently use RAND_bytes and check for failure.
RAND_pseudo_bytes just calls RAND_bytes now and only returns 0 or 1. Switch all
callers within the library call the new one and use the simpler failure check.
This fixes a few error checks that no longer work (< 0) and some missing ones.

Change-Id: Id51c79deec80075949f73fa1fbd7b76aac5570c6
Reviewed-on: https://boringssl-review.googlesource.com/2621
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 19:15:59 +00:00
David Benjamin
263eac02f5 Remove X509 parameter from ssl_cert_type.
No current use of ssl_cert_type passes a NULL EVP_PKEY, so it can be simplified
a little.

Change-Id: I2052cc3b6069cd30e4685ba8a6d0014016a4d712
Reviewed-on: https://boringssl-review.googlesource.com/2620
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 19:10:44 +00:00
David Benjamin
9cf708807c Consistently order ECDHE_ECDSA over ECDHE_RSA.
Currently we don't express an opinion. Most sites aren't likely to have a
choice since it depends on what certificates they have available. But we may as
well order them.

Change-Id: I4fffa5e392f42e19823cb8faa2e9e15a6bb91086
Reviewed-on: https://boringssl-review.googlesource.com/2607
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 02:56:36 +00:00
Adam Langley
2481975857 Reformat d1_{srtp|srvr}.c and s3_both.c
Change-Id: I4dc1463b75b12e15673da32e4945f83aaea123e6
2014-12-15 18:42:07 -08:00
David Benjamin
4841ce49a0 Fix EVP_Cipher error-handling.
Turns out the EVP_CIPH_FLAG_CUSTOM_CIPHER ciphers (i.e. legacy EVP_CIPHER
AES-GCM) have a completely different return value setup than the normal ones
which are the standard one/zero. (Except that they never return zero; see
TODO.)

Fix checks in ssl/ and remove remnants of EVP_CIPH_FLAG_CUSTOM_CIPHER in ssl/
as we're using EVP_AEAD now.

See CHANGES entry added in upstream's 3da0ca796cae6625bd26418afe0a1dc47bf5a77f.

Change-Id: Ia4d0ff59b03c35fab3a08141c60b9534cb7172e2
Reviewed-on: https://boringssl-review.googlesource.com/2606
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 01:51:55 +00:00
David Benjamin
ef5885e410 Don't change s->version after have_version is set.
Those version checks are if renego tried to change the version, but at that
point we're out of the initial null cipher and should leave the version fixed.

(On the server end, the code in question was dead after the version negotiation
rewrite anyway.)

Change-Id: I3242ba11bc9981ccf7fdb867176d59846cc49dd9
Reviewed-on: https://boringssl-review.googlesource.com/2605
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 01:44:35 +00:00
David Benjamin
e4824e8af0 Add outgoing messages to the handshake hash at set_handshake_header.
This avoids needing a should_add_to_finished_hash boolean on do_write. The
logic in do_write was a little awkward because do_write would be called
multiple times if the write took several iterations. This also gets complex if
DTLS retransmits are involved. (At a glance, it's not obvious the
BIO_CTRL_DGRAM_MTU_EXCEEDED case actually works.)

Doing it as the handshake message is being prepared avoids this concern. It
also gives a natural point for the extended master secret logic which needs to
do work after the finished hash has been sampled.

As a bonus, we can remove s->d1->retransmitting which was only used to deal
with this issue.

Change-Id: Ifedf23ee4a6c5e08f960d296a6eb1f337a16dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2604
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 01:43:51 +00:00
David Benjamin
44f2d1a9bf Use EVP_MAX_MD_SIZE to size the Finished message.
That comment is wrong as of TLS 1.2.

Change-Id: I900d5efc09d7468f2601d85f867833e43d046f6a
Reviewed-on: https://boringssl-review.googlesource.com/2603
Reviewed-by: Adam Langley <agl@google.com>
2014-12-16 01:38:51 +00:00