Commit Graph

41 Commits

Author SHA1 Message Date
David Benjamin
3f5d13812a Remove EVP_set_buggy_rsa_parser stub.
Change-Id: I848c79274119e73e39456c75231c8e3f6047fde2
Reviewed-on: https://boringssl-review.googlesource.com/22264
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-27 13:49:57 +00:00
David Benjamin
a37f286f4e Remove the buggy RSA parser.
I've left EVP_set_buggy_rsa_parser as a no-op stub for now, but it
shouldn't need to last very long. (Just waiting for a CL to land in a
consumer.)

Bug: chromium:735616
Change-Id: I6426588f84dd0803661a79c6636a0414f4e98855
Reviewed-on: https://boringssl-review.googlesource.com/22124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-10-24 17:39:46 +00:00
David Benjamin
246e27d807 Switch the buggy RSA parser off by default.
I'll fully remove this once Chrome 62 hits stable, in case any bug
reports come in for Chrome 61. Meanwhile switch the default to off so
that other consumers pick up the behavior. (Should have done this sooner
and forgot.)

Bug: chromium:735616
Change-Id: Ib27c4072f228cd3b5cce283accd22732eeef46b2
Reviewed-on: https://boringssl-review.googlesource.com/20484
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-18 19:42:51 +00:00
David Benjamin
808f832917 Run the comment converter on libcrypto.
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.

Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-18 21:49:04 +00:00
David Benjamin
1d59f6e3e2 Add a flag to toggle the buggy RSA parser.
It's about time we got rid of this. As a first step, introduce a flag,
so that some consumers may stage this change in appropriately.

BUG=chromium:534766,chromium:532048

Change-Id: Id53f0bacf5bdbf85dd71d1262d9f3a9ce3c4111f
Reviewed-on: https://boringssl-review.googlesource.com/16104
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-05-10 15:49:33 +00:00
Adam Langley
96dec443d9 Move rsa/ to fipsmodule/rsa/
Change-Id: Id20d371ae7a88a91aaba7a9e23574eccb9caeb3c
Reviewed-on: https://boringssl-review.googlesource.com/15849
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-05-04 21:22:39 +00:00
David Benjamin
17eeb9820c Unwind the rest of EVP_PKEY_supports_digest.
This is a remnant of a previous iteration of the SSL client certificate
bridging logic in Chromium.

Change-Id: Ifa8e15cc970395f179e2f6db65c97a342af5498d
Reviewed-on: https://boringssl-review.googlesource.com/14444
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-30 16:28:25 +00:00
David Benjamin
046b27815e Decouple crypto/evp from the OID table.
BUG=chromium:499653

Change-Id: I4e8d4af3129dbf61d4a8846ec9db685e83999d5e
Reviewed-on: https://boringssl-review.googlesource.com/7565
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 22:12:46 +00:00
David Benjamin
921d906bb6 Reimplement d2i_PrivateKey.
Functions which lose object reuse and need auditing:
- d2i_PrivateKey

This removes evp_asn1.c's dependency on the old stack. (Aside from
obj/.) It also takes old_priv_decode out of EVP_ASN1_METHOD in favor of
calling out to the new-style function. EVP_ASN1_METHOD no longer has any
old-style type-specific serialization hooks, only the PKCS#8 and SPKI
ones.

BUG=499653

Change-Id: Ic142dc05a5505b50e4717c260d3893b20e680194
Reviewed-on: https://boringssl-review.googlesource.com/7027
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 23:33:50 +00:00
David Benjamin
3f4f7ee08f PEM_write_bio_PrivateKey is always PKCS#8.
Every key type which has a legacy PEM encoding also has a PKCS#8
encoding. The fallback codepath is never reached.

This removes the only consumer of pem_str, so that may be removed from
EVP_PKEY_ASN1_METHOD.

Change-Id: Ic680bfc162e1dc76db8b8016f6c10f669b24f5aa
Reviewed-on: https://boringssl-review.googlesource.com/6870
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 22:41:17 +00:00
David Benjamin
8c07ad3e3b Pull EVP_PKEY print hooks out of the main method table.
This allows the static linker to drop it in consumers which don't need this
stuff (i.e. all sane ones), once crypto/x509 falls off. This cuts down
on a number of dependencies from the core crypto bits on crypto/asn1 and
crypto/x509.

BUG=499653

