Commit Graph

2345 Commits

Author SHA1 Message Date
David Benjamin
72f7e21087 Stop allowing SHA-224 in TLS 1.2.
Take the mappings for MD5 and SHA-224 values out of the code altogether. This
aligns with the current TLS 1.3 draft.

For MD5, this is a no-op. It is not currently possible to configure accepted
signature algorithms, MD5 wasn't in the hardcoded list, and we already had a
test ensuring we enforced our preferences correctly. MD5 also wasn't in the
default list of hashes our keys could sign and no one overrides it with a
different hash.

For SHA-224, this is not quite a no-op. The hardcoded accepted signature
algorithms list included SHA-224, so this will break servers relying on that.
However, Chrome's metrics have zero data points of servers picking SHA-224 and
no other major browser includes it. Thus that should be safe.

SHA-224 was also in the default list of hashes we are willing to sign. For
client certificates, Chromium's abstractions already did not allow signing
SHA-224, so this is a no-op there. For servers, this will break any clients
which only accept SHA-224. But no major browsers do this and I am not aware of
any client implementation which does such ridiculous thing.

(SHA-1's still in there. Getting rid of that one is going to take more effort.)

Change-Id: I6a765fdeea9e19348e409d58a0eac770b318e599
Reviewed-on: https://boringssl-review.googlesource.com/7020
Reviewed-by: Adam Langley <agl@google.com>
2016-01-29 21:30:00 +00:00
Brian Smith
5fa8f5bc9a Fix |-Werror=old-style-declaration| violations in poly1305_vec.c.
The |inline| must appear before the type.

Change-Id: Iecebbcc50024a846d7804228a858acfc33d68efd
Reviewed-on: https://boringssl-review.googlesource.com/7010
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-28 23:58:45 +00:00
David Benjamin
2cdf398773 Remove pkey_base_id.
This is never accessed.

Change-Id: I4cade5e907ad4c03e9de7634b53ef965f7240087
Reviewed-on: https://boringssl-review.googlesource.com/6864
Reviewed-by: Adam Langley <agl@google.com>
2016-01-28 15:55:24 +00:00
David Benjamin
415564fe2c Update draft-irtf-cfrg-curves-11 references to RFC 7748.
Change-Id: I6148df93a1748754ee6be9e2b98cc8afd38746cb
Reviewed-on: https://boringssl-review.googlesource.com/6960
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:53:26 +00:00
David Benjamin
4f6acaf0da Use more C++11 features.
Finally, we can stick ScopedFOO in containers.

Change-Id: I3ed166575822af9f182e8be8f4db723e1f08ea31
Reviewed-on: https://boringssl-review.googlesource.com/6553
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:52:37 +00:00
David Benjamin
c3774c1187 Fix some indentation.
Change-Id: I3507be754b489a99a04c0dea888cb1f3652e68c3
Reviewed-on: https://boringssl-review.googlesource.com/6854
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:51:45 +00:00
David Benjamin
0a2c9938a5 Don't allow the specifiedCurve form of ECParameters in SPKIs.
Although RFC 3279 allows both, per RFC 5912, keys must use a named curve
rather than spelling out the curve parameters. Although we do not allow
arbitrary curves, we do have to (pretty hackishly) recognize built-in
curves in ECPrivateKeys.

It seems the cause of this was that OpenSSL, unless you set asn1_flag on
the EC_GROUP, likes to encode keys by spelling out the parameters. This
is in violation of RFC 5915, though probably not in violation of one of
the other redundant ECC specifications. For more fun, it appears
asn1_flag defaults to *off* in the API and *on* in the command-line
tools.

I think the original cause was these defaults meant the pre-BoringSSL
Android/OpenSSL Chromium port wrote out Channel ID keys in this format.
By now this should no longer by an issue, but it'll warrant a bit more
investigation to be sure we can drop it.

For now, keep this logic out of SPKIs by not calling d2i_ECParameters.
d2i_ECParameters is a fairly pointless function when only named curves
are allowed. In testing other implementations, none of Firefox, Safari,
or IE11/Win will parse such certificates (i.e. the error is fatal and
unbypassable). Likewise, because Mac and Windows' underlying libraries
reject this, Chrome on Mac and Windows already rejects such things. Thus
this change should be compatible.

The following is the certificate and key I constructed to test with:

-----BEGIN CERTIFICATE-----
MIICwjCCAmqgAwIBAgIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC
QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp
dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ
BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l
dCBXaWRnaXRzIFB0eSBMdGQwggFLMIIBAwYHKoZIzj0CATCB9wIBATAsBgcqhkjO
PQEBAiEA/////wAAAAEAAAAAAAAAAAAAAAD///////////////8wWwQg/////wAA
AAEAAAAAAAAAAAAAAAD///////////////wEIFrGNdiqOpPns+u9VXaYhrxlHQaw
zFOw9jvOPD4n0mBLAxUAxJ02CIbnBJNqZnjhE50mt4GffpAEQQRrF9Hy4SxCR/i8
5uVjpEDydwN9gS3rM6D0oTlF2JjClk/jQuL+Gn+bjufrSnwPnhYrzjNXazFezsu2
QGg3v1H1AiEA/////wAAAAD//////////7zm+q2nF56E87nKwvxjJVECAQEDQgAE
5itp4r9ln5e+Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyD
SNsWGhz1HX7xlC1Lz3IiwaNQME4wHQYDVR0OBBYEFKuE0qyrlfCCThZ4B1VXX+Qm
jYLRMB8GA1UdIwQYMBaAFKuE0qyrlfCCThZ4B1VXX+QmjYLRMAwGA1UdEwQFMAMB
Af8wCQYHKoZIzj0EAQNHADBEAiBATB6aVJxDD6YAxEM4vf6Sbg2Ty334ldXpkNwc
TF+SngIgZ/f59kgDLf6YA04iLw1fUv5Wf1nLYJWwgrRFON5+zvw=
-----END CERTIFICATE-----
-----BEGIN EC PARAMETERS-----
MIH3AgEBMCwGByqGSM49AQECIQD/////AAAAAQAAAAAAAAAAAAAAAP//////////
/////zBbBCD/////AAAAAQAAAAAAAAAAAAAAAP///////////////AQgWsY12Ko6
k+ez671VdpiGvGUdBrDMU7D2O848PifSYEsDFQDEnTYIhucEk2pmeOETnSa3gZ9+
kARBBGsX0fLhLEJH+Lzm5WOkQPJ3A32BLeszoPShOUXYmMKWT+NC4v4af5uO5+tK
fA+eFivOM1drMV7Oy7ZAaDe/UfUCIQD/////AAAAAP//////////vOb6racXnoTz
ucrC/GMlUQIBAQ==
-----END EC PARAMETERS-----
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIAcPCHJ61KBKnN1ZyU2JaHcItW/JXTB3DujRyc4Ki7RqoAoGCCqGSM49
AwEHoUQDQgAE5itp4r9ln5e+Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY
+cPEox5W4nyDSNsWGhz1HX7xlC1Lz3IiwQ==
-----END EC PRIVATE KEY-----

BUG=522228

Change-Id: I3723411a633dc07c4640027de07500293f8f7913
Reviewed-on: https://boringssl-review.googlesource.com/6853
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:51:14 +00:00
David Benjamin
f6094e05ef Don't allow EVP_PKEY_RSA2.
OpenSSL accepts both OID 2.5.8.1.1 and OID 1.2.840.113549.1.1.1 for RSA
public keys. The latter comes from RFC 3279 and is widely implemented.
The former comes from the ITU-T version of X.509. Interestingly,
2.5.8.1.1 actually has a parameter, which OpenSSL ignores:

  rsa ALGORITHM ::= {
     KeySize
     IDENTIFIED BY id-ea-rsa
  }
  KeySize ::= INTEGER

Remove support for 2.5.8.1.1 completely. In tests with a self-signed
certificate and code inspection:

- IE11 on Win8 does not accept the certificate in a TLS handshake at
  all. Such a certificate is fatal and unbypassable. However Microsoft's
  libraries do seem to parse it, so Chrome on Windows allows one to
  click through the error. I'm guessing either the X.509 stack accepts
  it while the TLS stack doesn't recognize it as RSA or the X.509 stack
  is able to lightly parse it but not actually understand the key. (The
  system certificate UI didn't display it as an RSA key, so probably the
  latter?)

- Apple's certificate library on 10.11.2 does not parse the certificate
  at all. Both Safari and Chrome on Mac treat it as a fatal and
  unbypassable error.

- mozilla::pkix, from code inspection, does not accept such
  certificates. However, Firefox does allow clicking through the error.
  This is likely a consequence of mozilla::pkix and NSS having different
  ASN.1 stacks. I did not test this, but I expect this means Chrome on
  Linux also accepts it.

Given IE and Safari's results, it should be safe to simply remove this.
Firefox's data point is weak (perhaps someone is relying on being able
to click-through a self-signed 2.5.8.1.1 certificate), but it does
further ensure no valid certificate could be doing this.

