Commit Graph

41 Commits

Author SHA1 Message Date
David Benjamin
d3440b4d63 Give SSL_PRIVATE_KEY_METHOD a message-based API.
This allows us to implement custom RSA-PSS-based keys, so the async TLS
1.3 tests can proceed. For now, both sign and sign_digest exist, so
downstreams only need to manage a small change atomically. We'll remove
sign_digest separately.

In doing so, fold all the *_complete hooks into a single complete hook
as no one who implemented two operations ever used different function
pointers for them.

While I'm here, I've bumped BORINGSSL_API_VERSION. I do not believe we
have any SSL_PRIVATE_KEY_METHOD versions who cannot update atomically,
but save a round-trip in case we do. It's free.

Change-Id: I7f031aabfb3343805deee429b9e244aed5d76aed
Reviewed-on: https://boringssl-review.googlesource.com/8786
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-15 18:26:45 +00:00
David Benjamin
0c0b7e1e1f Widen SSL_PRIVATE_KEY_METHOD types to include the curve name.
This makes custom private keys and EVP_PKEYs symmetric again. There is
no longer a requirement that the caller pre-filter the configured
signing prefs.

Also switch EVP_PKEY_RSA to NID_rsaEncryption. These are identical, but
if some key types are to be NIDs, we should make them all NIDs.

Change-Id: I82ea41c27a3c57f4c4401ffe1ccad406783e4c64
Reviewed-on: https://boringssl-review.googlesource.com/8785
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-15 18:17:32 +00:00
David Benjamin
ca3d545d7f Add SSL_set_signing_algorithm_prefs.
This gives us a sigalg-based API for configuring signing algorithms.

Change-Id: Ib746a56ebd1061eadd2620cdb140d5171b59bc02
Reviewed-on: https://boringssl-review.googlesource.com/8784
Reviewed-by: Adam Langley <agl@google.com>
2016-07-15 18:10:29 +00:00
David Benjamin
32a66d51a6 Tidy up a few certificate-related utility functions.
These will all want to be shared with the TLS 1.3 handshake.

Change-Id: I4e50dc0ed2295d43c7ae800015d71c1406311801
Reviewed-on: https://boringssl-review.googlesource.com/8776
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-14 16:07:25 +00:00
David Benjamin
7944a9f008 Account for key size when selecting RSA-PSS.
RSASSA-PSS with SHA-512 is slightly too large for 1024-bit RSA. One
should not be using 1024-bit RSA, but it's common enough for tests
(including our own in runner before they were regenerated), that we
should probably do the size check and avoid unnecessary turbulence to
everyone else's test setups.

Change-Id: If0c7e401d7d05404755cba4cbff76de3bc65c138
Reviewed-on: https://boringssl-review.googlesource.com/8746
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-13 15:32:05 +00:00
Steven Valdez
eff1e8d9c7 Adding RSA-PSS signature algorithms.
[Rebased and tests added by davidben.]

In doing so, regenerate the test RSA certificate to be 2048-bit RSA.
RSA-PSS with SHA-512 is actually too large for 1024-bit RSA. Also make
the sigalg test loop test versions that do and don't work which subsumes
the ecdsa_sha1 TLS 1.3 test.

For now, RSA-PKCS1 is still allowed because NSS has yet to implement
RSA-PSS and we'd like to avoid complicated interop testing.

Change-Id: I686b003ef7042ff757bdaab8d5838b7a4d6edd87
Reviewed-on: https://boringssl-review.googlesource.com/8613
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 19:10:51 +00:00
David Benjamin
1fb125c74a Enforce ECDSA curve matching in TLS 1.3.
Implement in both C and Go. To test this, route config into all the
sign.go functions so we can expose bugs to skip the check.

Unfortunately, custom private keys are going to be a little weird since
we can't check their curve type. We may need to muse on what to do here.
Perhaps the key type bit should return an enum that includes the curve?
It's weird because, going forward, hopefully all new key types have
exactly one kind of signature so key type == sig alg == sig alg prefs.

Change-Id: I1f487ec143512ead931e3392e8be2a3172abe3d2
Reviewed-on: https://boringssl-review.googlesource.com/8701
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 18:40:08 +00:00
David Benjamin
887c300e25 Move the key type check from tls12_check_peer_sigalg to ssl_verify_*.
ssl_verify_* already ought to be checking this, so there's only a need
to check against the configured preferences.

Change-Id: I79bc771969c57f953278e622084641e6e20108e3
Reviewed-on: https://boringssl-review.googlesource.com/8698
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 18:25:05 +00:00
David Benjamin
a2d81f1a27 Don't decompose signature algorithms in ssl_rsa.c.
This is a lot more verbose and looks the same between RSA and ECDSA for
now, but it gives us room to implement the various algorithm-specific
checks. ECDSA algorithms must match the curve, PKCS#1 is forbidden in
TLS 1.3, etc.