Change-Id: I76a10a04dcc444c1ded31683df9f87725a95a4e6
Reviewed-on: https://boringssl-review.googlesource.com/5660
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 22:40:44 +00:00
David Benjamin
17727c6843 Move all signature algorithm code to crypto/x509.
All the signature algorithm logic depends on X509_ALGOR. This also
removes the X509_ALGOR-based EVP functions which are no longer used
externally. I think those APIs were a mistake on my part. The use in
Chromium was unnecessary (and has since been removed anyway). The new
X.509 stack will want to process the signatureAlgorithm itself to be
able to enforce policies on it.

This also moves the RSA_PSS_PARAMS bits to crypto/x509 from crypto/rsa.
That struct is also tied to crypto/x509. Any new RSA-PSS code would
have to use something else anyway.

BUG=499653

Change-Id: I6c4b4573b2800a2e0f863d35df94d048864b7c41
Reviewed-on: https://boringssl-review.googlesource.com/7025
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 22:39:02 +00:00
David Benjamin
e30a09e604 Implement new PKCS#8 parsers.
As with SPKI parsers, the intent is make EVP_PKEY capture the key's
constraints in full fidelity, so we'd have to add new types or store the
information in the underlying key object if people introduce variant key
types with weird constraints on them.

Note that because PKCS#8 has a space for arbitrary attributes, this
parser must admit a hole. I'm assuming for now that we don't need an API
that enforces no attributes and just ignore trailing data in the
structure for simplicity.

BUG=499653

Change-Id: I6fc641355e87136c7220f5d7693566d1144a68e8
Reviewed-on: https://boringssl-review.googlesource.com/6866
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 17:24:10 +00:00
David Benjamin
239a0abfd5 Slightly simplify and deprecate i2d_{Public,Private}Key.
There are all the type-specific serializations rather than something
tagged with a type. i2d_PrivateKey's PKCS#8 codepath was unreachable
because every EVP_PKEY type has an old_priv_encode function.

To prune EVP_PKEY_ASN1_METHOD further, replace i2d_PrivateKey into a
switch case so we don't need to keep old_priv_encode around. This cuts
down on a case of outside modules reaching into crypto/evp method
tables.

Change-Id: I30db2eed836d560056ba9d1425b960d0602c3cf2
Reviewed-on: https://boringssl-review.googlesource.com/6865
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:31:26 +00:00
David Benjamin
32fdc512ca Remove param_decode and param_encode EVP_PKEY hooks.
They're only used by a pair of PEM functions, which are never used.

BUG=499653

Change-Id: I89731485c66ca328c634efbdb7e182a917f2a963
Reviewed-on: https://boringssl-review.googlesource.com/6863
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:30:29 +00:00
David Benjamin
68772b31b0 Implement new SPKI parsers.
Many consumers need SPKI support (X.509, TLS, QUIC, WebCrypto), each
with different ways to set signature parameters. SPKIs themselves can
get complex with id-RSASSA-PSS keys which come with various constraints
in the key parameters. This suggests we want a common in-library
representation of an SPKI.

This adds two new functions EVP_parse_public_key and
EVP_marshal_public_key which converts EVP_PKEY to and from SPKI and
implements X509_PUBKEY functions with them. EVP_PKEY seems to have been
intended to be able to express the supported SPKI types with
full-fidelity, so these APIs will continue this.

This means future support for id-RSASSA-PSS would *not* repurpose
EVP_PKEY_RSA. I'm worried about code assuming EVP_PKEY_RSA implies
acting on the RSA* is legal. Instead, it'd add an EVP_PKEY_RSA_PSS and
the data pointer would be some (exposed, so the caller may still check
key size, etc.) RSA_PSS_KEY struct. Internally, the EVP_PKEY_CTX
implementation would enforce the key constraints. If RSA_PSS_KEY would
later need its own API, that code would move there, but that seems
unlikely.

Ideally we'd have a 1:1 correspondence with key OID, although we may
have to fudge things if mistakes happen in standardization. (Whether or
not X.509 reuses id-ecPublicKey for Ed25519, we'll give it a separate
EVP_PKEY type.)

DSA parsing hooks are still implemented, missing parameters and all for
now. This isn't any worse than before.

Decoupling from the giant crypto/obj OID table will be a later task.

BUG=522228