The following is the 2.5.8.1.1 certificate I constructed to test with.
The private key is key.pem from ssl/test/runner:

-----BEGIN CERTIFICATE-----
MIICVTCCAb6gAwIBAgIJAPuwTC6rEJsMMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV
BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX
aWRnaXRzIFB0eSBMdGQwHhcNMTQwNDIzMjA1MDQwWhcNMTcwNDIyMjA1MDQwWjBF
MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50
ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGcMAoGBFUIAQECAgQAA4GNADCBiQKBgQDY
K8imMuRi/03z0K1Zi0WnvfFHvwlYeyK9Na6XJYaUoIDAtB92kWdGMdAQhLciHnAj
kXLI6W15OoV3gA/ElRZ1xUpxTMhjP6PyY5wqT5r6y8FxbiiFKKAnHmUcrgfVW28t
Q+0rkLGMryRtrukXOgXBv7gcrmU7G1jC2a7WqmeI8QIDAQABo1AwTjAdBgNVHQ4E
FgQUi3XVrMsIvg4fZbf6Vr5sp3Xaha8wHwYDVR0jBBgwFoAUi3XVrMsIvg4fZbf6
Vr5sp3Xaha8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQUFAAOBgQAIZuUICtYv
w3cbpCGX6HNCtyI0guOfbytcdwzRkQaCsYNSDrTxrSSWxHwqg3Dl/RlvS+T3Yaua
Xkioadstwt7GDP6MwpIpdbjchh0XZd3kjdJWqXSvihUDpRePNjNS2LmJW8GWfB3c
F6UVyNK+wcApRY+goREIhyYupAHUexR7FQ==
-----END CERTIFICATE-----

