Commit Graph

73 Commits

Author SHA1 Message Date
David Benjamin
02084ea398 Decouple PKCS8_encrypt and PKCS8_decrypt's core from crypto/asn1.
These will be used by Chromium's crypto::ECPrivateKey to work with
EncryptedPrivateKeyInfo structures.

Note this comes with a behavior change: PKCS8_encrypt and PKCS8_decrypt
will no longer preserve PKCS#8 PrivateKeyInfo attributes. However, those
functions are only called by Chromium which does not care. They are also
called by the PEM code, but not in a way which exposes attributes.

The PKCS#12 PFX code is made to use PKCS8_parse_encrypted_private_key
because it's cleaner (no more tossing X509_SIG around) and to ease
decoupling that in the future.

crypto/pkcs8's dependency on the legacy ASN.1 stack is now limited to
pkcs8_x509.c.

BUG=54

Change-Id: I173e605d175e982c6b0250dd22187b73aca15b1a
Reviewed-on: https://boringssl-review.googlesource.com/14215
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>
2017-03-26 04:00:26 +00:00
David Benjamin
7ce10d5da7 Partially split out crypto/pkcs8's legacy ASN.1 dependencies.
PKCS8_encrypt and PKCS8_decrypt still need to be split. The code for
processing PKCS#12 files is, for now, placed entirely in pkcs8_x509.c.
If we need to split it up, it should be straightforward to do so.
(Introduce a CRYPTO_BUFFER version of PKCS12_get_key_and_certs and go
from there.)

BUG=54

Change-Id: I9c87e916ec29ee14dbbd81c4d3fc10ac8a461f1a
Reviewed-on: https://boringssl-review.googlesource.com/14214
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-03-26 00:17:25 +00:00
David Benjamin
8cd7bbf514 Push password encoding back into pkcs12_key_gen.
With PKCS8_encrypt_pbe and PKCS8_decrypt_pbe gone in
3e8b782c0c, we can restore the old
arrangement where the password encoding was handled in pkcs12_key_gen.
This simplifies the interface for the follow-up crypto/asn1 split.

Note this change is *not* a no-op for PKCS#12 files which use PBES2.
Before, we would perform the PKCS#12 password encoding for all parts of
PKCS#12 processing. The new behavior is we only perform it for the parts
that go through the PKCS#12 KDF. For such a file, it would only be the
MAC.

I believe the specification supports our new behavior. Although RFC 7292
B.1 says something which implies that the transformation is about
converting passwords to byte strings and would thus be universal,
appendix B itself is prefaced with:

   Note that this method for password privacy mode is not recommended
   and is deprecated for new usage.  The procedures and algorithms
   defined in PKCS #5 v2.1 [13] [22] should be used instead.
   Specifically, PBES2 should be used as encryption scheme, with PBKDF2
   as the key derivation function.

"This method" refers to the key derivation and not the password
formatting, but it does give support to the theory that password
formatting is tied to PKCS#12 key derivation.

