Commit Graph

24 Commits

Author SHA1 Message Date
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
c3ae38b4f8 Remove DH EVP_PKEY hooks.
They would never work. Better notice when callers depend on it than fail at
runtime.

This depends on https://android-review.googlesource.com/#/c/183610/ in
Conscrypt.

Change-Id: I3411f291416df834cf85850890617625a2e76939
Reviewed-on: https://boringssl-review.googlesource.com/6552
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 17:38:06 +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
20c373118c Become partially -Wmissing-variable-declarations-clean.
There's a few things that will be kind of a nuisance and possibly not worth it
(crypto/asn1 dumps a lot of undeclared things, etc.). But it caught some
mistakes. Even without the warning, making sure to include the externs before
defining a function helps catch type mismatches.

Change-Id: I3dab282aaba6023e7cebc94ed7a767a5d7446b08
Reviewed-on: https://boringssl-review.googlesource.com/6484
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 20:09:20 +00:00
Matt Braithwaite
685402fadd Recognize PEM-encoded DSA private keys.
This change makes |EVP_PKEY_asn1_find_str|, which is used by
|PEM_read_bio_PrivateKey|, recognize "DSA" as well as "EC" and "RSA".

Change-Id: I39cce12f600cec6a71df75312a41f8395429af62
Reviewed-on: https://boringssl-review.googlesource.com/5743
Reviewed-by: Adam Langley <agl@google.com>
2015-08-21 17:39:05 +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
cce5a98efb Remove EVP_PKEY_dup.
All callers have been moved to EVP_PKEY_up_ref. (Neither spelling exists
upstream so we only had our own callers to move.)

Change-Id: I267f14054780fe3d6dc1170b7b6ae3811a0d1a9a
Reviewed-on: https://boringssl-review.googlesource.com/5291
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 21:38:39 +00:00
David Benjamin
65ee9b7ce9 Remove EVP_PKEY_HMAC.
This removes EVP_PKEY_HMAC and all the support code around it. EVP_MD requires
a lot of extra glue to support HMAC. This lets us prune it all away.

As a bonus, it removes a (minor) dependency from EVP to the legacy ASN.1 stack.

Change-Id: I5a9e3e39f518429828dbf13d14647fb37d9dc35a
Reviewed-on: https://boringssl-review.googlesource.com/5120
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 00:03:02 +00:00
Matt Braithwaite
444dce4934 Do-nothing fns |OpenSSL_add_all_ciphers| and |OpenSSL_add_all_digests|.
Change-Id: Ied2338f84e03ee1736152b82096335b78a256ca4
Reviewed-on: https://boringssl-review.googlesource.com/4850
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 21:07:48 +00:00
Adam Langley
0da323a8b8 Convert reference counts in crypto/
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.

Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:15:26 +00:00
David Benjamin
255fa1be81 Fix EVP_PKEY_assign_DH.
Or rather fix in so far as that call will always fail now, rather than
mix up EC and DH EVP_PKEY. We don't implement EVP_PKEY_DH.

Change-Id: I752978f3440b59d963b5c13f2349284d7d799182
Reviewed-on: https://boringssl-review.googlesource.com/4567
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:28:49 +00:00
Adam Langley
517da2f1ef Add |BIO_up_ref| and |EVP_PKEY_up_ref|.
This avoids callers having to worry about |CRYPTO_add| and what the
correct lock to use it with is. (Esp since we'll probably change the way
that reference counts work in the future.)

Change-Id: I972bf0cc3be6099e0255e64a0fd50249062d1eb4
Reviewed-on: https://boringssl-review.googlesource.com/4623
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:30:03 +00:00
Brian Smith
054e682675 Eliminate unnecessary includes from low-level crypto modules.
Beyond generally eliminating unnecessary includes, eliminate as many
includes of headers that declare/define particularly error-prone
functionality like strlen, malloc, and free. crypto/err/internal.h was
added to remove the dependency on openssl/thread.h from the public
openssl/err.h header. The include of <stdlib.h> in openssl/mem.h was
retained since it defines OPENSSL_malloc and friends as macros around
the stdlib.h functions. The public x509.h, x509v3.h, and ssl.h headers
were not changed in order to minimize breakage of source compatibility
with external code.

Change-Id: I0d264b73ad0a720587774430b2ab8f8275960329
Reviewed-on: https://boringssl-review.googlesource.com/4220
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:49:18 +00:00
Adam Langley
765b66cf04 Add DSA support to EVP.
Sadly, it turns out that we have need of this, at least for now. The
code is taken from upstream and changed only as much as needed.

This only imports keys and doesn't know how to actually perform
operations on them for now.

Change-Id: I0db70fb938186cb7a91d03f068b386c59ed90b84
2015-04-06 16:58:45 -07:00
David Benjamin
87ac295d5d Remove unused fields from EVP_PKEY.
None of these are ever set.

Change-Id: If92554a4c560bf8fa2d630c419a946711db30839
Reviewed-on: https://boringssl-review.googlesource.com/4114
Reviewed-by: Adam Langley <agl@google.com>
2015-03-25 15:32:33 +00:00
David Benjamin
7cc29ab833 Take advantage of normalized return values.
Now that much of EVP has been normalized to 0/1, a lot of code can just use
boolean operators. (As can some code which was already using them...)

Change-Id: I6bb17edfd6f67050bf1706d59d8f37df57535faa
Reviewed-on: https://boringssl-review.googlesource.com/3875
Reviewed-by: Adam Langley <agl@google.com>
2015-03-13 19:40:42 +00:00
David Benjamin
6eb000dbee Add in missing curly braces part 3.
Everything else.

Change-Id: Iac02b144465b4e7b6d69ea22ff2aaf52695ae732
2015-02-11 15:14:46 -08:00
Adam Langley
2b2d66d409 Remove string.h from base.h.
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.

This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.

Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-02 19:14:15 +00:00
David Benjamin
c20febe177 Add EVP_PKEY_supports_digest.
This is intended for TLS client auth with Windows CAPI- and CNG-backed keys
which implement sign over sign_raw and do not support all hash functions. Only
plumbed through RSA for now.

Change-Id: Ica42e7fb026840f817a169da9372dda226f7d6fd
Reviewed-on: https://boringssl-review.googlesource.com/2250
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:18:36 +00:00
David Benjamin
c44d2f4cb8 Convert all zero-argument functions to '(void)'
Otherwise, in C, it becomes a K&R function declaration which doesn't actually
type-check the number of arguments.

Change-Id: I0731a9fefca46fb1c266bfb1c33d464cf451a22e
Reviewed-on: https://boringssl-review.googlesource.com/1582
Reviewed-by: Adam Langley <agl@google.com>
2014-08-21 01:06:07 +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
David Benjamin
d970488862 Typo in EVP_PKEY_asn1_find_str.
Other importing an EC PRIVATE KEY doesn't work.

Change-Id: I58b4e88d287cd7fc772a1958df5878ecff1c5b06
Reviewed-on: https://boringssl-review.googlesource.com/1090
Reviewed-by: Adam Langley <agl@google.com>
2014-07-02 21:06:57 +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