BUG=522228

Change-Id: I031d03c0f53a16cbc749c4a5d8be6efca50dc863
Reviewed-on: https://boringssl-review.googlesource.com/6852
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:43:37 +00:00
David Benjamin
c612e61e1d Fix minor stylistic problem.
Normally this would be pretty scary:
  if (...) {
  } if (...) {
  }
But it's an early return anyway.

Change-Id: I0a8965b5e294d3aaa803be47f4006ea0311c431d
Reviewed-on: https://boringssl-review.googlesource.com/6851
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:35:35 +00:00
David Benjamin
719594e512 Un-const EVP_PKEY_CTX_set0_rsa_oaep_label and fix overflow check.
It takes ownership of the buffer, so it's not actually const. The
const-ness gets dropped once it transits through EVP_PKEY_CTX_ctrl.

Also compare against INT_MAX explicitly for the overflow check. I'm not sure
whether the casting version is undefined, but comparing against INT_MAX matches
the rest of the codebase when transiting in and out of signed ints.

Change-Id: I131165a4b5f0ebe02c6db3e7e3e0d1af5b771710
Reviewed-on: https://boringssl-review.googlesource.com/6850
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:34:38 +00:00
David Benjamin
b6155e60f3 Remove app_data from EVP_PKEY_CTX.
It's never used. It's not clear why one would want such a thing.
EVP_PKEY_CTX has no way for callers to register callbacks, which means
there shouldn't be a way for the library to present you an EVP_PKEY_CTX
out-of-context. (Whereas app_data/ex_data makes sense on SSL because of
its numerous callbacks or RSA because of RSA_METHOD.)

Change-Id: I55af537ab101682677af34f6ac1f2c27b5899a89
Reviewed-on: https://boringssl-review.googlesource.com/6849
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:29:34 +00:00
David Benjamin
4e98e5c903 Implement pkey_ec_keygen with EC_KEY APIs.
This removes the last caller of EVP_PKEY_copy_parameters within the
library.

Change-Id: I6af138d364973b18f52baf55c36c50a24a56bd44
Reviewed-on: https://boringssl-review.googlesource.com/6848
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:28:43 +00:00
David Benjamin
692878a5f4 Remove EVP_PKEY_CTRL_EC_PARAMGEN_CURVE_NID.
This is never exposed.

Change-Id: I332bc45f724eb42d68a0839e72b21593d01005ee
Reviewed-on: https://boringssl-review.googlesource.com/6847
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:27:42 +00:00
David Benjamin
8ac35f0274 Remove unused EVP_PKEY_METHOD hooks.
foo_init hooks are never implemented. Even upstream never uses them. The
flags member is also never used. We also don't expose paramgen, so
remove it.

Change-Id: I51d9439316c5163520ab7168693c457f33e59417
Reviewed-on: https://boringssl-review.googlesource.com/6846
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:23:46 +00:00
David Benjamin
9bf1b1b440 Remove group_clear_finish EC_GROUP hooks.
These are never called. Group parameters are not secret anyway. This is
a remnant of upstream's EC_GROUP_clear_free.