Change-Id: I0e3964edf20cb795a18b0991d17e5ca8bce3e28c
Reviewed-on: https://boringssl-review.googlesource.com/6861
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:28:07 +00:00
Brian Smith
5ba06897be Don't cast |OPENSSL_malloc|/|OPENSSL_realloc| result.
C has implicit conversion of |void *| to other pointer types so these
casts are unnecessary. Clean them up to make the code easier to read
and to make it easier to find dangerous casts.

Change-Id: I26988a672e8ed4d69c75cfbb284413999b475464
Reviewed-on: https://boringssl-review.googlesource.com/7102
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-11 22:07:56 +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
28243c08db Add PSS parameter check.
Avoid seg fault by checking mgf1 parameter is not NULL. This can be
triggered during certificate verification so could be a DoS attack
against a client or a server enabling client authentication.

Thanks to Loïc Jonas Etienne (Qnective AG) for discovering this bug.

CVE-2015-3194

(Imported from upstream's c394a488942387246653833359a5c94b5832674e and test
data from 00456fded43eadd4bb94bf675ae4ea5d158a764f.)

Change-Id: Ic97059d42722fd810973ccb0c26c415c4eaae79a
Reviewed-on: https://boringssl-review.googlesource.com/6617
Reviewed-by: Adam Langley <agl@google.com>
2015-12-03 16:47:12 +00:00
David Benjamin
4c60d356a9 Work around even more Estonian ID card misissuances.
Not content with signing negative RSA moduli, still other Estonian IDs have too
many leading zeros. Work around those too.

This workaround will be removed in six months.

BUG=534766

Change-Id: Ica23b1b1499f9dbe39e94cf7b540900860e8e135
Reviewed-on: https://boringssl-review.googlesource.com/5980
Reviewed-by: Adam Langley <agl@google.com>
2015-09-23 20:35:47 +00:00
David Benjamin
c71567dd50 Update the Estonian workaround comments.
Target date for removal of the workaround is 6 months.

BUG=532048

Change-Id: I402f75e46736936725575559cd8eb194115ab0df
Reviewed-on: https://boringssl-review.googlesource.com/5910
Reviewed-by: Adam Langley <agl@google.com>
2015-09-18 18:42:34 +00:00
David Benjamin
231cb82145 Work around broken Estonian smart cards. Again.
Estonian IDs issued between September 2014 to September 2015 are broken and use
negative moduli. They last five years and are common enough that we need to
work around this bug.

Add parallel "buggy" versions of BN_cbs2unsigned and RSA_parse_public_key which
tolerate this mistake, to align with OpenSSL's previous behavior. This code is
currently hooked up to rsa_pub_decode in RSA_ASN1_METHOD so that d2i_X509 is
tolerant. (This isn't a huge deal as the rest of that stack still uses the
legacy ASN.1 code which is overly lenient in many other ways.)

In future, when Chromium isn't using crypto/x509 and has more unified
certificate handling code, we can put client certificates under a slightly
different codepath, so this needn't hold for all certificates forever. Then in
September 2019, when the broken Estonian certificates all expire, we can purge
this codepath altogether.

BUG=532048

Change-Id: Iadb245048c71dba2eec45dd066c4a6e077140751
Reviewed-on: https://boringssl-review.googlesource.com/5894
Reviewed-by: Adam Langley <agl@google.com>
2015-09-15 21:18:15 +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
c0e245a546 Parse RSAPublicKey with CBS.
BUG=499653

Change-Id: If5d98ed23e65a84f9f0e303024f91cce078f3d18
Reviewed-on: https://boringssl-review.googlesource.com/5272
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 22:39:28 +00:00
David Benjamin
2e40091dd4 PKCS8_pkey_set0 doesn't take ownership on error.
It carefully NULLs all references to the buffer, so the failure cases
still need to call OPENSSL_free.

Change-Id: Ia14341ecea81296f94e467325ab6eff1362e987e
Reviewed-on: https://boringssl-review.googlesource.com/5271
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 02:30:37 +00:00
David Benjamin
42ae3adcf6 Remove info field on EVP_PKEY_ASN1_METHOD.
Nothing ever reads it.

Change-Id: Id375c461aa2feb6877a14e19eb2daefec7a03f89
Reviewed-on: https://boringssl-review.googlesource.com/5345
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:46 +00:00
Adam Langley
839b881c61 Multi-prime RSA support.
RSA with more than two primes is specified in
https://tools.ietf.org/html/rfc3447, although the idea goes back far
earier than that.

