Commit Graph

231 Commits

Author SHA1 Message Date
David Benjamin
7bbeead507 A bunch of dead assignments.
Caught by clang scan-build.

Change-Id: I4f10c879dc137d4a14a7a395764d28e5caa033ff
Reviewed-on: https://boringssl-review.googlesource.com/1342
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:44:03 +00:00
David Benjamin
072c953f40 Convert some malloc + memcpys into BUF_memdup.
Slightly tidier.

Change-Id: Ib3cb4dc262c88087bd56b446a6f7a05d1e57ade6
Reviewed-on: https://boringssl-review.googlesource.com/1345
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:42:40 +00:00
David Benjamin
9a37359008 Don't malloc(0) on empty extensions list.
Caught by clang scan-build.

Change-Id: I29092d659f1ac21c6a74b925f6abc0283e7652fc
Reviewed-on: https://boringssl-review.googlesource.com/1344
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:35:38 +00:00
David Benjamin
61b66ffcc2 Fix error-handling bugs.
Caught by clang scan-build.

Change-Id: I133d0338fe38172d687c02099d909366a59ee95b
Reviewed-on: https://boringssl-review.googlesource.com/1343
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:34:55 +00:00
David Benjamin
59b3a728dc Fix some DTLS alerts.
Caught by clang scan-build.

Change-Id: Ib17368557d3e8b2edac91e0441e64c21a5eb1318
Reviewed-on: https://boringssl-review.googlesource.com/1341
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:34:23 +00:00
Adam Langley
482b4f189f Fix DTLS certificate requesting code.
Use same logic when determining when to expect a client certificate for
both TLS and DTLS.

PR#3452