Change-Id: I23a4076eae8e4561abddbe74d0ba72641532f229
Reviewed-on: https://boringssl-review.googlesource.com/6823
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:17:43 +00:00
David Benjamin
acb2451807 Rename the BIGNUM ASN.1 functions.
There's many ways to serialize a BIGNUM, so not including asn1 in the name is
confusing (and collides with BN_bn2cbb_padded). Since BN_asn12bn looks
ridiculous, match the parse/marshal naming scheme of other modules instead.

Change-Id: I53d22ae0537a98e223ed943e943c48cb0743cf51
Reviewed-on: https://boringssl-review.googlesource.com/6822
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:37:44 +00:00
David Benjamin
647cd02e59 Fix 32-bit build.
__uint128_t and friends don't exist in 32-bit. (Build fix for
https://boringssl-review.googlesource.com/#/c/6975/.)

Change-Id: I959a1f23c8cb3f11344f1da50cecd82d3080e3a0
Reviewed-on: https://boringssl-review.googlesource.com/6983
Reviewed-by: Adam Langley <agl@google.com>
2016-01-27 22:29:52 +00:00
David Benjamin
b04c905da9 Remove the arch-specific HOST_c2l/HOST_l2c implementations.
These do not appear to have much discernable effect on performance. Three
comparison runs:

Before:
Did 5414000 SHA-1 (16 bytes) operations in 1000009us (5413951.3 ops/sec): 86.6 MB/s
Did 1607000 SHA-1 (256 bytes) operations in 1000403us (1606352.6 ops/sec): 411.2 MB/s
Did 70000 SHA-1 (8192 bytes) operations in 1014426us (69004.5 ops/sec): 565.3 MB/s
Did 2991000 SHA-256 (16 bytes) operations in 1000204us (2990390.0 ops/sec): 47.8 MB/s
Did 741000 SHA-256 (256 bytes) operations in 1000371us (740725.2 ops/sec): 189.6 MB/s
Did 31000 SHA-256 (8192 bytes) operations in 1019327us (30412.2 ops/sec): 249.1 MB/s
Did 2340000 SHA-512 (16 bytes) operations in 1000312us (2339270.1 ops/sec): 37.4 MB/s
Did 880000 SHA-512 (256 bytes) operations in 1000879us (879227.2 ops/sec): 225.1 MB/s
Did 44000 SHA-512 (8192 bytes) operations in 1013355us (43420.1 ops/sec): 355.7 MB/s
After:
Did 5259000 SHA-1 (16 bytes) operations in 1000013us (5258931.6 ops/sec): 84.1 MB/s
Did 1547000 SHA-1 (256 bytes) operations in 1000011us (1546983.0 ops/sec): 396.0 MB/s
Did 69000 SHA-1 (8192 bytes) operations in 1001089us (68924.9 ops/sec): 564.6 MB/s
Did 2984000 SHA-256 (16 bytes) operations in 1000207us (2983382.4 ops/sec): 47.7 MB/s
Did 734000 SHA-256 (256 bytes) operations in 1000317us (733767.4 ops/sec): 187.8 MB/s
Did 31000 SHA-256 (8192 bytes) operations in 1021065us (30360.5 ops/sec): 248.7 MB/s
Did 2324000 SHA-512 (16 bytes) operations in 1000116us (2323730.4 ops/sec): 37.2 MB/s
Did 828000 SHA-512 (256 bytes) operations in 1001046us (827134.8 ops/sec): 211.7 MB/s
Did 43000 SHA-512 (8192 bytes) operations in 1003381us (42855.1 ops/sec): 351.1 MB/s

---

Before:
Did 5415000 SHA-1 (16 bytes) operations in 1000055us (5414702.2 ops/sec): 86.6 MB/s
Did 1604000 SHA-1 (256 bytes) operations in 1000524us (1603159.9 ops/sec): 410.4 MB/s
Did 71000 SHA-1 (8192 bytes) operations in 1007686us (70458.5 ops/sec): 577.2 MB/s
Did 2984000 SHA-256 (16 bytes) operations in 1000472us (2982592.2 ops/sec): 47.7 MB/s
Did 738000 SHA-256 (256 bytes) operations in 1000885us (737347.4 ops/sec): 188.8 MB/s
Did 30000 SHA-256 (8192 bytes) operations in 1020475us (29398.1 ops/sec): 240.8 MB/s
Did 2297000 SHA-512 (16 bytes) operations in 1000391us (2296102.2 ops/sec): 36.7 MB/s
Did 882000 SHA-512 (256 bytes) operations in 1000389us (881657.0 ops/sec): 225.7 MB/s
Did 43000 SHA-512 (8192 bytes) operations in 1001313us (42943.6 ops/sec): 351.8 MB/s
After:
Did 5228000 SHA-1 (16 bytes) operations in 1000035us (5227817.0 ops/sec): 83.6 MB/s
Did 1575000 SHA-1 (256 bytes) operations in 1000410us (1574354.5 ops/sec): 403.0 MB/s
Did 69000 SHA-1 (8192 bytes) operations in 1004180us (68712.8 ops/sec): 562.9 MB/s
Did 2884000 SHA-256 (16 bytes) operations in 1000093us (2883731.8 ops/sec): 46.1 MB/s
Did 718000 SHA-256 (256 bytes) operations in 1000413us (717703.6 ops/sec): 183.7 MB/s
Did 31000 SHA-256 (8192 bytes) operations in 1030257us (30089.6 ops/sec): 246.5 MB/s
Did 2286000 SHA-512 (16 bytes) operations in 1000172us (2285606.9 ops/sec): 36.6 MB/s
Did 979000 SHA-512 (256 bytes) operations in 1000384us (978624.2 ops/sec): 250.5 MB/s
Did 47000 SHA-512 (8192 bytes) operations in 1017846us (46175.9 ops/sec): 378.3 MB/s