Change-Id: I348cfae664d7b08195a2ab1190820b410e74c5e9
Reviewed-on: https://boringssl-review.googlesource.com/8694
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 18:23:24 +00:00
Adam Langley
310d3f63f3 Change |EVP_PKEY_up_ref| to return int.
Upstream have added |EVP_PKEY_up_ref|, but their version returns an int.
Having this function with a different signature like that is dangerous
so this change aligns BoringSSL with upstream. Users of this function in
Chromium and internally should already have been updated.

Change-Id: I0a7aeaf1a1ca3b0f0c635e2ee3826aa100b18157
Reviewed-on: https://boringssl-review.googlesource.com/8736
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 17:55:41 +00:00
David Benjamin
d246b81751 Don't decompose sigalgs in key preferences.
Instead, in SSL_set_private_key_digest_prefs, convert the NID list to a
sigalgs list. We'll need to add a new API later when custom key callers
are ready to start advertising RSA-PSS.

This removes all callers of tls12_get_hash except inside the signing and
verifying functions.

Change-Id: Ie534f3b736c6ac6ebeb0d7770d489f72e3321865
Reviewed-on: https://boringssl-review.googlesource.com/8693
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12 16:31:20 +00:00
Steven Valdez
2b8415e8ff Move the Digest/Sign split for SignatureAlgorithms to a lower level.
In order to delay the digest of the handshake transcript and unify
around message-based signing callbacks, a copy of the transcript is kept
around until we are sure there is no certificate authentication.

This removes support for SSL_PRIVATE_KEY_METHOD as a client in SSL 3.0.

Change-Id: If8999a19ca021b4ff439319ab91e2cd2103caa64
Reviewed-on: https://boringssl-review.googlesource.com/8561
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-01 19:01:33 +00:00
Steven Valdez
f0451ca37d Cleaning up internal use of Signature Algorithms.
The signing logic itself still depends on pre-hashed messages and will be fixed
in later commits.

Change-Id: I901b0d99917c311653d44efa34a044bbb9f11e57
Reviewed-on: https://boringssl-review.googlesource.com/8545
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-29 21:22:25 +00:00
Tom Thorogood
66b2fe8e02 Add |SSL_CTX_set_private_key_method| to parallel |SSL_set_private_key_method|
This change adds a |SSL_CTX_set_private_key_method| method that sets key_method on a SSL_CTX's cert.

It allows the private key method to be set once and inherited.

A copy of key_method (from SSL_CTX's cert to SSL's cert) is added in |ssl_cert_dup|.

Change-Id: Icb62e9055e689cfe2d5caa3a638797120634b63f
Reviewed-on: https://boringssl-review.googlesource.com/7340
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-07 18:16:58 +00:00
David Benjamin
758d12732a Add get0 getters for EVP_PKEY.
Right now your options are:
- Bounce on a reference and deal with cleanup needlessly.
- Manually check the type tag and peek into the union.

We probably have no hope of opaquifying this struct, but for new code, let's
recommend using this function rather than the more error-prone thing.

Change-Id: I9b39ff95fe4264a3f7d1e0d2894db337aa968f6c
Reviewed-on: https://boringssl-review.googlesource.com/6551
Reviewed-by: Adam Langley <agl@google.com>
2015-11-20 23:34:12 +00:00
David Benjamin
3a59611726 size_t SSL*_use_*_ASN1.
So long as we're not getting rid of them (the certificate variants may
be useful when we decouple from crypto/x509 anyway), get the types and
bounds checks right.

Also reject trailing data and require the input be a single element.
Note: this is a slight compatibility risk, but we did it for
SSL*_use_RSAPrivateKey_ASN1 previously and I think it's probably worth
seeing if anything breaks here.

Change-Id: I64fa3fc6249021ccf59584d68e56ff424a190082
Reviewed-on: https://boringssl-review.googlesource.com/6490
Reviewed-by: Adam Langley <agl@google.com>
2015-11-16 23:59:14 +00:00
nagendra modadugu
3398dbf279 Add server-side support for asynchronous RSA decryption.
Change-Id: I6df623f3e9bc88acc52043f16b34649b7af67663
Reviewed-on: https://boringssl-review.googlesource.com/5531
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 20:26:20 +00:00
Steven Valdez
0d62f26c36 Adding more options for signing digest fallback.
Allow configuring digest preferences for the private key. Some
smartcards have limited support for signing digests, notably Windows
CAPI keys and old Estonian smartcards. Chromium used the supports_digest
hook in SSL_PRIVATE_KEY_METHOD to limit such keys to SHA1. However,
detecting those keys was a heuristic, so some SHA256-capable keys
authenticating to SHA256-only servers regressed in the switch to
BoringSSL. Replace this mechanism with an API to configure digest
preference order. This way heuristically-detected SHA1-only keys may be
configured by Chromium as SHA1-preferring rather than SHA1-requiring.