(Of course, if one believes PKCS#12's assertion that their inane
encoding (NUL-terminated UTF-16!) is because PKCS#5 failed to talk about
passwords as Unicode strings, one would think that PBES2 (also in
PKCS#5) would have the same issue and thus need PKCS#12 to valiantly
save the day with an encoding...)

This matches OpenSSL's behavior and that of recent versions of NSS. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1268141. I was unable to
figure out what variants, if any, macOS accepts.

BUG=54

Change-Id: I9a1bb4d5e168e6e76b82241e4634b1103e620b9b
Reviewed-on: https://boringssl-review.googlesource.com/14213
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>
2017-03-25 21:25:30 +00:00
David Benjamin
3cb047e56c Decouple PKCS#12 hash lookup from the OID table.
This isn't strictly necessary for Chromium yet, but we already have a
decoupled version of hash algorithm parsing available. For now, don't
export it but eventually we may wish to use it for OCSP.

BUG=54

Change-Id: If460d38d48bd47a2b4a853779f210c0cf7ee236b
Reviewed-on: https://boringssl-review.googlesource.com/14211
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-03-25 21:22:50 +00:00
David Benjamin
f35e8384a8 Fix parsing of PBKDF2 parameters.
The OPTIONAL prf field is an AlgorithmIdentifier, not an OID.  I messed
this up in the recent rewrite.

Fix the parsing and add a test, produced by commenting out the logic in
OpenSSL to omit the field for hmacWithSHA1. (We don't currently support
any other PBKDF2, or I'd just add a test for that.)

Change-Id: I7d258bb01b93cd203a6fc1b8cccbddfdbc4dbbad
Reviewed-on: https://boringssl-review.googlesource.com/14330
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>
2017-03-25 16:29:06 +00:00
David Benjamin
1d4fa785bc Decouple PBE lookup from the OID table.
BUG=54

Change-Id: Ia792dadcbda4efb22b45ae69a6e425ae2b341f61
Reviewed-on: https://boringssl-review.googlesource.com/14210
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>
2017-03-25 16:28:44 +00:00
David Benjamin
96e744c176 Decouple PKCS#5 cipher lookup from OID table.
We still need to expose a suitable API for Chromium to consume, but the
core implementation itself should now be ready.

The supported cipher list is based on what EVP_get_cipherbynid currently
supports, excluding the entries which don't have OIDs.

BUG=54

Change-Id: I3befca0a34b330ec1f663a029a8fbf049a4406bd
Reviewed-on: https://boringssl-review.googlesource.com/14212
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>
2017-03-22 18:30:24 +00:00
David Benjamin
d851842228 Reduce crypto/pkcs8 dependency on OID table.
To remove the OID table from Chromium, we'll need to decouple a lot of
this code. In preparation for that, detach the easy cases from the OID
table. What remains is PBES, cipher, and digest OIDs which will be doing
in follow-up changes.

BUG=54

Change-Id: Ie205d23d042e21114ca1faf68917fdc870969d09
Reviewed-on: https://boringssl-review.googlesource.com/14209
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>
2017-03-21 21:10:00 +00:00
David Benjamin
3e8b782c0c Remove "raw" versions of PKCS8_encrypt and PKCS8_decrypt.
These were added in an attempt to deal with the empty vs. NULL confusion
in PKCS#12. Instead, PKCS8_encrypt and PKCS8_decrypt already treated
NULL special. Since we're stuck with supporting APIs like those anyway,
Chromium has been converted to use that feature. This cuts down on the
number of APIs we need to decouple from crypto/asn1.

BUG=54

Change-Id: Ie2d4798d326c5171ea5d731da0a2c11278bc0241
Reviewed-on: https://boringssl-review.googlesource.com/13885
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>
2017-02-16 23:04:15 +00:00
David Benjamin
8b8d22c961 Parse PKCS#12 files more accurately.
Mercifully, PKCS#12 does not actually make ContentInfo and SafeBag
mutually recursive. The top-level object in a PKCS#12 is a SEQUENCE of
data or encrypted data ContentInfos. Their payloads are a SEQUENCE of
SafeBags (aka SafeContents).

SafeBag is a similar structure to ContentInfo but not identical (it has
attributes in it which we ignore) and actually carries the objects.
There is only recursion if the SafeContents bag type is used, which we
do not process.

This means we don't need to manage recursion depth. This also no longer
allows trailing data after the SEQUENCE and removes the comment about
NSS. The test file still passes, so I'm guessing something else was
going on?

Change-Id: I68e2f8a5cc4b339597429d15dc3588bd39267e0a
Reviewed-on: https://boringssl-review.googlesource.com/13071
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 16:56:05 +00:00
David Benjamin
7f539fa008 Handle overflow in ascii_to_ucs2.
Change-Id: Ie9a0039931a1a8d48a82c11ef5c58d6ee084ca4c
Reviewed-on: https://boringssl-review.googlesource.com/13070
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 01:27:18 +00:00
David Benjamin
9d0e7fb6e7 Rework PKCS{5,8,12} code.
Avoid the X509_ALGOR dependency entirely. The public API is still using
the legacy ASN.1 structures for now, but the conversions are lifted to
the API boundary. Once we resolve that and the OID table dependency,
this module will no longer block unshipping crypto/asn1 and friends from
Chromium.

This changes the calling convention around the two kinds of PBE suites
we support. Each PBE suite provides a free-form encrypt_init function to
setup an EVP_CIPHER_CTX and write the AlgorithmIdentifer to a CBB. It
then provides a common decrypt_init function which sets up an
EVP_CIPHER_CTX given a CBS of the parameter. The common encrypt code
determines how to call which encrypt_init function. The common decrypt
code parses the OID out of the AlgorithmIdentifer and then dispatches to
decrypt_init.

Note this means the encryption codepath no longer involves parsing back
out a AlgorithmIdentifier it just serialized. We don't have a good story
to access an already serialized piece of a CBB in progress (reallocs can
invalidate the pointer in a CBS), so it's easier to cut this step out
entirely.

Also note this renames the "PBES1" schemes from PKCS#5 to PKCS#12. This
makes it easier to get at the PKCS#12 key derivation hooks. Although
PKCS#12 claims these are variants of PKCS#5's PBES1, they're not very
related. PKCS#12 swaps out the key derivation and even defines its own
AlgorithmIdentifier parameter structure (identical to the PKCS#5 PBES1
one). The only thing of PBES1 that survives is the CBC mode padding
scheme, which is deep in EVP_CIPHER for us. (Of course, all this musing
on layering is moot because we don't implement non-PKCS#12 PBES1 schemes
anyway.)

This also moves some of the random API features (default iteration
count, default salt generation) out of the PBE suites and into the
common code.

BUG=54

Change-Id: Ie96924c73a229be2915be98eab680cadd17326db
Reviewed-on: https://boringssl-review.googlesource.com/13069
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 01:25:14 +00:00
David Benjamin
314d81420c Reimplement pkcs12_pbe_keyivgen with CBS.
BUG=54

Change-Id: Ie003a9635b33ad6f7e430684f0eb6975c613ebf3
Reviewed-on: https://boringssl-review.googlesource.com/13068
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 00:54:08 +00:00
David Benjamin
d1afc41869 Reimplement PKCS5_pbe_set with CBB.
BUG=54

Change-Id: I41bd43948140037c8e5c1b6502e1c882293befec
Reviewed-on: https://boringssl-review.googlesource.com/13067
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 00:51:52 +00:00
Adam Langley
2a25aae0f5 Ensure that CBB is |CBB_zero|ed before possibly calling |CBB_cleanup|.
Change-Id: Ic1f58f87c67104c8a51af59086a1bb1e5ccb0e5b
Reviewed-on: https://boringssl-review.googlesource.com/13084
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-11 00:49:37 +00:00
David Benjamin
4fae069c00 Reimplement PKCS5_v2_PBE_keyivgen.
This gets us closer to decoupling from crypto/asn1.

BUG=54

Change-Id: I06ec04ed3cb47c2f56a94c6defa97398bfd0e013
Reviewed-on: https://boringssl-review.googlesource.com/13066
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 00:37:56 +00:00
David Benjamin
e464e81f89 Reimplement PKCS5_pbe2_set with CBB.
This is not quite an end state (it still outputs an X509_ALGOR, the way
the generated salt is fed into key derivation is odd, and it uses the
giant OID table), but replaces a large chunk of it.

BUG=54

Change-Id: I0a0cca13e44e6a09dfaf6aed3b357cb077dc46d1
Reviewed-on: https://boringssl-review.googlesource.com/13065
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-11 00:34:25 +00:00
David Benjamin
ac83bea85d Trim dead code from PKCS#5 PBE2 bits.
Many of these parameters are constants.

Change-Id: I148dbea0063e478a132253f4e9dc71d5d20320c2
Reviewed-on: https://boringssl-review.googlesource.com/13064
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-11 00:13:59 +00:00
David Benjamin
9ba19b8e88 Test we can round-trip PKCS8_{encrypt,decrypt}.
This is a very basic test, but it's something.

Change-Id: Ic044297e97ce5719673869113ce581de4621ebbd
Reviewed-on: https://boringssl-review.googlesource.com/13061
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:49:37 +00:00
David Benjamin
8f3f6be0d5 Const-correct the PKCS8 salt parameter.
Change-Id: Iad9b0898b3a602fc2e554c4fd59a599c61cd8ef7
Reviewed-on: https://boringssl-review.googlesource.com/13063
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:42:10 +00:00
David Benjamin
35349e9fac Unexport PKCS5 functions.
They're not called externally. Unexporting these will make it easier to
rewrite the PKCS{5,8,12} code to use CBS/CBB rather than X509_ALGOR.
Getting rid of those callers in Chromium probably won't happen for a
while since it's in our on-disk formats. (And a unit test for some NSS
client cert glue uses it.)

BUG=54

Change-Id: Id4148a2ad567484782a6e0322b68dde0619159fc
Reviewed-on: https://boringssl-review.googlesource.com/13062
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:41:44 +00:00
David Benjamin
20dbc1ff20 Import some PKCS8_decrypt test vectors from Chromium.
This includes examples with both the NULL and empty passwords, thanks to
PKCS#12's password ambiguity.

Change-Id: Iae31840c1d31929fa9ac231509acaa80ef5b74bb
Reviewed-on: https://boringssl-review.googlesource.com/13060
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:40:54 +00:00
David Benjamin
17cf2cb1d2 Work around language and compiler bug in memcpy, etc.
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>
2016-12-21 20:34:47 +00:00
David Benjamin
54091230cd Use C99 for size_t loops.
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>
2016-09-12 19:44:24 +00:00
Alessandro Ghedini
1fc7e9ccd2 Remove trailing ';' from macros
For consistency and to avoid a pedantic GCC warning (even though it's
mostly old legacy code).

Change-Id: Iea63eb0a82ff52914adc33b83e48450f4f6a49ef
Reviewed-on: https://boringssl-review.googlesource.com/11021
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>
2016-09-12 19:17:26 +00:00
Matt Braithwaite
d17d74d73f Replace Scoped* heap types with bssl::UniquePtr.
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>
2016-09-01 22:22:54 +00:00
Steven Valdez
cb96654404 Adding ARRAY_SIZE macro for getting the size of constant arrays.
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>
2016-08-19 19:30:39 +00:00
Adam Langley
10f97f3bfc Revert "Move C++ helpers into |bssl| namespace."
This reverts commit 09feb0f3d9.

(In order to make WebRTC happy this also needs to be reverted.)
2016-07-12 08:09:33 -07:00
Adam Langley
d2b5af56cf Revert scoped_types.h change.
This reverts commits:
8d79ed6740
19fdcb5234
8d79ed6740

Because WebRTC (at least) includes our headers in an extern "C" block,
which precludes having any C++ in them.

Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
2016-07-12 08:05:38 -07:00
Adam Langley
8c3c3135a2 Remove scoped_types.h.
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>
2016-07-11 23:08:27 +00:00
Adam Langley
09feb0f3d9 Move C++ helpers into |bssl| namespace.
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>
2016-07-11 23:04:52 +00:00
David Benjamin
1fc7564ba7 Add standalone PKCS#8 and SPKI fuzzers.
We already had coverage for our new EVP_PKEY parsers, but it's good to have
some that cover them directly. The initial corpus was generated manually with
der-ascii and should cover most of the insanity around EC key serialization.

BUG=15

Change-Id: I7aaf56876680bfd5a89f5e365c5052eee03ba862
Reviewed-on: https://boringssl-review.googlesource.com/7728
Reviewed-by: Adam Langley <agl@google.com>
2016-04-25 21:57:28 +00:00
David Benjamin
582d2847ed Reimplement PKCS#12 key derivation.
This is avoids pulling in BIGNUM for doing a straight-forward addition on a
block-sized value, and avoids a ton of mallocs. It's also -Wconversion-clean,
unlike the old one.

In doing so, this replaces the HMAC_MAX_MD_CBLOCK with EVP_MAX_MD_BLOCK_SIZE.
By having the maximum block size available, most of the temporary values in the
key derivation don't need to be malloc'd.

BUG=22

Change-Id: I940a62bba4ea32bf82b1190098f3bf185d4cc7fe
Reviewed-on: https://boringssl-review.googlesource.com/7688
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-19 18:16:38 +00:00
David Benjamin
981936791e Remove some easy obj.h dependencies.
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>
2016-03-31 20:50:33 +00:00
Steven Valdez
d8eea14443 BIO_new_mem_buf should take const void *
BIO_FLAGS_MEM_RDONLY keeps the invariant.

(Imported from upstream's a38a159bfcbc94214dda00e0e6b1fc6454a23b78)

Change-Id: I4cb35615d76b77929915e370dbb7fec1455da069
Reviewed-on: https://boringssl-review.googlesource.com/7214
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-24 19:14:19 +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
375124b162 Parse BER for PKCS#12 more accurately.
CBS_asn1_ber_to_der currently uses heuristics because implicitly-tagged
constructed strings in BER are ambiguous with implicitly-tagged sequences. It's
not possible to convert BER to DER without knowing the schema.

Fortunately, implicitly tagged strings don't appear often so instead split the
job up: CBS_asn1_ber_to_der fixes indefinite-length elements and constructed
strings it can see. Implicitly-tagged strings it leaves uncoverted, but they
will only nest one level down (because BER kindly allows one to nest
constructed strings arbitrarily!).

CBS_get_asn1_implicit_string then performs the final concatenation at parse
time. This isn't much more complex and lets us parse BER more accurately and
also reject a number of mis-encoded values (e.g. constructed INTEGERs are not a
thing) we'd previously let through. The downside is the post-conversion parsing
code must be aware of this limitation of CBS_asn1_ber_to_der. Fortunately,
there's only one implicitly-tagged string in our PKCS#12 code.

(In the category of things that really really don't matter, but I had spare
cycles and the old BER converter is weird.)

Change-Id: Iebdd13b08559fa158b308ef83a5bb07bfdf80ae8
Reviewed-on: https://boringssl-review.googlesource.com/7052
Reviewed-by: Adam Langley <agl@google.com>
2016-02-16 19:32:50 +00:00
Adam Langley
d057454f90 Changes to support node.js's use of PKCS#12.
node.js uses a memory BIO in the wrong mode which, for now, we work
around. It also passes in NULL (rather than empty) strings and a
non-NULL out-arg for |d2i_PKCS12_bio|.

Change-Id: Ib565b4a202775bb32fdcb76db8a4e8c54268c052
Reviewed-on: https://boringssl-review.googlesource.com/7012
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 19:21:59 +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
David Benjamin
af07365b49 Check for overflow when parsing a CBS with d2i_*.
Until we've done away with the d2i_* stack completely, boundaries need
to be mindful of the type mismatch. d2i_* takes a long, not a size_t.

Change-Id: If02f9ca2cfde02d0929ac18275d09bf5df400f3a
Reviewed-on: https://boringssl-review.googlesource.com/6491
Reviewed-by: Adam Langley <agl@google.com>
2015-11-16 23:17:42 +00:00
Adam Langley
2e64f1b5d5 Check PKCS#8 pkey field is valid before cleansing.
(Imported from upstream's 52e028b9de371da62c1e51b46592517b1068d770.)

Change-Id: If980d774671b9b5ba997db3fd7d4043525a85609
Reviewed-on: https://boringssl-review.googlesource.com/6445
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-09 23:06:13 +00:00
Adam Langley
de659cdc2e Fix PKCS#8 on 32-bit systems.
The previous commit fixed a signed/unsigned warning but, on 32-bit
systems, long is only 32 bits, so the fix was incorrect there.

Change-Id: I6afe340164de0e176c7f710fcdd095b2a4cddee4
2015-10-27 16:18:51 -07:00
Adam Langley
13f1dd497f Fix a couple more signed/unsigned compares.
Different compilers find different problems.

Change-Id: I732611005ae1cbfcb4bc70c3f98af2c18b0a04da
2015-10-27 16:07:26 -07:00
Adam Langley
96c2a28171 Fix all sign/unsigned warnings with Clang and GCC.
Change-Id: If2a83698236f7b0dcd46701ccd257a85463d6ce5
Reviewed-on: https://boringssl-review.googlesource.com/4992
Reviewed-by: Adam Langley <agl@google.com>
2015-10-27 22:48:00 +00:00
David Benjamin
301afaf223 Add a run_tests target to run all tests.
It's very annoying having to remember the right incant every time I want
to switch around between my build, build-release, build-asan, etc.,
output directories.

Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and
Ninja 1.5+). This combination gives a USES_TERMINAL flag to
add_custom_target which uses Ninja's "console" pool, otherwise the
output buffering gets in the way. Ubuntu LTS is still on an older CMake,
so do a version check in the meantime.

CMake also has its own test mechanism (CTest), but this doesn't use it.
It seems to prefer knowing what all the tests are and then tries to do
its own output management and parallelizing and such. We already have
our own runners. all_tests.go could actually be converted tidily, but
generate_build_files.py also needs to read it, and runner.go has very
specific needs.

Naming the target ninja -C build test would be nice, but CTest squats
that name and CMake grumps when you use a reserved name, so I've gone
with run_tests.

Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c
Reviewed-on: https://boringssl-review.googlesource.com/6270
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 20:33:44 +00:00
Eric Roman
1aec2cbad2 Reject iterationCount == 0 when parsing PBKDF2-params.
Previously a value of 0 would be accepted and intepreted as equivalent
to 1. This contradicts RFC 2898 which defines:

     iterationCount INTEGER (1..MAX),

BUG=https://crbug.com/534961

Change-Id: I89623980f99fde3ca3780880d311955d3f6fe0b5
Reviewed-on: https://boringssl-review.googlesource.com/5971
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-10-02 16:19:04 +00:00
Adam Langley
2492586058 Add |PKCS12_verify_mac|.
This utility function is provided for API-compatibility and simply calls
|PKCS12_parse| internally.

BUG=536939

Change-Id: I86c548e5dfd64b6c473e497b95adfa5947fe9529
Reviewed-on: https://boringssl-review.googlesource.com/6008
Reviewed-by: Adam Langley <agl@google.com>
2015-09-29 20:30:35 +00:00
Matt Braithwaite
e000472166 pkcs8.c: Add PBES2 to list of password-based encryption methods.
This consists mostly of re-adding OpenSSL's implementation of PBKDF2
(very loosely based upon e0d26bb3).  The meat of it, namely
|PKCS5_PBKDF2_HMAC|, was already present, but unused.

In addition, |PKCS8_encrypt| and |PKCS8_decrypt| must be changed to
not perform UCS-2 conversion in the PBES2 case.

Change-Id: Id170ecabc43c79491600051147d1d6d3c7273dbc
Reviewed-on: https://boringssl-review.googlesource.com/5745
Reviewed-by: Adam Langley <agl@google.com>
2015-08-28 20:33:33 +00:00
Adam Langley
73415b6aa0 Move arm_arch.h and fix up lots of include paths.
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.

It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.

Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
2015-08-26 01:57:59 +00:00
David Benjamin
b143eca16e Consistently participate in err for PKCS12_* functions.
err is a horrible API, but functions should consistently participate or not
participate in it. We were missing a few codepaths.

Change-Id: I762074d5030b8e9d3e5bba9f8fa91fbdccbee25a
Reviewed-on: https://boringssl-review.googlesource.com/5571
Reviewed-by: Adam Langley <agl@google.com>
2015-08-05 18:53:06 +00:00