---

Before:
Did 5429000 SHA-1 (16 bytes) operations in 1000104us (5428435.4 ops/sec): 86.9 MB/s
Did 1604000 SHA-1 (256 bytes) operations in 1000473us (1603241.7 ops/sec): 410.4 MB/s
Did 69000 SHA-1 (8192 bytes) operations in 1002621us (68819.6 ops/sec): 563.8 MB/s
Did 3021000 SHA-256 (16 bytes) operations in 1000152us (3020540.9 ops/sec): 48.3 MB/s
Did 735000 SHA-256 (256 bytes) operations in 1000048us (734964.7 ops/sec): 188.2 MB/s
Did 31000 SHA-256 (8192 bytes) operations in 1019902us (30395.1 ops/sec): 249.0 MB/s
Did 2301000 SHA-512 (16 bytes) operations in 1000207us (2300523.8 ops/sec): 36.8 MB/s
Did 881000 SHA-512 (256 bytes) operations in 1001122us (880012.6 ops/sec): 225.3 MB/s
Did 44000 SHA-512 (8192 bytes) operations in 1015313us (43336.4 ops/sec): 355.0 MB/s
After:
Did 5264000 SHA-1 (16 bytes) operations in 1000061us (5263678.9 ops/sec): 84.2 MB/s
Did 1587000 SHA-1 (256 bytes) operations in 1000293us (1586535.1 ops/sec): 406.2 MB/s
Did 71000 SHA-1 (8192 bytes) operations in 1007587us (70465.4 ops/sec): 577.3 MB/s
Did 2967000 SHA-256 (16 bytes) operations in 1000240us (2966288.1 ops/sec): 47.5 MB/s
Did 737000 SHA-256 (256 bytes) operations in 1000874us (736356.4 ops/sec): 188.5 MB/s
Did 31000 SHA-256 (8192 bytes) operations in 1019630us (30403.2 ops/sec): 249.1 MB/s
Did 2326000 SHA-512 (16 bytes) operations in 1000413us (2325039.8 ops/sec): 37.2 MB/s
Did 885000 SHA-512 (256 bytes) operations in 1000253us (884776.2 ops/sec): 226.5 MB/s
Did 44000 SHA-512 (8192 bytes) operations in 1013216us (43426.1 ops/sec): 355.7 MB/s

Change-Id: Ifd4500f4e9f41ffc0f73542141e8888b4d7f1e0b
Reviewed-on: https://boringssl-review.googlesource.com/6652
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:26:32 +00:00
David Benjamin
8f2d4e344c Fix documentation string.
SSL_CTX_set_retain_only_sha256_of_client_certs's comment wasn't quite right.

Change-Id: I40527eebf2988d117cd8bd544bb5f16936c2cbfb
Reviewed-on: https://boringssl-review.googlesource.com/6982
Reviewed-by: Adam Langley <agl@google.com>
2016-01-27 22:20:32 +00:00
Brian Smith
87c7640773 Use |inline| in crypto/poly1305/poly1305_vec.c.
The code was using `#define INLINE` instead, but we have `inline` so
use it.

Change-Id: Id05eaec4720061c5d9a7278e20127c2bebcb2495
Reviewed-on: https://boringssl-review.googlesource.com/6976
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-27 22:15:34 +00:00
Brian Smith
24e428899b Define int128_t and uint128_t in one place.
Change-Id: Ia93130aadf319eaba1b6f2ec2896a4c50d9e8ede
Reviewed-on: https://boringssl-review.googlesource.com/6975
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-27 22:15:04 +00:00
Brian Smith
f547007332 Use |alignas| more in crypto/chacha/chacha_vec.c.
Commit 75a64c08fc missed one case where
the GCC syntax should have been replaced with |alignas|.