In doing so, clean up the shared_sigalgs machinery somewhat.

BUG=468076

Change-Id: I996a2df213ae4d8b4062f0ab85b15262ca26f3c6
Reviewed-on: https://boringssl-review.googlesource.com/5755
Reviewed-by: Adam Langley <agl@google.com>
2015-09-23 21:55:01 +00:00
David Benjamin
443a1f65e2 Toss file-related convenience bits of ssl/ into a corner.
Quite a lot of consumers of the SSL stack will never need to touch files from
the SSL stack, but enough do that we can't just ditch them. Toss that all into
their own file so a static linker can drop it.

Change-Id: Ia07de939889eb09e3ab16aebcc1b6869ca8b75a0
Reviewed-on: https://boringssl-review.googlesource.com/5820
Reviewed-by: Adam Langley <agl@google.com>
2015-09-08 23:34:40 +00:00
nagendra modadugu
601448aa13 Add server-side support for asynchronous signing.
The RSA key exchange needs decryption and is still unsupported.

Change-Id: I8c13b74e25a5424356afbe6e97b5f700a56de41f
Reviewed-on: https://boringssl-review.googlesource.com/5467
Reviewed-by: Adam Langley <agl@google.com>
2015-07-31 01:14:29 +00:00
David Benjamin
3570d73bf1 Remove the func parameter to OPENSSL_PUT_ERROR.
Much of this was done automatically with
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/'
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+,  ([a-zA-Z_0-9]+\);)/\1\2/'

BUG=468039

Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8
Reviewed-on: https://boringssl-review.googlesource.com/5276
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:37 +00:00
David Benjamin
74f711083d Parse RSAPrivateKey with CBS.
This removes the version field from RSA and instead handles versioning
as part of parsing. (As a bonus, we now correctly limit multi-prime RSA
to version 1 keys.)

Most consumers are also converted. old_rsa_priv_{de,en}code are left
alone for now. Those hooks are passed in parameters which match the old
d2i/i2d pattern (they're only used in d2i_PrivateKey and
i2d_PrivateKey).

Include a test which, among other things, checks that public keys being
serialized as private keys are handled properly.

BUG=499653

Change-Id: Icdd5f0382c4a84f9c8867024f29756e1a306ba08
Reviewed-on: https://boringssl-review.googlesource.com/5273
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 22:50:53 +00:00
David Benjamin
d1d8078025 Fold away certificate slots mechanism.
This allows us to remove the confusing EVP_PKEY argument to the
SSL_PRIVATE_KEY_METHOD wrapper functions. It also simplifies some of the
book-keeping around the CERT structure, as well as the API for
configuring certificates themselves. The current one is a little odd as
some functions automatically route to the slot while others affect the
most recently touched slot. Others still (extra_certs) apply to all
slots, making them not terribly useful.

Consumers with complex needs should use cert_cb or the early callback
(select_certificate_cb) to configure whatever they like based on the
ClientHello.

BUG=486295

Change-Id: Ice29ffeb867fa4959898b70dfc50fc00137f01f3
Reviewed-on: https://boringssl-review.googlesource.com/5351
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:22:13 +00:00
David Benjamin
b4d65fda70 Implement asynchronous private key operations for client auth.
This adds a new API, SSL_set_private_key_method, which allows the consumer to
customize private key operations. For simplicity, it is incompatible with the
multiple slots feature (which will hopefully go away) but does not, for now,
break it.

The new method is only routed up for the client for now. The server will
require a decrypt hook as well for the plain RSA key exchange.

BUG=347404

Change-Id: I35d69095c29134c34c2af88c613ad557d6957614
Reviewed-on: https://boringssl-review.googlesource.com/5049
Reviewed-by: Adam Langley <agl@google.com>
2015-06-18 22:14:51 +00:00
David Benjamin
9a10f8fd88 Switch EVP_PKEY_dup calls to EVP_PKEY_up_ref.
Keep internal callers up-to-date with deprecations.

Change-Id: I7ee171afc669592d170f83bd4064857d59332878
Reviewed-on: https://boringssl-review.googlesource.com/4640
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:57:09 +00:00
David Benjamin
9362b6e235 Errors are uint32_t, not unsigned long.
Change-Id: Ic2339b771d949a555b8d05a3b24dc2e990b9d8d3
Reviewed-on: https://boringssl-review.googlesource.com/4555
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:48:01 +00:00
David Benjamin
2755a3eda3 Remove unnecessary NULL checks, part 5.
Finally, the ssl stack.