(Imported from upstream's 666a597ffb9bcf3ba2d49e711fcca28df91eff9d)

Change-Id: Ia267255a32c0b3b9a7da1c53f13ef6f620ff5ec1
2014-07-28 17:05:14 -07:00
Adam Langley
887b1c3d49 Don't limit message sizes in ssl3_get_cert_verify.
PR#319 (reoponed version).

(Imported from upstream's 8358302d47c8b6e0680d91ffee11302331f17a69)

Change-Id: I222c454620ab445d4906ee2c2b6a9ad80e56018e
2014-07-28 17:05:13 -07:00
Adam Langley
e3142a788c Make disabling last cipher work.
(Imported from upstream's 8f243ab6c173096943ca079648d240eba1e46dd3)

Change-Id: I929e481842c489aa28f28cf066b4556160a23efe
2014-07-28 17:05:13 -07:00
Adam Langley
3e14885558 Fix memory leak.
PR#2531

(Imported from upstream's 15de0f609c859883347357aaf6cc685c797358bd)

Change-Id: I4ee3a5ce22ee047a8eaa2c74f26971109a23680c
2014-07-28 17:05:13 -07:00
Adam Langley
46cfb0e4ee Remove redundant check.
PR#3174

(Imported from upstream's ec77f276e14f4b835cdd42fa175d74dcda532663)

Change-Id: I7e33d67c201bdd088520d060cc8e4e9713223e69
2014-07-28 17:05:13 -07:00
David Benjamin
7e3305eebd Fix False Start without session tickets.
One of the state transitions wasn't rewritten to CR_CHANGE. Add a test to
exercise this codepath. Also SSL_cutthrough_complete references the state.

Change-Id: Ib2f7ac5ac3f0348864efa93cf13cfd87454572f0
Reviewed-on: https://boringssl-review.googlesource.com/1337
Reviewed-by: Adam Langley <agl@google.com>
2014-07-28 20:31:56 +00:00
Piotr Sikora
108300573f Hook ssl_stat.c into build.
Change-Id: I049b2ed1b83e38375ddb658abf588a8e3355acb8
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
Reviewed-on: https://boringssl-review.googlesource.com/1190
Reviewed-by: Adam Langley <agl@google.com>
2014-07-28 18:50:46 +00:00
David Benjamin
0eb5a2df4a Fix tls1_get_curvelist default curves list size.
The length is the number of elements now, not the size in bytes. Caught by
ASan.

Change-Id: I4c5ccee61711e8d2e272b9bacd292dbff04b5133
Reviewed-on: https://boringssl-review.googlesource.com/1336
Reviewed-by: Adam Langley <agl@google.com>
2014-07-26 02:56:37 +00:00
David Benjamin
0aa0767340 Improve constant-time padding check in RSA key exchange.
Although the PKCS#1 padding check is internally constant-time, it is not
constant time at the crypto/ ssl/ API boundary. Expose a constant-time
RSA_message_index_PKCS1_type_2 function and integrate it into the
timing-sensitive portion of the RSA key exchange logic.

Change-Id: I6fa64ddc9d65564d05529d9b2985da7650d058c3
Reviewed-on: https://boringssl-review.googlesource.com/1301
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 20:25:15 +00:00
David Benjamin
86271ee9f8 Change CCS_OK to EXPECT_CCS.
Now that the flag is set accurately, use it to enforce that the handshake and
CCS synchronization. If EXPECT_CCS is set, enforce that:

(a) No handshake records may be received before ChangeCipherSpec.

(b) There is no pending handshake data at the point EXPECT_CCS is set.

Change-Id: I04b228fe6a7a771cf6600b7d38aa762b2d553f08
Reviewed-on: https://boringssl-review.googlesource.com/1299
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 17:58:58 +00:00
David Benjamin
cb5abad717 Consolidate CCS_OK paths in s3_clnt.c.
Introduce a CR_CHANGE state just before entering CR_FINISHED_A. This replaces
the CCS_OK in the CR_FINISHED_A/CR_FINISHED_B case which otherwise would get
applied after partial reads of Finished. The other CCS_OK settings are
redundant with this one.

The copy in tls_secret_session_cb codepath is made unnecessary with
9eaeef81fa.

The copy in the normal session resumption case is unnecessary with
6444287806. Before that commit, OpenSSL would
potentially read Finished a state early. Now that we are strict (and get the
book-keeping correct) for expecting the NewSessionTicket message it too is
redundant.

Of particular note is the one after ssl3_send_finished. That was added in
response to upstream's PR#3400. I've reproduced the bug and concluded it was
actually a bug around expecting a NewSessionTicket message. That has been fixed
properly in 6444287806 by resetting
tlsext_expect_ticket on renegotiations.

Change-Id: I6a928386994fcd5efff26a5f0efb12b65bf7f299
Reviewed-on: https://boringssl-review.googlesource.com/1298
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 17:49:12 +00:00
David Benjamin
84ec49e914 Consolidate CCS_OK codepaths in s3_srvr.c.
Rename SSL3_ST_SR_POST_CLIENT_CERT to SSL3_ST_SR_CHANGE and have this be the
point at which CCS_OK is set. The copy before ssl3_get_finished is redundant as
we never transition to SR_FINISHED directly.

Change-Id: I3eefeb821e7ae53d52dacc587fdc59de9ea9a667
Reviewed-on: https://boringssl-review.googlesource.com/1297
Reviewed-by: Adam Langley <agl@google.com>
2014-07-25 17:44:40 +00:00
David Benjamin
77a942b7fe Don't use the RSA key exchange with a signing-only key.
This removes the last case where the server generates an RSA key for the
ServerKeyExchange. Remove the code for this. Client support to accept them
still remains.

Leave the APIs for now, but they don't do anything anymore.

Change-Id: I84439e034cc575719f5bc9b3e501165e12b62107
Reviewed-on: https://boringssl-review.googlesource.com/1286
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:35:40 +00:00
David Benjamin
cd9969434c Pass parameters to tls1_process_sigalgs as a CBS.
Slightly cleaner; it means we can use CBS_stow.

Change-Id: I074aa2d73a79648013dea025ee531beeea2af4a2
Reviewed-on: https://boringssl-review.googlesource.com/1287
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:18:38 +00:00
David Benjamin
060d9d2c56 Remove support code for export cipher suites.
Now the only case where temporary RSA keys are used on the server end is
non-signing keys.

Change-Id: I55f6c206e798dd28548c386fdffd555ccc395477
Reviewed-on: https://boringssl-review.googlesource.com/1285
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:14:08 +00:00
David Benjamin
5ffeb7c22f Remove two more quirks.
SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG and
SSL_OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG. Neither of them have code that's even
enabled.

Change-Id: I866aabe1aa37e8ee145aaeaecaff6704c3ad21bc
Reviewed-on: https://boringssl-review.googlesource.com/1284
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:12:04 +00:00
David Benjamin
b9cc33a4d6 Remove SSL_OP_EPHEMERAL_RSA quirk.
Also fix a place where fixes for the condition for sending ServerKeyExchange in
s3_srvr.c were never propogated to d1_srvr.c. Tidy up that logic to use
ssl_cipher_requires_server_key_exchange and simplify the PSK check.

Change-Id: Ie36d378f733e59a8df405bc869f2346af59bd574
Reviewed-on: https://boringssl-review.googlesource.com/1283
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:11:50 +00:00
David Benjamin
ff175b4a46 unifdef OPENSSL_NO_PSK.
Get those out of the way.

Change-Id: I4cc8c34cf637379ad734c43623f76ae72f22014e
Reviewed-on: https://boringssl-review.googlesource.com/1282
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:11:05 +00:00
David Benjamin
019c3cc64a Remove last remnants of GOST support.
This removes support code for a "stream_mac" mode only used by GOST. Also get
rid of this

   /* I should fix this up TLS TLS TLS TLS TLS XXXXXXXX */

comment next to it. It's not actually related to GOST (dates to OpenSSL initial
commit), but isn't especially helpful at this point.

Change-Id: Ib13c6e27e16e0d1fb59ed0142ddf913b9abc20b7
Reviewed-on: https://boringssl-review.googlesource.com/1281
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:10:53 +00:00
David Benjamin
0e498f14d2 Remove some remnants of SSLv2 support.
Some ssl23 functions that can be folded into ssl3, declarations and macros that
don't exist anymore.

Change-Id: I8057fb0bab8b6fe7e4da7b90a4945f7f22e29cd9
Reviewed-on: https://boringssl-review.googlesource.com/1280
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:10:41 +00:00
David Benjamin
39482a13ac Remove get_cipher_by_char and put_cipher_by_char.
Without SSLv2, all cipher suite values are 2 bytes. Represent them as a
uint16_t and make all functions pass those around rather than pointers.

This removes SSL_CIPHER_find as it's unused.

Change-Id: Iea0b75abee4352a8333a4b8e39a161430ae55ea6
Reviewed-on: https://boringssl-review.googlesource.com/1259
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:10:29 +00:00
David Benjamin
0eb17906ab Remove support for sending V2ClientHello.
Accepting them as a server is still necessary, but this code is unreachable.
Without SSLv2 support, none of the cipher suites are SSLv2, so
ssl23_no_ssl2_ciphers always returns true and we send a V3ClientHello.

Change-Id: I09030f2c6e375660453c74e4f094d95e9908c3e1
Reviewed-on: https://boringssl-review.googlesource.com/1258
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:10:18 +00:00
David Benjamin
d23f412a8a Improve test coverage around NewSessionTicket message.
Test both when the peer doesn't support session tickets and when the server
promises a NewSessionTicket message but doesn't deliver.

Change-Id: I48f338094002beac2e6b80e41851c72822b3b9d5
Reviewed-on: https://boringssl-review.googlesource.com/1300
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:09:55 +00:00
David Benjamin
072334d943 Consistently use uint16_t for curve IDs.
Don't retain curve IDs in serialized form; serialization only happens when
writing and reading from the wire. The internal representation is a uint16_t
which matches the range of the value and avoids all the checks for the first
byte being 0.

This also fixes a bug in tls1_check_ec_tmp_key's suite B logic; the || should
have been &&, though now it's gone.

This doesn't relieve some of the other assumptions about curve IDs:
tls1_set_curves still assumes that all curve IDs are under 32, and
tls1_ec_curve_id2nid still assumes 0 is not a valid curve ID. Add a
compile-time assert and a comment to document this. We're up to 28 now, so this
may well need to be revised sooner or later.

Remove SSL_get_shared_curve as it's new and unused API, using it in a loop is
O(N^3), and lets us simplify a function.

Change-Id: I82778cb82648d82f7b5de8c5341e0e1febdf5611
Reviewed-on: https://boringssl-review.googlesource.com/1256
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:08:34 +00:00
David Benjamin
f3ec83dee0 Add EarlyChangeCipherSpec tests.
Adapted from patch in https://www.imperialviolet.org/2014/06/05/earlyccs.html.

Change-Id: I14bf314d105780e23e6bd09217870deff5744979
Reviewed-on: https://boringssl-review.googlesource.com/1292
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:05:13 +00:00
David Benjamin
1d5c83e063 Add test coverage for session resumption with tickets.
The shim is now passed two file descriptors. In a session resumption test, the
second is used in an abbreviated handshake immediately after the first.

Change-Id: I1f348c05f1a8ff2881fb46fc9e869696f74071c6
Reviewed-on: https://boringssl-review.googlesource.com/1291
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:04:52 +00:00
David Benjamin
6444287806 Do not allow a Finished message when NewSessionTicket is expected.
Per spec, the server sends it iff it sends the extension in ServerHello. There
is no need to probe for whether Finished is or isn't sent. NSS is strict about
this (wait_new_session_ticket never transitions to wait_change_cipher without a
NewSessionTicket message), so this is safe.

Reset tlsext_ticket_expected in ssl_scan_serverhello_tlsext to ensure state
from the initial handshake doesn't confuse renegotiation.  This is another one
of those per-handshake states that should be systematically reset on each
handshake. For now, reset it properly at least.

Change-Id: I7d16439ce632b9abf42f62d5d8e1303cb6f0be1f
Reviewed-on: https://boringssl-review.googlesource.com/1296
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:53:29 +00:00
David Benjamin
407886f589 Remove ssl3_check_finished.
ssl3_get_new_session_ticket is sensible and fills in a session_id for stateless
sessions, so the resumption will already be detected at this point. Remove the
codepath in ssl3_client_hello which allows for resuming sessions with empty
session_ids. The rest of the code doesn't allow it either.

This removes another codepath where we potentially probe a Finished message
early.

Change-Id: I2749b5c65c7ce98c6f30566d8716360ff1bba24c
Reviewed-on: https://boringssl-review.googlesource.com/1295
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:52:38 +00:00
David Benjamin
9eaeef81fa Set s->hit when using tls_session_secret_cb.
tls_session_secret_cb is used for EAP-FAST which computes the master secret
externally and enters the abbreviated handshake. It appears to only have been
working because ssl3_check_finished would drive it into the appropriate state
afterwards. That, in turn, only has been working because EAP-FAST misuses the
session ticket extension for some other field, so ssl3_check_finished isn't a
no-op.

Instead, set s->hit so it follows the abbreviated state machine directly.

If we ever build wpa_supplicant with BoringSSL, this will require some testing.
(And, if not, this API should be removed.)

Change-Id: Ie2992a9bba049f7120522b996f739906e38a070e
Reviewed-on: https://boringssl-review.googlesource.com/1294
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:50:29 +00:00
David Benjamin
6553b379e2 Don't accidentally read Finished in ssl3_get_cert_verify.
This removes one place where we set CCS_OK. ssl3_get_cert_verify already knows
whether or not to expect a CertificateVerify message, so there is no need to
look ahead and potentially read ChangeCipherSpec early.

Change-Id: I80f4ec218b073c1007b01dbe1e3bd529fb848d37
Reviewed-on: https://boringssl-review.googlesource.com/1293
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:45:55 +00:00
David Benjamin
b61b4de6df Fix runner.go session ticket support.
Some test code to insert a bogus session ticket was retained. Also,
decryptTicket mutated its input, in turn, mutating the ClientHello,
breaking the Finished hash.

The latter fix should probably be merged upstream.

Change-Id: I6949f842c67e19df8742561fb0b849af9f5f099d
Reviewed-on: https://boringssl-review.googlesource.com/1290
Reviewed-by: Adam Langley <agl@google.com>
2014-07-23 15:31:24 +00:00
David Benjamin
42be6456ca Add SkipChangeCipherSpec-Server-NPN test.
Finished isn't always the first post-CCS message.

Change-Id: I4f70eeed57cf732693d07212b096efb2594c5b3c
Reviewed-on: https://boringssl-review.googlesource.com/1288
Reviewed-by: Adam Langley <agl@google.com>
2014-07-21 19:24:32 +00:00
David Benjamin
de620d9c87 runner: Require the CertificateVerify signature and hash to match.
This change can probably be ported over to upstream crypto/tls. The current Go
TLS implementation ignores the signature and hash algorithm lists in
CertificateVerify and CertificateRequest. Take these into account so that our
tests assert OpenSSL fills them out correctly.

Also fix a bug in the original code where 'err' within the switch block get
shadowed.

Change-Id: I5d9c0b31ebb4662ecc767ed885a20707f0e86216
Reviewed-on: https://boringssl-review.googlesource.com/1253
Reviewed-by: Adam Langley <agl@google.com>
2014-07-21 15:59:48 +00:00
David Benjamin
a0e5223bb8 Add SkipChangeCipherSpec tests.
They pass, but this is an error case that is probably worth a test.

Change-Id: I37b2eec34a1781fa8342eea57ee4f9da81ce17ed
Reviewed-on: https://boringssl-review.googlesource.com/1257
Reviewed-by: Adam Langley <agl@google.com>
2014-07-21 02:14:13 +00:00
David Benjamin
ecc0ce7e67 Introduce EVP_PKEY_is_opaque to replace RSA_METHOD_FLAG_NO_CHECK.
Custom RSA and ECDSA keys may not expose the key material. Plumb and "opaque"
bit out of the *_METHOD up to EVP_PKEY. Query that in ssl_rsa.c to skip the
sanity checks for certificate and key matching.

Change-Id: I362a2d5116bfd1803560dfca1d69a91153e895fc
Reviewed-on: https://boringssl-review.googlesource.com/1255
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 23:35:04 +00:00
Adam Langley
37a623cf29 Have BIO_get_mem_data return a size_t and uint8_t*
Change-Id: I883f9c3527b572a2140ae4899cf4409cdc25c6dc
Reviewed-on: https://boringssl-review.googlesource.com/1261
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 17:22:05 +00:00
Adam Langley
6defe177fe Fix typo in DTLS-SRTP extension parsing.
Change-Id: I1fd4c22b20a6272cd65448f5f62b9790d7597aa7
Reviewed-on: https://boringssl-review.googlesource.com/1260
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 00:52:51 +00:00
Adam Langley
045cc5590a Don't delay CKX and Finished for False Start.
Android never did this - they patched out the point in the code that set
the SSL3_FLAGS_DELAY_CLIENT_FINISHED flag when doing False Start.
Also, from the unittests it appears that NSS doesn't do this either.
Thus this change brings BoringSSL into line with existing behaviour.

SSL3_FLAGS_DELAY_CLIENT_FINISHED wasn't introduced with False Start,
it's an option in vanilla OpenSSL. But I can't find anything that uses
it and, since it's going to be untested, I've removed it completely in
this change.

Change-Id: I910537bfa35e74ab88778b83612cf5607d485969
Reviewed-on: https://boringssl-review.googlesource.com/1221
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 18:59:38 +00:00
David Benjamin
2e52121acd Fix magic SSL reason codes.
SSL reason codes corresponding to alerts have special values. Teach
make_errors.go that values above 1000 are reserved (otherwise it will assign
new values in that namespace). Also fix all the existing reason codes which
corresponded to alerts.

Change-Id: Ieabdf8fd59f4802938616934e1d84e659227cf84
Reviewed-on: https://boringssl-review.googlesource.com/1212
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 18:54:06 +00:00
David Benjamin
8f3234b2c8 Fix another flipped condition.
This one in code that's not compiled though.

Change-Id: I8fb6c2df4669a70223889d31b233b577cf3e6b22
Reviewed-on: https://boringssl-review.googlesource.com/1211
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 17:11:47 +00:00
David Benjamin
407a10cf43 Fix parsing of CertificateRequests.
Got one of the conditions flipped.

Change-Id: I327a9c13e42865459e8d69a431b0d3a2bc6b54a5
Reviewed-on: https://boringssl-review.googlesource.com/1210
Reviewed-by: Adam Langley <agl@google.com>
2014-07-16 17:07:15 +00:00
David Benjamin
22f9bccde5 Port ssl3_get_client_hello to CBS.
Also fix some DTLS cookie bugs. rcvd_cookie is never referenced after being
saved (and the length isn't saved, so it couldn't be used anyway), and the
cookie verification failed to check the length.

For convenience, add a CBS_mem_equal helper function. Saves a bit of
repetition.

Change-Id: I187137733b069f0ac8d8b1bf151eeb80d388b971
Reviewed-on: https://boringssl-review.googlesource.com/1174
Reviewed-by: Adam Langley <agl@google.com>
2014-07-15 18:30:09 +00:00
David Benjamin
14c83e7d00 Refactor PSK logic in ssl3_get_client_key_exchange.
This avoids duplicating the code to build the final premaster in PSK and
ECDHE_PSK. It also ports it to CBB for an initial trial of the API. Computing
the premaster secret now proceeds in four steps:

1. If a PSK key exchange (alg_a), look up the pre-shared key.
2. Compute the premaster secret based on alg_k. If PSK, it's all zeros.
3. If a PSK key exchange (alg_a), wrap the premaster in a struct with the
   pre-shared key.
4. Use the possibly modified premaster to compute the master secret.

Change-Id: Ib511dd2724cbed42c82b82a676f641114cec5470
Reviewed-on: https://boringssl-review.googlesource.com/1173
Reviewed-by: Adam Langley <agl@google.com>
2014-07-15 00:02:14 +00:00
David Benjamin
820c74af65 Tidy up some PSK cipher checks.
The only PSK cipher suite that computes the shared secret early is PSK. Also
there were two (unreachable because of earlier checks) codepaths where we're
exit this function without a master secret.

Change-Id: I3b64fc007b83c4bc46ddb6e14382fb285d8095f9
Reviewed-on: https://boringssl-review.googlesource.com/1172
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:56:29 +00:00
David Benjamin
35c023014b Port ssl3_get_client_key_exchange to CBS.
Change-Id: I065554d058395322a4ac675155bfe66c874b47ad
Reviewed-on: https://boringssl-review.googlesource.com/1171
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:56:05 +00:00
David Benjamin
8f8040dd4f Rename ssl3_get_key_exchange to ssl3_get_server_key_exchange.
More consistent with ssl3_send_server_key_exchange and the message name.

Change-Id: If0f435a89bdf117297d349099708fff0bd5a6e98
Reviewed-on: https://boringssl-review.googlesource.com/1170
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:55:35 +00:00
David Benjamin
05da6e1641 Port tls12_check_peer_sigalg to CBS.
This avoids having to do the CBS_skip dance and is better about returning the
right alert.

Change-Id: Id84eba307d7c67269ccbc07a38d9044b6f4f7c6c
Reviewed-on: https://boringssl-review.googlesource.com/1169
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:55:01 +00:00
David Benjamin
a03d95d156 Port ssl3_get_server_hello to CBS.
Change-Id: I8ec531cf327653b4779ff6a29cac62e240502269
Reviewed-on: https://boringssl-review.googlesource.com/1167
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:52:52 +00:00
David Benjamin
46062681ab Port ssl3_get_cert_status to CBS.
Change-Id: I18b68f32fceb0f9273f2d86ec201ebf9350103df
Reviewed-on: https://boringssl-review.googlesource.com/1166
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:42:28 +00:00
David Benjamin
e044c3d8a2 Port ssl3_get_new_session_ticket to CBS.
Change-Id: Iabca923c9be48d001abd3b12b8c6898e604aa85a
Reviewed-on: https://boringssl-review.googlesource.com/1165
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:38:16 +00:00
David Benjamin
ed43958853 Port ssl3_get_key_exchange to CBS.
Also tidy up some variable names and update RSA_verify call for it no longer
returning -1. Add CBS helper functions for dealing with C strings.

Change-Id: Ibc398d27714744f5d99d4f94ae38210cbc89471a
Reviewed-on: https://boringssl-review.googlesource.com/1164
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:37:29 +00:00
David Benjamin
1f5f62b204 Add a server NPN test.
Change-Id: Ib34a24e86bb5de117ecf5609918e130c1ff9532e
Reviewed-on: https://boringssl-review.googlesource.com/1161
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 23:08:01 +00:00
Adam Langley
4c921e1bbc Move public headers to include/openssl/
Previously, public headers lived next to the respective code and there
were symlinks from include/openssl to them.

This doesn't work on Windows.

This change moves the headers to live in include/openssl. In cases where
some symlinks pointed to the same header, I've added a file that just
includes the intended target. These cases are all for backwards-compat.

Change-Id: I6e285b74caf621c644b5168a4877db226b07fd92
Reviewed-on: https://boringssl-review.googlesource.com/1180
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 22:42:18 +00:00
David Benjamin
6897dbece1 Port ssl3_get_cert_verify to CBS.
Also tidy up a little now that {RSA,ECDSA}_verify don't have two separate error
codes.

Change-Id: Id0e9248f63766771032a131fd96d86d2596ade32
Reviewed-on: https://boringssl-review.googlesource.com/1168
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:47:17 +00:00
David Benjamin
67666e7823 Add tests for the server accepting client certificates.
Change-Id: I9acc4363c6b9804d5fe464053393cf16ffb7785c
Reviewed-on: https://boringssl-review.googlesource.com/1159
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:45:57 +00:00
David Benjamin
51b1f7427b Make init_msg a uint8_t*.
It's current a void* and gets explicitly cast everywhere. Make it a uint8_t and
only add the casts when converting it come init_buf, which internally stores a
char*.

Change-Id: I28bed129e46ed37ee1ce378d5c3bd0738fc1177f
Reviewed-on: https://boringssl-review.googlesource.com/1163
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:43:20 +00:00
David Benjamin
9c651c9ef6 Be strict about requiring ServerKeyExchange.
Missing ServerKeyExchange is handled, but only because it hits an
ERR_R_INTERNAL_ERROR in ssl3_send_client_key_exchange in trying to find the
server ECDH parameters. Be strict about requiring it for ECDHE.

Change-Id: Ifce5b73c8bd14746b8a2185f479d550e9e3f84df
Reviewed-on: https://boringssl-review.googlesource.com/1157
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:42:30 +00:00
David Benjamin
ced551f967 Port ssl3_get_next_proto to CBS.
NPNServerTest in runner.go provides test coverage.

Change-Id: I5503ccbc4270e7f9f42ebc30c21e8077a430cf9f
Reviewed-on: https://boringssl-review.googlesource.com/1162
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:28:47 +00:00
David Benjamin
d5b1f84d6e Port ssl3_get_client_certificate to CBS.
Server client certificate tests provide test coverage.

Change-Id: I272b8099675f2a747f3ca878327c5f0b6936a988
Reviewed-on: https://boringssl-review.googlesource.com/1160
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 21:27:42 +00:00
David Benjamin
9a6232f34d Port ssl3_get_channel_id to CBS.
Change-Id: I3c36c62b6f073304e89dec5b3dfe3694b6339f6e
Reviewed-on: https://boringssl-review.googlesource.com/1158
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:51:28 +00:00
David Benjamin
e8f3d666cc Be strict about expecting a server Certificate message.
Introduce a ssl_cipher_has_server_public_key to save the repeated
NULL/PSK/RSA_PSK[*] check. Don't allow skipping to ServerKeyExchange when
expecting Certificate; the messages expected are determined by the cipher
suite. The ssl3_get_server_public_key call is already guarded.

As the previous test demonstrates, this is safe because of the
ssl3_check_cert_and_algorithm call, but avoid the looseness in the parsing
there.

[*] NB: we don't implement RSA_PSK, and OpenSSL has never implemented it.

Change-Id: I0571e6bcbeb8eb883f77878bdc98d1aa3a287cf3
Reviewed-on: https://boringssl-review.googlesource.com/1156
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:50:30 +00:00
David Benjamin
1c375dda8a Add UnauthenticatedECDH bug test.
This works, but there's enough shared codepaths that it's worth a test to
ensure it stays that way.

Change-Id: I5d5a729811e35832170322957258304213204e3b
Reviewed-on: https://boringssl-review.googlesource.com/1155
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:50:09 +00:00
David Benjamin
d26aea6c96 Remove remnants of KRB5 support.
This drops the bits of logic that allowed Certificate messages to be optional
for a KRB5 cipher suite.

Change-Id: I2a71b7c13d7e76f4f5542d4074169f80f3617240
Reviewed-on: https://boringssl-review.googlesource.com/1154
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:57 +00:00
David Benjamin
0c49ec97f4 Fix potential memory leak.
This can't happen because we don't implement RSA_PSK, but we probably should
check here.

Probably |sess_cert| shouldn't be attached to SSL_SESSION anyway; it's only
relevant when initializing the session and if it's accessed afterwards, it'll
be shared and cause problems.

Change-Id: Id868e523195f33c22e057f9b89dc02fe68e9b554
Reviewed-on: https://boringssl-review.googlesource.com/1153
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:39 +00:00
David Benjamin
5235f74bc2 Fix algorithm_auth and SSL_k* mismatch.
Fixes bug introduced in c26c802a89. Only one of
the two halves got flipped.

Change-Id: I0b3905ab22b0f83f093e1720af85594b1a970a7f
Reviewed-on: https://boringssl-review.googlesource.com/1152
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:26 +00:00
David Benjamin
7bf334a9ff Perform bounds checks in hmac_signctx.
Match the other EVP_DigestSignFinal implementations. Fix the instances in
ssl/t1_enc.c which were not following the EVP_DigestSignFinal contract; on
entry, *out_len should contain the size of the buffer.

Change-Id: Icd44d97a4c98704dea975798c0101d5a37274d17
Reviewed-on: https://boringssl-review.googlesource.com/1130
Reviewed-by: Adam Langley <agl@google.com>
2014-07-10 20:59:10 +00:00
David Benjamin
0e2908a806 Unify signing codepath of ssl3_send_client_verify.
Instead of, in the pre-TLS-1.2 case, reaching into the EVP and manually
signing, compute the digest separately from signing. Then use EVP_PKEY_sign.
This will make it easier to implement https://crbug.com/347404 by having only
one signing codepath as well as make that logic simpler.

Also add a bounds check while we're here, although the buffer is too large to
actually matter.

runner.go client auth tests should cover code changes.

Change-Id: I7d87181bbcc5a761660412452e508d24c4725327
Reviewed-on: https://boringssl-review.googlesource.com/1122
Reviewed-by: Adam Langley <agl@google.com>
2014-07-10 00:05:14 +00:00
David Benjamin
636293bf25 Add client auth tests.
Change-Id: If3ecae4c97f67085b9880ffa49dd616f1436ce97
Reviewed-on: https://boringssl-review.googlesource.com/1112
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 21:04:06 +00:00
David Benjamin
7b03051103 Add a test for certificate types parsing.
Change-Id: Icddd39ae183f981f78a65427a4dda34449ca389a
Reviewed-on: https://boringssl-review.googlesource.com/1111
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 21:03:54 +00:00
David Benjamin
676d1e780e Separate client and server certificate_types.
This is the first of reorganizing state between connection state and handshake
state. The existing set are retained in cert_st for the server; they are server
configuration. The client gets a copy in s->s3->tmp alongside other handshake
state.

With other handshake state moved there, hopefully we can reset that state in
one go and possibly not even maintain it when there is no handshake in
progress.  Rather than currently where we sometimes confused connection state
and handshake state and have to reset as appropriate on renegotiate.

While I'm here, document the fields and name them something more useful than
'ctypes'.

Change-Id: Ib927579f0004fc5c6854fce2127625df669b2b6d
Reviewed-on: https://boringssl-review.googlesource.com/1113
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 19:51:08 +00:00
David Benjamin
8f2c20eb70 Port early callback support to CBS.
Resolve one of the TODOs since it's quick. Adjust the
-expect-server-name test to assert it both in the normal codepath and
in the early callback, to provide test coverage for
SSL_early_callback_ctx_extension_get.

Change-Id: I4d71158b9fd2f4fbb54d3e51184bd25d117bdc91
Reviewed-on: https://boringssl-review.googlesource.com/1120
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 16:48:31 +00:00
David Benjamin
35a7a4492d Check duplicate extensions before processing.
ClientHello and ServerHello are not allowed to include duplicate extensions.
Add a new helper function to check this and call as appropriate. Remove ad-hoc
per-extension duplicate checks which are no unnecessary.

Add runner.go tests to verify such message correctly rejected.

Change-Id: I7babd5b642dfec941459512869e2dd6de26a831c
Reviewed-on: https://boringssl-review.googlesource.com/1100
Reviewed-by: Adam Langley <agl@google.com>
2014-07-08 22:17:59 +00:00
David Benjamin
398ba895fb Remove SSL_copy_session_id.
This is the only codepath that allowed a cert_st to be shared between two
ssl_st's. Given that the cert_st currently contains some per-connection and
even per-handshake state, this probably doesn't work.

Remove the function altogether and don't ref-count cert_st.

Change-Id: I66d5346117cb59b6063e7b9b893d1c4b40cb6867
Reviewed-on: https://boringssl-review.googlesource.com/1110
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 22:43:56 +00:00
David Benjamin
9d28c75774 Fix some OPENSSL_PUT_ERROR calls.
The function names are wrong.

Change-Id: Icbaeb541a2dcc504f69af81a7505e5cfbeed91f0
Reviewed-on: https://boringssl-review.googlesource.com/1101
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:32:38 +00:00
David Benjamin
b9621b9c1a Remove is_probably_safari logic.
We handle it externally now.

Change-Id: Ib561f64078809645195fd1a859b3256499038847
Reviewed-on: https://boringssl-review.googlesource.com/1098
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:32:29 +00:00
David Benjamin
5468b23797 Remove rest of DANE code.
We pulled in some of upstream's removal, but not the rest of it.

Change-Id: I8ea6681848f3c59955b7d2ce935c077d024245be
Reviewed-on: https://boringssl-review.googlesource.com/1099
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:32:04 +00:00
David Benjamin
64f4c91b89 Remove OPENSSL_FIPS blocks.
Done with unifdef with some manual edits to remove empty lines.

Change-Id: I40d163539cab8ef0e01e45b7dc6a1a0a37733c3e
Reviewed-on: https://boringssl-review.googlesource.com/1097
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:31:50 +00:00
David Benjamin
6dbd73db5d Remove OPENSSL_NO_TLSEXT compilation option.
Mostly done with unifdef.

Change-Id: I876f79f9e96d77628d696b09694363d07aee6b74
Reviewed-on: https://boringssl-review.googlesource.com/1096
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:31:37 +00:00
Alex Chernyakhovsky
31955f91dc Port Certificate Request parsing to crypto/bytestring
Along the way, clean up the certificate types code to not have the
hard-coded fixed-size array.

Change-Id: If3e5978f7c5099478a3dfa37a0a7059072f5454a
Reviewed-on: https://boringssl-review.googlesource.com/1103
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:27:04 +00:00
Alex Chernyakhovsky
9c890d4b70 Remove SSL_OP_NETSCAPE_CA_DN_BUG
SSL_OP_NETSCAPE_CA_DN_BUG is not included in SSL_OP_ALL.

Change-Id: I1635ad2721ed2742b1dff189d68bfc67a1c840a6
Reviewed-on: https://boringssl-review.googlesource.com/1102
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:21:59 +00:00
Alex Chernyakhovsky
3c5034e97c Remove OPENSSL_NO_RSA
Building without RSA support is unreasonable. Changes were made by
running

find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_RSA
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_RSA

using unifdef 2.10 and some newlines were removed manually.

Change-Id: Iea559e2d4b3d1053f28a4a9cc2f7a3d1f6cabd61
Reviewed-on: https://boringssl-review.googlesource.com/1095
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:20:53 +00:00
Alex Chernyakhovsky
c6318e349a Port Server Certificate parsing to crypto/bytestring
Change-Id: I79e5475be4d8d1fe6fcce6ae7ab5b43e0dc98efa
Reviewed-on: https://boringssl-review.googlesource.com/1094
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:20:19 +00:00
David Benjamin
dc72ff75bd Port ClientHello extensions parsing to crypto/bytestring.
Change-Id: I673c929b78bcf6952db8dfb295dd79d455bcb2a0
Reviewed-on: https://boringssl-review.googlesource.com/1070
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 19:48:22 +00:00
Alex Chernyakhovsky
2897ed3634 Remove CHARSET_EBCDIC
Since crypto/ebcdic.{c,h} are not present in BoringSSL, remove the #ifdefs
Changes were made by running

find . -type f -name *.c | xargs unifdef -m -U CHARSET_EBCDIC
find . -type f -name *.h | xargs unifdef -m -U CHARSET_EBCDIC

using unifdef 2.10.

An additional two ifdefs (CHARSET_EBCDIC_not) were removed manually.

Change-Id: Ie174bb00782cc44c63b0f9fab69619b3a9f66d42
Reviewed-on: https://boringssl-review.googlesource.com/1093
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 19:30:35 +00:00
David Benjamin
8af6639565 Fix leak in ssl_parse_clienthello_use_srtp_ext.
If parsing the MKI value fails, clnt is never freed.

Change-Id: Ic85edf0d6efc54ca0828f333bc389c0dbf58f491
Reviewed-on: https://boringssl-review.googlesource.com/1072
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 22:55:24 +00:00
David Benjamin
197b3abaa8 Add a test that server_name extensions are parsed correctly.
Change-Id: Id4025835df49eb498df9a48fc81061541778569b
Reviewed-on: https://boringssl-review.googlesource.com/1092
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 22:52:59 +00:00
David Benjamin
025b3d3459 Add some basic server tests to runner.go.
client_shim.cc and runner.go are generalized to handle both ends. Plumb a bit
through the test case to control which and add server versions of all the
cipher suite tests.

Change-Id: Iab2640b390f7ed7160c9a9bf6bb34b8bec761b2e
Reviewed-on: https://boringssl-review.googlesource.com/1091
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 22:52:02 +00:00
David Benjamin
4c852c5363 Add missing break statement in ssl3_get_client_hello.
dc9b141127 added a default case when importing
the patch but accidentally falls through all the time.

Change-Id: Ieb9beeb9e3ffcf77f2842841eda7d28a62fe8072
Reviewed-on: https://boringssl-review.googlesource.com/1073
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 19:46:50 +00:00
David Benjamin
325b5c3667 Fix FallbackSCSV test.
It wasn't actually testing SSL_enable_fallback_scsv, just that not calling it
didn't send an SCSV. Plumb the 'flag' parameter to testCase through and add a
test that asserts it does get sent when expected. (Make it a []string since Go
doesn't distinguish nil string from "" and for flexibility.)

Change-Id: I124c01e045aebbed5c1d87b7196de7c2026f26f3
Reviewed-on: https://boringssl-review.googlesource.com/1071
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 00:30:29 +00:00
David Benjamin
2b0aeecabf Remove authz extension (RFC5878)
Found no users of the functions which control the feature. (Also I don't
particularly want to port all of that to CBS...)

Change-Id: I55da42c44d57252bd47bdcb30431be5e6e90dc56
Reviewed-on: https://boringssl-review.googlesource.com/1061
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 20:48:38 +00:00
David Benjamin
0442ac5ce4 Update comment for TLSEXT_TYPE_renegotiate.
Change-Id: I2b831cf04e15116b849dae714d77602da4cbad91
Reviewed-on: https://boringssl-review.googlesource.com/1064
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 18:55:50 +00:00
David Benjamin
2e5d686e5a Make ssl_parse_serverhello_use_srtp_ext return 0 on failure.
Match most other functions. 03973096f4 changed
the call site, but not the function itself.

Change-Id: Iac018cbed8a454f50fb0da5e2419f503d8cb652d
Reviewed-on: https://boringssl-review.googlesource.com/1060
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 18:55:32 +00:00
David Benjamin
fd76191821 Remove opaque_prf_input extension.
This code doesn't even get built unless you go out of your way to pass an
extension value at build time.

Change-Id: I92ffcdfb18505c96e5ef390c8954a54cee19967f
Reviewed-on: https://boringssl-review.googlesource.com/1063
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 18:55:19 +00:00
David Benjamin
3c1345e9db Remove tlsext_custom_types field.
Remnant of custom extensions support.

Change-Id: I4b91a0962709dccaf7fdf6b118d47d6e0dbadb91
Reviewed-on: https://boringssl-review.googlesource.com/1062
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 18:54:59 +00:00
Adam Langley
9447dff5a2 Use RC4-MD5 AEAD in ssl/
This change adds the infrastructure to use stateful AEADs in ssl/ and
specifically wires in the stitched, RC4-MD5 AEAD. Over time, all
cipher suites will be supported via the AEAD interface and the old
EVP_CIPHER code will die off.

Change-Id: I44ed3ca2672e1342c6b632be08fee9272d113f8e
Reviewed-on: https://boringssl-review.googlesource.com/1044
Reviewed-by: Adam Langley <agl@google.com>
2014-06-30 23:39:17 +00:00