Change-Id: Iebdaa9c9a2c0aff171f0b5d4daac607e351a4b7e
Reviewed-on: https://boringssl-review.googlesource.com/6974
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-27 22:12:22 +00:00
Brian Smith
9333d6df11 Fix data <-> function pointer casts in thread_win.c.
The uses of |memcpy| to cast pointer-to-function to pointer-to-data and
back again did not have well-defined semantics. Use a union instead to
avoid the need for such a conversion get well-defined semantics.

Change-Id: I8ee54a83ba75440f7bc78c194eb55e2cf09b05d8
Reviewed-on: https://boringssl-review.googlesource.com/6972
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-27 22:08:26 +00:00
Brian Smith
f5f4be8fac Fix pointer-to-non-volatile cast in thread_win.c.
Casting a pointer-to-non-volatile to pointer-to-volatile can be a no-op
as the compiler only requires volatile semantics when the pointed-to
object is a volatile object and there are no pointers-to-non-volatile
involved. This probably doesn't matter unless building with the MSVC
-volatile:iso flag, and maybe not even then, but it is good practice
anyway.

Change-Id: I94900d3dc61de3b8ce2ddecab2811907a9a7adbf
Reviewed-on: https://boringssl-review.googlesource.com/6973
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-27 22:06:11 +00:00
Adam Langley
54a8d7c14f Use Barrett reduction in CBC processing rather than tricks.
Division isn't constant-time on Intel chips so the code was adding a
large multiple of md_size to try and force the operation to always take
the maximum amount of time.

I'm less convinced, these days, that compilers aren't going to get smart
enough to optimise that away so use Barrett reduction instead.

Change-Id: Ib8c514192682a2fcb4b1fb7e7c6dd1301d9888d0
Reviewed-on: https://boringssl-review.googlesource.com/6906
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:05:01 +00:00
David Benjamin
91b2501f02 Add functions for accessing read_sequence and write_sequence.
OpenSSL 1.1.0 doesn't seem to have these two, so this isn't based on anything.
Have them return uint64_t in preparation for switching the internal
representation to uint64_t so ssl_record_sequence_update can go away.

Change-Id: I21d55e9a29861c992f409ed293e0930a7aaef7a3
Reviewed-on: https://boringssl-review.googlesource.com/6941
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:03:57 +00:00
David Benjamin
cdd0b7e775 Add SSL_CTX_set_retain_only_sha256_of_client_certs.
We have the hook on the SSL_CTX, but it should be possible to set it without
reaching into SSL_CTX.

Change-Id: I93db070c7c944be374543442a8de3ce655a28928
Reviewed-on: https://boringssl-review.googlesource.com/6880
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:02:53 +00:00
David Benjamin
4b9205b583 Align SSL_CTX_set1_tls_channel_id with SSL_set1_tls_channel_id.
They should use the same P-256 check.

Change-Id: I66dd63663e638cba35b8f70f9cf119c718af4aec
Reviewed-on: https://boringssl-review.googlesource.com/6845
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 21:51:39 +00:00
David Benjamin
b83003ebc6 Don't initialize enc_method before version negotiation.
Move it into ssl->s3 so it automatically behaves correctly on SSL_clear.
ssl->version is still a mess though.

Change-Id: I17a692a04a845886ec4f8de229fa6cf99fa7e24a
Reviewed-on: https://boringssl-review.googlesource.com/6844
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 21:38:12 +00:00
David Benjamin
a565d29ce6 Remove alert mapping machinery.
For TLS, this machinery only exists to swallow no_certificate alerts
which only get sent in an SSL 3.0 codepath anyway. It's much less a
no-op for SSL 3.0 which, strictly speaking, has only a subset of TLS's
alerts.

This gets messy around version negotiation because of the complex
relationship between enc_method, have_version, and version which all get
set at different times. Given that SSL 3.0 is nearly dead and all these
alerts are fatal to the connection anyway, this doesn't seem worth
carrying around. (It doesn't work very well anyway. An SSLv3-only server
may still send a record_overflow alert before version negotiation.)

This removes the last place enc_method is accessed prior to version
negotiation.

Change-Id: I79a704259fca69e4df76bd5a6846c9373f46f5a9
Reviewed-on: https://boringssl-review.googlesource.com/6843
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 21:28:48 +00:00
David Benjamin
a1e9cabd8b Replace enc_flags with normalized version checks.
This removes the various non-PRF checks from SSL3_ENC_METHOD so that can
have a clearer purpose. It also makes TLS 1.0 through 1.2's
SSL3_ENC_METHOD tables identical and gives us an assert to ensure
nothing accesses the version bits before version negotiation.
Accordingly, ssl_needs_record_splitting was reordered slightly so we
don't rely on enc_method being initialized to TLS 1.2
pre-version-negotiation.