Change-Id: Iea10e302825947da36ad46eaf3e8e2bce060fde2
Reviewed-on: https://boringssl-review.googlesource.com/4518
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:16:19 +00:00
David Benjamin
2ee94aabf5 Rename ssl_locl.h to internal.h
Match the other internal headers.

Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa
Reviewed-on: https://boringssl-review.googlesource.com/4280
Reviewed-by: Adam Langley <agl@google.com>
2015-04-10 22:14:09 +00:00
David Benjamin
a5a3eeb9cc Remove ssl_cert_inst()
It created the cert structure in SSL_CTX or SSL if it was NULL, but they can
never be NULL as the comments already said.

(Imported from upstream's 2c3823491d8812560922a58677e3ad2db4b2ec8d.)

Change-Id: I97c7bb306d6f3c18597850db9f08023b2ef74839
Reviewed-on: https://boringssl-review.googlesource.com/4042
Reviewed-by: Adam Langley <agl@google.com>
2015-03-19 11:35:46 +00:00
David Benjamin
310db06b79 Don't EVP_PKEY_copy_parameters when configuring cert and key.
I believe this is a remnant of DSA. The logic strangely fails to check for
failure and then goes out of its way to ERR_clear_error. I believe this is so
that keys that are missing parameters silently move on. This dates to
upstream's dfeab0689f69c0b4bd3480ffd37a9cacc2f17d9c, which is SSLeay 0.9.1b. At
that time, EVP_PKEY_copy_parameters only did anything for DSA. (Now it only
does anything for ECDSA.)

My read is that this comes from DSA in PKIX's "optional domain parameters"
craziness. RFC 3279 says:

   If the DSA domain parameters are omitted from the SubjectPublicKeyInfo
   AlgorithmIdentifier and the CA signed the subject certificate using a
   signature algorithm other than DSA, then the subject's DSA domain parameters
   are distributed by other means.

This was probably part of some weird thing where, if your certificate is
missing parameters, the server would know what to use based on the private key.

(Also this was making the malloc tests unhappy.)

Change-Id: I8d8122a9f50a19e2bbe067f311a8e2d30774935c
Reviewed-on: https://boringssl-review.googlesource.com/3484
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 21:03:29 +00:00
Adam Langley
fcf25833bc Reformat the rest of ssl/.
Change-Id: I7dc264f7e29b3ba8be4c717583467edf71bf8dd9
2014-12-18 17:43:03 -08: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
f31e681acf Clean up ssl_set_cert_masks.
It doesn't depend on the cipher now that export ciphers are gone. It need only
be called once. Also remove the valid bit; nothing ever reads it. Its output is
also only used within a function, so make mask_k and mask_a local variables.

So all the configuration-based checks are in one place, change the input
parameter from CERT to SSL and move the PSK and ECDHE checks to the mask
computation. This avoids having to evaluate the temporary EC key for each
cipher.

The remaining uses are on the client which uses them differently (disabled
features rather than enabled ones). Those too may as well be local variables,
so leave a TODO.

Change-Id: Ibcb574341795d4016ea749f0290a793eed798874
Reviewed-on: https://boringssl-review.googlesource.com/2287
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:21:52 +00:00
David Benjamin
a7d1363fcb Prune removed key types from SSL_PKEY_*.
Remove all the logic managing key types that aren't being used anymore.

Change-Id: I101369164588048e64ba1c84a6b8aac8f3a221cd
Reviewed-on: https://boringssl-review.googlesource.com/1567
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:15:32 +00:00
David Benjamin
150c617cfc Add X509_up_ref and use it internally.
Avoid needing to manually increment the reference count and using the right
lock, both here and in Chromium.

Change-Id: If116ebc224cfb1c4711f7e2c06f1fd2c97af21dd
Reviewed-on: https://boringssl-review.googlesource.com/1415
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:06:34 +00:00
David Benjamin
e6e15fc3a1 Use EVP_PKEY_dup instead of manually incrementing the refcount.
Reference counting should be internal to the type, otherwise callers need to
know which lock to use.

Change-Id: If4d805876a321ef6dece115c805e605584ff311e
Reviewed-on: https://boringssl-review.googlesource.com/1414
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:03:56 +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
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
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
d7e23c17a9 Remove serverinfo and custom extensions support.
If we need an extension, we can implement it in-library.

Change-Id: I0eac5affcd8e7252b998b6c86ed2068234134b08
Reviewed-on: https://boringssl-review.googlesource.com/1051
Reviewed-by: Adam Langley <agl@google.com>
2014-06-26 20:51:50 +00:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00