This change ports some of the changes in
http://rt.openssl.org/Ticket/Display.html?id=3477&user=guest&pass=guest
to BoringSSL—specifically those bits that are under an OpenSSL license.

Change-Id: I51e8e345e2148702b8ce12e00518f6ef4683d3e1
Reviewed-on: https://boringssl-review.googlesource.com/4870
Reviewed-by: Adam Langley <agl@google.com>
2015-06-05 18:39:44 +00:00
David Benjamin
cca4ba7611 Remove unnecessary NULL checks, part 3.
Finish up the e's.

Change-Id: Iabb8da000fbca6efee541edb469b90896f60d54b
Reviewed-on: https://boringssl-review.googlesource.com/4516
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:12:04 +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
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
Adam Langley
0e7f89f96c Remove pkey_ctrl.
It only included ASN1_PKEY_CTRL_DEFAULT_MD_NID and that's unused in
BoringSSL.

Change-Id: Idfcbd0f26f6448ce307c53ddef334f2e63c85a64
2014-11-10 13:45:32 -08:00
David Benjamin
8cfd8ad144 Remove unused EVP_PKEY_ASN1_METHODs and flags.
The only alias, EVP_PKEY_RSA2, is handled programmatically. ASN1_PKEY_ALIAS and
ASN1_PKEY_DYNAMIC are then unused and unexported and can be removed.

Change-Id: I990650636bac3b802c8b439257c67ce7a3f8bc70
Reviewed-on: https://boringssl-review.googlesource.com/2124
Reviewed-by: Adam Langley <agl@google.com>
2014-11-04 00:28:09 +00:00
David Benjamin
0248889950 Don't mix and match libraries and errors.
The same library code applies for both the error and the function, so modules
cannot easily report errors from each other. Switch evp/algorithm.c's error
codes to the EVP library. Remove the original error codes so it's obvious some
changes are needed.

- X509_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED -> EVP_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED
  ASN1_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED -> EVP_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED
  (Actually, the X509 version of this error code doesn't exist in OpenSSL. It should
   have been ASN1.)

- ASN1_R_UNKNOWN_SIGNATURE_ALGORITHM -> EVP_R_UNKNOWN_SIGNATURE_ALGORITHM

- ASN1_R_WRONG_PUBLIC_KEY_TYPE -> EVP_R_WRONG_PUBLIC_KEY_TYPE

- ASN1_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM -> EVP_R_UNKNOWN_MESSAGE_DIGEST_ALGORITHM

Change-Id: I05b1a05b465d800c85f7d63ca74588edf40847b9
Reviewed-on: https://boringssl-review.googlesource.com/1940
Reviewed-by: Adam Langley <agl@google.com>
2014-10-13 22:56:28 +00:00
David Benjamin
8f160a680b Add EVP_DigestVerifyInitFromAlgorithm and EVP_DigestSignAlgorithm.
Factor the AlgorithmIdentifier portions of ASN1_item_sign and ASN1_item_verify
out. This makes it possible to initialize a signature context from an
AlgorithmIdentifier without needing the data parsed into an ASN1_ITEM/void*
pair and reserialized.

Change-Id: Idc2e06b1310a3f801aa25de323d39d2b7a44ef50
Reviewed-on: https://boringssl-review.googlesource.com/1916
Reviewed-by: Adam Langley <agl@google.com>
2014-10-09 21:52:52 +00:00
David Benjamin
75a1dd0dbe Remove EVP_PKEY_CTX case in rsa_pss_to_ctx.
We only ever use the EVP_PKEY case, not the EVP_PKEY_CTX one.

Change-Id: Ibead854f793663da0a9e474599507d9c3ff920cb
Reviewed-on: https://boringssl-review.googlesource.com/1915
Reviewed-by: Adam Langley <agl@google.com>
2014-10-09 21:07:32 +00:00
David Benjamin
f4b4952719 Fix rsa_pss_to_ctx return value.
It should return 0 for failure, not -1; the call site was expecting 0 anyway.

Change-Id: I24ab5d3695b8ac438e40be1a4fd74ecd3b845f5a
Reviewed-on: https://boringssl-review.googlesource.com/1914
Reviewed-by: Adam Langley <agl@google.com>
2014-10-09 21:04:36 +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
467c87e318 remove duplicate 0x for default RSASSA-PSS salt len
(Imported from upstream's 42d73874edb4eb6681b769d9850afebe97adf329)
2014-06-20 13:17:40 -07: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