This leaves alert_value as the only part of SSL3_ENC_METHOD which may be
accessed before version negotiation.

Change-Id: If9e299e2ef5511b5fa442b2af654eed054c3e675
Reviewed-on: https://boringssl-review.googlesource.com/6842
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 21:17:55 +00:00
Adam Langley
ef7dba6ac7 Fix error in ce9d85ee.
I used a size_t out of habit, but |RSA_public_decrypt| is an old-style
function.

Change-Id: Ibd94d03743fe0099d61578ec15c19fa5333127db
2016-01-26 15:31:23 -08:00
Adam Langley
ce9d85eedd Tweaks for node.js
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.

Changes:
  ∙ Support verify_recover. This is very obscure and the motivation
    appears to be https://github.com/nodejs/node/issues/477 – where it's
    not clear that anyone understands what it means :(
  ∙ Add a few, no-op #defines
  ∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
    reach into these structs in the future.
  ∙ Add EC_get_builtin_curves.
  ∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
    decrepit.

Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:42 +00:00
Adam Langley
eac0ce09d8 Have doc.go parse struct comments.
In code, structs that happened to have a '(' somewhere in their body
would cause the parser to go wrong. This change fixes that and updates
the comments on a number of structs.

Change-Id: Ia76ead266615a3d5875b64a0857a0177fec2bd00
Reviewed-on: https://boringssl-review.googlesource.com/6970
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:23 +00:00
David Benjamin
241ae837f0 Add some tests to ensure we ignore bogus curves and ciphers.
We haven't had problems with this, but make sure it stays that way.
Bogus signature algorithms are already covered.

Change-Id: I085350d89d79741dba3f30fc7c9f92de16bf242a
Reviewed-on: https://boringssl-review.googlesource.com/6910
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-26 21:51:55 +00:00
David Benjamin
f6494f4928 Add a SSL_get_pending_cipher API.
Conscrypt needs to, in the certificate verification callback, know the key
exchange + auth method of the current cipher suite to pass into
X509TrustManager.checkServerTrusted. Currently it reaches into the struct to
get it. Add an API for this.

Change-Id: Ib4e0a1fbf1d9ea24e0114f760b7524e1f7bafe33
Reviewed-on: https://boringssl-review.googlesource.com/6881
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-26 21:51:02 +00:00
David Benjamin
7027d25c6b Also add a no-op stub for OPENSSL_config.
Apparently OpenSSL's API is made entirely of initialization functions.
Some external libraries like to initialize with OPENSSL_config instead.

Change-Id: I28efe97fc5eb21309f560c84112b80e947f8bb17
Reviewed-on: https://boringssl-review.googlesource.com/6981
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 15:48:51 +00:00
David Benjamin
e5aa791a1c Add a few more no-op stubs for cURL compatibility.
With these stubs, cURL should not need any BoringSSL #ifdefs at all,
except for their OCSP #ifdefs (which can switch to the more generally
useful OPENSSL_NO_OCSP) and the workaround for wincrypt.h macro
collisions. That we intentionally leave to the consumer rather than add
a partial hack that makes the build sensitive to include order.

