PEM_X509_INFO_read_bio is weird. It decrypts certificates and CRLs, but
not private keys. We had some comments just saying we were trying to
preserve historical (untested) behavior, but I think I've figured out
why. It's so you can inspect a bundle of certs + encrypted keys without
knowing the password. Attempting but failing to decrypt is fatal.
On the flip side, this means that you cannot use this to decrypt the
private key even if you wanted to! This was probably a mistake in
SSLeay, but probably not worth fixing since this function's grouping
behavior doesn't handle certificate chains right anyway.
But we should at least document and test the intended behavior. This
tests that encrypted private keys are left as placeholders, though I
haven't filled in an encrypted certificate or CRL. (The main nuisance
there is assembling a test input because OpenSSL's APIs don't even let
you make them.)
Bug: 387737061
Change-Id: Iebcafdba4924bbcb6298bde24013a508aecc716a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74810
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The Proc-Type header starts with a version number. From RFC 1421,
Section 4.6.1.1:
> The "Proc-Type:" field has two subfields, separated by a comma. The
> first subfield is a decimal number which is used to distinguish among
> incompatible encapsulated header field interpretations which may
> arise as changes are made to this standard. Messages processed
> according to this RFC will carry the subfield value "4" to
> distinguish them from messages processed in accordance with prior PEM
> RFCs.
RFC 1421 was an update of RFC 1113, which used X-Proc-Type: 3,ENCRYPTED,
which we do not support. RFC 1040 defined X-Proc-Type: 2 and RFC 989
defined X-Proc-Type: 1,E. As far as I can tell, no other numbers for
Proc-Type have ever existed, and likely never will.
If we were to ever Proc-Type: 5, we currently return failure but forget
to put something on the error queue. It's possible this was originally
done for extensibility, but it just breaks internal invariants. By
returning zero, the callers will treat this as an error anyway. We'll
just confuse code that expects OpenSSL to return an error. Also
OpenSSL's PEM APIs all treat failure to decrypt (e.g. due to unsupported
future cipher) as an error, so we should treat an unsupported future PEM
encryption scheme as an error too.
Change-Id: Ia1f4f6776fea8d8a465a73105bf3ce24a587f26b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74809
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
These aren't used externally. While I'm here, const-correct
PEM_do_header. Really we could have just made these file-local except
that PEM_X509_INFO_read_bio does something weird.
Bug: 42290574
Change-Id: I455b9c31da0efb854925bbe38797d3c0e221fcdf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74807
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This applies the OpenSSL "copyright consolidation" process from the
following upstream changes:
* e0a651945c
* 3fb2cf1ad1
* ac3d0e1377
* c2f312f5c2
* 596d6b7e1c
* e18cf66aaf
* 846e33c729
* 440e5d805f
* 21dcbebc6e
* 6286757141
* 4f22f40507
* d2e9e32018
* 2039c421b0
* b1322259d9
* aa6bb1352b
* b6cff313cb
* 9e20068958
* 6aa36e8e5a
* 44c8a5e2b9
This was mostly automated, but partially manual. The automated portion
can be reproduced by checking OpenSSL to commit
44c8a5e2b9af8909844cc002c53049311634b314, and running the following:
git grep -l -E 'Copyright remains Eric Young|Copyright.*The OpenSSL Project\.|Written by.*for the OpenSSL Project' crypto/ decrepit/ include/ ssl/ | grep -v objects.go > files.txt
cat files.txt | xargs -n1 perl -i ./util/copyright.pl
From there, some years were fixed up manually according to
go/openssl-copyright-consolidation-comparison (internal-only).
Three files required additional manual fixing:
- crypto/ecdh_extra/ecdh_extra.cc
- crypto/fipsmodule/ecdh/ecdh.cc.inc
- include/openssl/ecdh.h
These files have an OpenSSL header, but *after* a different header, so
the script does not correctly detect the now redundant OpenSSL header.
They were manually modified to remove it. This matches what seems to
have been done to crypto/ec/ecdh_ossl.c in OpenSSL's
4f22f40507fea3f272637eb8e00cadf1f34b10d9.
Bug: 364634028
Change-Id: I79a559a409ebe2476f2cb8a48a488ac5dd77c90a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74710
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This aligns with https://github.com/openssl/openssl/pull/6173 from
upstream OpenSSL. As part of this, I had to fix PEM_def_callback (which
is different in us vs BoringSSL) to use -1 as the error value, not 0.
Otherwise errors get misinterpreted as empty strings.
As part of this, make sure all the functions being fixed are covered by
tests.
Fixed: 362788352
Change-Id: I2b5071534c77944d473580fda98d23ae3b54e2d5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/70787
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Also rename to pass and pass_len, which makes a bit more sense for what
these are.
Change-Id: If3421ed7890c92cd11130641a8a2e090cc7f8b91
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65810
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
It's less bad than I originally wrote because trust properties only
matter if configured on the X509_STORE. Add a test for this.
This is good because lots of functions trigger d2i_X509_AUX, so I think
we have to assume attackers can specify these values. Nonetheless, this
is surprising, so document which functions trigger this.
Change-Id: I73ce44acfa2a373ef3f3ef09c3e46cea98124f33
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65791
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Although this is only used by <openssl/pem.h>, one existing caller
expects the free functions to be defined in <openssl/x509.h>. It's not
really worth it to put it in the other header, so just move it back.
Change-Id: I7e719d51110b567296fcd797f72d13aa41de73af
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64287
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
X509_INFO only exists to be a return value to PEM_X509_INFO_read. There
is no use in letting callers create these objects, since they cannot do
anything with it. Only X509_INFO_free is needed.
Also cut a ton of unused fields from X509_PKEY.
Change-Id: I322589f04883903e1fe5c23c3966ecf631e85b7f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64127
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Also unexport PEM_proc_type and PEM_dek_info. They're never called
externally, just private functions within one file. Also, while I'm
here, fix the include guard on asn1/internal.h.
Bug: 516
Change-Id: I6961a65f638e7b464a8c349663898a954d7826b4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58548
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Change-Id: I6fa17e204cb2003a6803e01604c0187420b4e39b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56945
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Nothing calls this function, it doesn't support most key types, and
accesses pkey.rsa without checking the type. Just remove it.
Change-Id: I073dfe74c545c7e08578b85105c88a19bbddf58a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53505
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
We've never tested this and plenty of files depend on FILE* APIs without
ifdefs.
Change-Id: I8c51c043e068b30bdde1723c3810d3e890eabfca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48771
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Adam Langley