Playing around with the code, we seem to have sufficient positive test
vectors for the logic around the high bits, but not negative test
vectors. Add some. Also add a negative test vector for the trailing
byte.
(For future reference, use openssl rsautl -raw for raw RSA operations
and openssl pkeyutil for EVP_PKEY_sign.)
Change-Id: I36eddf048e51e037fd924902cd13dcb3c62bfd02
Reviewed-on: https://boringssl-review.googlesource.com/14325
Commit-Queue: David Benjamin <davidben@google.com>
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>
(Imported from upstream's 04cf39207f94abf89b3964c7710f22f829a1a78f.)
The other half of the change was fixed earlier, but this logic was still
off. This code is kind of a mess and needs a rewrite, but import the
change to get it correct and sufficiently tested first.
(If we could take the sLen = -2 case away altogether, that would be
great...)
Change-Id: I5786e980f26648822633fc216315e8f77ed4d45b
Reviewed-on: https://boringssl-review.googlesource.com/14321
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
One test case is commented out, to be fixed in a follow-up.
Change-Id: I543c7f54e63837c6e8088fdcbb03226e0144b2e5
Reviewed-on: https://boringssl-review.googlesource.com/14320
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Import test data from:
ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1/pkcs-1v2-1-vec.zip
This is a set of RSA-PSS and RSA-OAEP test vectors including some edge cases
with unusual key sizes.
(Imported from upstream's 946a515a2b370dbadb1f8c39e3586a8f1e3cff1a.)
Change-Id: I1d8aa85a8578e47b26c74bb4e4c246975619d574
Reviewed-on: https://boringssl-review.googlesource.com/14318
Commit-Queue: David Benjamin <davidben@google.com>
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>
BUG=129
Change-Id: Ie64a445a42fb3a6d16818b1fabba8481e6e9ad94
Reviewed-on: https://boringssl-review.googlesource.com/14029
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>
Within the library, we never need to exponentiate modulo an even number.
In fact, all the remaining BN_mod_exp calls are modulo an odd prime.
This extends 617804adc5 to the rest of the
library.
Change-Id: I4273439faa6a516c99673b28f8ae38ddfff7e42d
Reviewed-on: https://boringssl-review.googlesource.com/14024
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
If copy fails, we shouldn't call cleanup. Also remove some pointless
NULL checks after EVP_PKEY_up_ref.
See also upstream's 748cb9a17f4f2b77aad816cf658cd4025dc847ee.
Change-Id: I2acb6892cde1ab662ca6a620d87179f9be609cba
Reviewed-on: https://boringssl-review.googlesource.com/13830
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>
Change-Id: I81a94be94103d3c763cd6b2c1b8196300808c6fe
Reviewed-on: https://boringssl-review.googlesource.com/13386
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>
(Imported from upstream's 13ab87083af862e4af752efa4b0552149ed2cc19.)
Change-Id: I2f7cf8454d28d47f5ca19544479b2ab98143a3ef
Reviewed-on: https://boringssl-review.googlesource.com/13048
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html
Add OPENSSL_memcpy, etc., wrappers which avoid this problem.
BUG=23
Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is imported from upstream's
71bbc79b7d3b1195a7a7dd5f547d52ddce32d6f0 and test vectors taken
initially from 2d7bbd6c9fb6865e0df480602c3612652189e182 (with a handful
more added).
The tests are a little odd because OpenSSL supports this "salt length
recovery" mode and they go through that codepath for all verifications.
Change-Id: I220104fe87e2a1a1458c99656f9791d8abfbbb98
Reviewed-on: https://boringssl-review.googlesource.com/12822
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
We've taken to writing bssl::UniquePtr in full, so it's not buying
us much.
Change-Id: Ia2689366cbb17282c8063608dddcc675518ec0ca
Reviewed-on: https://boringssl-review.googlesource.com/12628
Reviewed-by: David Benjamin <davidben@google.com>
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left
everything in crypto/x509 and friends alone.
There's some instances in gcm.c that are non-trivial and pulled into a
separate CL for ease of review.
Change-Id: I6515804e3097f7e90855f1e7610868ee87117223
Reviewed-on: https://boringssl-review.googlesource.com/10801
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>
Now that we have the extern "C++" trick, we can just embed them in the
normal headers. Move the EVP_CIPHER_CTX deleter to cipher.h and, in
doing so, take away a little bit of boilerplate in defining deleters.
Change-Id: I4a4b8d0db5274a3607914d94e76a38996bd611ec
Reviewed-on: https://boringssl-review.googlesource.com/10804
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Unlike the Scoped* types, bssl::UniquePtr is available to C++ users, and
offered for a large variety of types. The 'extern "C++"' trick is used
to make the C++ bits digestible to C callers that wrap header files in
'extern "C"'.
Change-Id: Ifbca4c2997d6628e33028c7d7620c72aff0f862e
Reviewed-on: https://boringssl-review.googlesource.com/10521
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>
Change-Id: Ie60744761f5aa434a71a998f5ca98a8f8b1c25d5
Reviewed-on: https://boringssl-review.googlesource.com/10447
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>
This gets cURL building against both BoringSSL as it is and BoringSSL
with OPENSSL_VERSION_NUMBER set to 1.1.0.
BUG=91
Change-Id: I5be73b84df701fe76f3055b1239ae4704a931082
Reviewed-on: https://boringssl-review.googlesource.com/10180
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
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>
This reverts commits:
8d79ed674019fdcb52348d79ed6740
Because WebRTC (at least) includes our headers in an extern "C" block,
which precludes having any C++ in them.
Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
This change scatters the contents of the two scoped_types.h files into
the headers for each of the areas of the code. The types are now in the
|bssl| namespace.
Change-Id: I802b8de68fba4786b6a0ac1bacd11d81d5842423
Reviewed-on: https://boringssl-review.googlesource.com/8731
Reviewed-by: Adam Langley <agl@google.com>
We currently have the situation where the |tool| and |bssl_shim| code
includes scoped_types.h from crypto/test and ssl/test. That's weird and
shouldn't happen. Also, our C++ consumers might quite like to have
access to the scoped types.
Thus this change moves some of the template code to base.h and puts it
all in a |bssl| namespace to prepare for scattering these types into
their respective headers. In order that all the existing test code be
able to access these types, it's all moved into the same namespace.
Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb
Reviewed-on: https://boringssl-review.googlesource.com/8730
Reviewed-by: David Benjamin <davidben@google.com>
I named the compatibility function wrong.
Change-Id: Idc289c317c5826c338c1daf58a2d3b26b09a7e49
Reviewed-on: https://boringssl-review.googlesource.com/8301
Reviewed-by: Adam Langley <agl@google.com>
There's a __pragma expression which allows this. Android builds us Windows with
MinGW for some reason, so we actually do have to tolerate non-MSVC-compatible
Windows compilers. (Clang for Windows is much more sensible than MinGW and
intentionally mimicks MSVC.)
MinGW doesn't understand MSVC's pragmas and warns a lot. #pragma warning is
safe to suppress, so wrap those to shush them. This also lets us do away with a
few ifdefs.
Change-Id: I1f5a8bec4940d4b2d947c4c1cc9341bc15ec4972
Reviewed-on: https://boringssl-review.googlesource.com/8236
Reviewed-by: Adam Langley <agl@google.com>
More spring-cleaning of unnecessary incompatibilities. Since
OpenSSL_add_all_algorithms_conf doesn't specify a configuration file, it's
perfectly sound to have such a function.
Dear BoringSSL, please add all algorithms.
Uh, sure. They were already all there, but I have added them!
PS: Could you also load all your configuration files while you're at it.
...I don't have any. Fine. I have loaded all configuration files which I
recognize. *mutters under breath* why does everyone ask all these strange
questions...
Change-Id: I57f956933d9e519445bf22f89853bd5f56904172
Reviewed-on: https://boringssl-review.googlesource.com/8160
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
One of the codepaths didn't free the group. Found by libFuzzer.
BUG=chromium:603893
Change-Id: Icb81f2f89a8c1a52e29069321498986b193a0e56
Reviewed-on: https://boringssl-review.googlesource.com/7685
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
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>
Instead, embed the (very short) encoding of the OID into built_in_curve.
BUG=chromium:499653
Change-Id: I0db36f83c71fbd3321831f54fa5022f8304b30cd
Reviewed-on: https://boringssl-review.googlesource.com/7564
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
A lot of consumers of obj.h only want the NID values. Others didn't need
it at all. This also removes some OBJ_nid2sn and OBJ_nid2ln calls in EVP
error paths which isn't worth pulling a large table in for.
BUG=chromium:499653
Change-Id: Id6dff578f993012e35b740a13b8e4f9c2edc0744
Reviewed-on: https://boringssl-review.googlesource.com/7563
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I24d0179ca5019e82ca1494c8773f373f8c09ce82
Reviewed-on: https://boringssl-review.googlesource.com/7566
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: If76154c8d255600e925a408acdc674fc7dad0359
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7526
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I6267c9bfb66940d0b6fe5368514210a058ebd3cc
Reviewed-on: https://boringssl-review.googlesource.com/7494
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Instead of crashing when an empty key is passed to
EVP_marshal_public_key(), return with an
EVP_R_UNSUPPORTED_ALGORITHM_ERROR. This brings e.g. X509_PUBKEY_set()
closer to how it behaved before 68772b31 (previously, it returned an
error on an empty public key rather than dereferencing pkey->ameth).
Change-Id: Ieac368725adb7f22329c035d9d0685b44b885888
Reviewed-on: https://boringssl-review.googlesource.com/7351
Reviewed-by: David Benjamin <davidben@google.com>
If d2i_PrivateKey hit the PKCS#8 codepath, it didn't enforce that the key was
of the specified type.
Note that this requires tweaking d2i_AutoPrivateKey slightly. A PKCS #8
PrivateKeyInfo may have 3 or 4 elements (optional attributes), so we were
relying on this bug for d2i_AutoPrivateKey to work.
Change-Id: If50b7a742f535d208e944ba37c3a585689d1da43
Reviewed-on: https://boringssl-review.googlesource.com/7253
Reviewed-by: Adam Langley <agl@google.com>
It's not used anywhere else, in the library or consumers (Google ones or
ones I could find on Debian codesearch). This is a sufficiently
specialized function that the risk of a third-party library newly
depending on it is low. This removes the last include of asn1.h or
x509.h in crypto/evp.
(This is almost entirely cosmetic because it wasn't keeping the static linker
from doing the right thing anyway. But if we were want to separate the legacy
ASN.1 stack into its own decrepit-like target, we'll need to be pickier about
separation.)
Change-Id: I9be97c9321572e3a2ed093e1d50036b7654cff41
Reviewed-on: https://boringssl-review.googlesource.com/7080
Reviewed-by: Adam Langley <agl@google.com>
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>
This is kind of a ridiculous function. It would be nice to lose it, but
SSL_use_PrivateKey_file actually calls into it (by way of
d2i_PrivateKey_bio).
BUG=499653
Change-Id: I83634f6982b15f4b877e29f6793b7e00a1c10450
Reviewed-on: https://boringssl-review.googlesource.com/7026
Reviewed-by: Adam Langley <agl@google.com>
EVP_PKEY_asn1_find can already be private. EVP_PKEY_asn1_find_str is used
only so the PEM code can get at legacy encoders. Since this is all
legacy non-PKCS8 stuff, we can just explicitly list out the three cases
in the two places that need it. If this changes, we can later add a
table in crypto/pem mapping string to EVP_PKEY type.
With this, EVP_PKEY_ASN1_METHOD is no longer exposed in the public API
and nothing outside of EVP_PKEY reaches into it. Unexport all of that.
Change-Id: Iab661014247dbdbc31e5e9887364176ec5ad2a6d
Reviewed-on: https://boringssl-review.googlesource.com/6871
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
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>
This is in preparation for moving the logic itself to crypto/x509, so
the lower-level functions will not be as readily available.
Change-Id: I6507b895317df831ab11d0588c5b09bbb2aa2c24
Reviewed-on: https://boringssl-review.googlesource.com/7023
Reviewed-by: Adam Langley <agl@google.com>
It's only used by crypto/x509, and we don't even support DSA in
crypto/x509 anymore since the EVP_PKEY_CTX hooks aren't wired up.
Change-Id: I1b8538353eb51df353cf9171b1cbb0bb47a879a3
Reviewed-on: https://boringssl-review.googlesource.com/7024
Reviewed-by: Adam Langley <agl@google.com>
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>
Previously, OpenSSL supported many different DSA PKCS#8 encodings. Only
support the standard format. One of the workaround formats (SEQUENCE of
private key and public key) seems to be a workaround for an old Netscape
bug. From inspection, NSS seems to have fixed this from the first open
source commit.
Change-Id: I1e097b675145954b4d7a0bed8733e5a25c25fd8e
Reviewed-on: https://boringssl-review.googlesource.com/7074
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
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>