(I'll send them a patch upstream once this cycles in.)

Change-Id: I815fe67e51e80e9aafa9b91ae68867ca1ff1d623
Reviewed-on: https://boringssl-review.googlesource.com/6980
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 15:48:41 +00:00
David Benjamin
5aae776ede Remove calls to ERR_load_crypto_strings.
Since the error string logic was rewritten, this hasn't done anything.

Change-Id: Icb73dca65e852bb3c7d04c260d591906ec72c15f
Reviewed-on: https://boringssl-review.googlesource.com/6961
Reviewed-by: Adam Langley <agl@google.com>
2016-01-25 23:09:08 +00:00
Adam Langley
75a64c08fc Remove some mingw support cruft.
This was needed for Android, but “the new version of mingw has moved all
of time_s.h into time.h” [1].

[1] https://android-review.googlesource.com/#/c/196597/

Change-Id: I17e66ed93606f3e6a774af3290c15b5ca151449f
Reviewed-on: https://boringssl-review.googlesource.com/6971
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-25 23:05:45 +00:00
Brian Smith
7cae9f5b6c Use |alignas| for alignment.
MSVC doesn't have stdalign.h and so doesn't support |alignas| in C
code. Define |alignas(x)| as a synonym for |__decltype(align(x))|
instead for it.

This also fixes -Wcast-qual warnings in rsaz_exp.c.

Change-Id: Ifce9031724cb93f5a4aa1f567e7af61b272df9d5
Reviewed-on: https://boringssl-review.googlesource.com/6924
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-25 23:05:04 +00:00
Brian Smith
34749f47da Remove unnecessary assignment of |e| in |rsa_setup_blinding|.
After its initial assignment, |e| is immediately reassigned another
value and so the initial assignment from |BN_CTX_get| is useless. If
that were not the case, then the |BN_free(e)| at the end of the
function would be very bad.

Change-Id: Id63a172073501c8ac157db9188a22f55ee36b205
Reviewed-on: https://boringssl-review.googlesource.com/6951
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-23 17:08:23 +00:00
David Benjamin
232127d245 Fold EC_GROUP_new_curve_GFp and EC_GROUP_set_generator into a EC_GROUP_new_arbitrary.
This is only for Conscrypt which always calls the pair in succession. (Indeed
it wouldn't make any sense to not call it.) Remove those two APIs and replace
with a single merged API. This way incomplete EC_GROUPs never escape outside
our API boundary and EC_GROUPs may *finally* be made immutable.

Also add a test for this to make sure I didn't mess it up.

Add a temporary BORINGSSL_201512 define to ease the transition for Conscrypt.
Conscrypt requires https://android-review.googlesource.com/#/c/187801/ before
picking up this change.

Change-Id: I3706c2ceac31ed2313175ba5ee724bd5c74ef6e1
Reviewed-on: https://boringssl-review.googlesource.com/6550
Reviewed-by: Adam Langley <agl@google.com>
2016-01-21 22:35:46 +00:00
David Benjamin
95219feafd Fix some documentation comments.
The new OPENSSL_PRINTF_FORMAT_FUNC macro let doc.go catch a few problems. It
also confuses doc.go, but this CL doesn't address that. At some point we
probably need to give it a real C parser.

Change-Id: I39f945df04520d1e0a0ba390cac7b308baae0622
Reviewed-on: https://boringssl-review.googlesource.com/6940
Reviewed-by: Adam Langley <agl@google.com>
2016-01-21 22:12:08 +00:00
Brian Smith
d3a4e280db Fix trivial -Wcast-qual violations.
Fix casts from const to non-const where dropping the constness is
completely unnecessary. The changes to chacha_vec.c don't result in any
changes to chacha_vec_arm.S.

Change-Id: I2f10081fd0e73ff5db746347c5971f263a5221a6
Reviewed-on: https://boringssl-review.googlesource.com/6923
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 21:06:02 +00:00
Brian Smith
a646258c14 Enable stronger format string checking |-Wformat=2|.
Also, factor out flags based to both the C and C++ compiler into a
single variable.

Change-Id: I432de0cc516e95a0d48146fae2dda8b7b3b38d4b
Reviewed-on: https://boringssl-review.googlesource.com/6922
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 21:00:31 +00:00
Brian Smith
d92f1d39a8 Fix |sscanf| format string in cpu-intel.c.
Fix the signness of the format flag in the |sscanf| call in cpu-intel.c.

Change-Id: I31251d79aa146bf9c78be47020ee83d30864a3d2
Reviewed-on: https://boringssl-review.googlesource.com/6921
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 20:59:35 +00:00
Brian Smith
8d3c43e4b1 Annotate |ERR_add_error_dataf| as |OPENSSL_PRINTF_FORMAT_FUNC|.
Besides being a good idea anyway, this avoids clang warning about using
a non-literal format string when |ERR_add_error_dataf| calls
|BIO_vsnprintf|.

Change-Id: Iebc84d9c9d85e08e93010267d473387b661717a5
Reviewed-on: https://boringssl-review.googlesource.com/6920
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 20:59:18 +00:00
Brian Smith
061332f216 Define |OPENSSL_PRINTF_FORMAT_FUNC| for format string annotations.
This centralizes the conditional logic into openssl/base.h so that it
doesn't have to be repeated. The name |OPENSSL_PRINTF_FORMAT_FUNC| was
chosen in anticipation of eventually defining an
|OPENSSL_PRINTF_FORMAT_ARG| for MSVC-style parameter annotations.

Change-Id: I273e6eddd209e696dc9f82099008c35b6d477cdb
Reviewed-on: https://boringssl-review.googlesource.com/6909
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 20:58:51 +00:00
Brian Smith
0687bdfc12 Fix -Wformat-nonliteral violation in ssl_cipher.c.
Besides avoiding the -Wformat-nonliteral warning, it is easier to
review (changes to) the code when the format string is passed to the
function as a literal.

Change-Id: I5093ad4494d5ebeea3f2671509b916cd6c5fb173
Reviewed-on: https://boringssl-review.googlesource.com/6908
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 20:56:59 +00:00