These functions are not in any released version of OpenSSL. The history
is they were added to 1.0.2 beta for CT, but then removed in favor of
i2d_re_X509_tbs. We forked in between the two events.
I'm not sure what the reasoning was upstream's end. I'm thinking:
- X509 currently only captures the serialized TBSCertificate. It might
be nice to capture the whole Certificate to avoid needing a
serialization in X509_cmp and make it easier to interop with other
stacks. (Unclear.) That would require not exporting the X509_CINF
standalone for serialization.
- The modified bit means, without locking, i2d_X509 is not const or
thread-safe. We *might* be able to shift the re-encoding to
i2d_re_X509_tbs, which is already inherently non-const. That requires
not having X509_CINF_set_modified.
I'm not sure how feasible either of these are, but between that,
upstream alignment, and X509_CINF otherwise being absent from public
accessors, it seems worth removing.
Update-Note: X509_get_cert_info, X509_CINF_set_modified, and
X509_CINF_get_signature are removed. I believe all callers have been
updated. Callers should use i2d_re_X509_tbs, i2d_X509_tbs, and
X509_get0_tbs_sigalg instead.
Change-Id: Ic1906ba383faa7903973cb498402518985dd838c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46985
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is mostly to confirm the STACK_OF(ASN1_TYPE) was created the right
number of times.
Change-Id: I30c32f91cb6091e63bfcaebb0fe966270e503d93
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46984
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The X509_ATTRIBUTE structure includes a hack to tolerate malformed
attributes that encode the value directly instead of a set of values.
This form is never created by OpenSSL and shouldn't be needed any more.
(Imported from upstream's e20b57270dece66ce2c68aeb5d14dd6d9f3c5d68.)
This also changes X509_ATTRIBUTE_set1_data slightly. Previously,
set1_data would override whatever was previously in the X509_ATTRIBUTE,
but leak memory. Now set1_data appends to the set. (PKCS#10 attributes
use SET OF ANY as value.) It's unclear to me if this was intentional on
upstream's part. (The attrtype == 0 case only makes sense in the old
behavior.) Since there is no other way to create a two-element SET and
upstream has long since released this behavior, I left it matching
upstream.
Update-Note: Given OpenSSL hasn't accepted these for five years, it's
unlikely anything depends on it. If something breaks, we can revert this
and revisit. No one calls X509_ATTRIBUTE_set1_data on a non-empty
X509_ATTRIBUTE, so the behavior change there should be safe.
Change-Id: Ic03c793b7d42784072ec0d9a7b6424aecc738632
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46947
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
x509_req.c changes imported from upstream's
9b0a453190efc9b14cc04e74ce2e8e35af45fb39.
Update-Note: Direct accesses of X509_ATTRIBUTE should be replaced with
one of the accessors. I couldn't find any direct accesses, so hopefully
this is fine.
Change-Id: I7eab6375d5dcf366ef72e5ce059f3558c947f35b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46946
Reviewed-by: Adam Langley <agl@google.com>
I stopped short of documenting the add1_attr_by functions because the
type parameter is a bit of a mess. It appears to be several enums put
together. To that end, I've updated the documentation on
V_ASN1_MAX_UNIVERSAL to note that we also need to avoid MBSTRING_FLAG.
As a preview of what I'm putting off to later, see
X509_ATTRIBUTE_set1_data for how the type parameter is used. set1_data
is extra fun because PKCS#10 attributes are set-valued. Plus there's
upstream's e20b57270dece66ce2c68aeb5d14dd6d9f3c5d68, which we should
import first.
Change-Id: I3453a0b224e42c6e22828c7d332ee133e09e6173
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46945
Reviewed-by: Adam Langley <agl@google.com>
Update-Note: Direct accesses of X509_PUBKEY should be replaced with one
of the accessors. I believe all callers have been fixed at this point.
Change-Id: Ib325782867478fb548da1bf5ef0023cf989f125b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46944
Reviewed-by: Adam Langley <agl@google.com>
The ASN1_BOOLEAN representation is a mess. ASN1_BOOLEAN is an int
and if non-negative (negative values mean omitted or default), gets cast
to uint8_t and encoded as the value. This means callers are simply
expected to know true is 0xff, not 1. Fix this by only encoding 0 or
0xff.
This also fixes a bug where values like 0x100 are interpreted as true
(e.g. in the tasn_enc.c logic to handle default values), but encoded as
false because the cast only looks at the least significant byte.
This CL does not change the parsing behavior, which is to allow any BER
encoding and preserve the value in the in-memory representation (though
we should tighten that). However the BER encode will no longer be
preserved when re-encoding.
Update-Note: Callers setting ASN1_BOOLEANs to a positive value other
than 0xff will now encode 0xff. This probably fixes a bug, but if anyone
was attaching significance to incorrectly-encoded booleans, that will
break.
Change-Id: I5bb53e068d5900daca07299a27c0551e78ffa91d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46924
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See go/handshake-hints (internal).
CL originally by Bin Wu <wub@google.com>. I just reworked the tests and
tidied it up a bit. This is the start of a replacement for the split
handshakes API. For now, only TLS 1.3 is supported. It starts with an
initial set of hints, but we can add more later. (In particular, we
should probably apply the remote handshaker's extension order to avoid
needing to capability protect such changes.)
Change-Id: I7b6a6dfaa84c6c6e3436d2a4026c3652b8a79f0f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46535
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
They still need to be Python-2-compatible until I figure out how to
switch the version used in the CI.
I've left out make_curve25519_tables.py because it's some bytes vs
unicode mess I don't care to figure out. We should just rewrite that in
Go which should also be much faster anyway.
Change-Id: I4446641815315a84c2979b1be1e1949f88cbacf8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46884
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See also 86a90dc749af91f8a7b8da6628c9ffca2bae3009 from upstream. This
differs from upstream's which treats {NULL, 2} as a valid way to spell
the empty list. (I think this is a mistake and have asked them about
it.)
Upstream's CL also, for them, newly makes the empty list disable ALPN,
when previously they'd disable it but misread it as a malloc failure.
For us, we'd already fixed the misreading due to our switch to
bssl::Array and bssl::Span, but the documentation was odd. This CL
preserves that behavior, but updates the documentation and writes a
test.
Update-Note: SSL_CTX_set_alpn_protos and SSL_set_alpn_protos will now
reject invalud inputs. Previously, they would accept them, but silently
send an invalid ALPN extension which the server would almost certainly
error on.
Change-Id: Id5830b2d8c3a5cee4712878fe92ee350c4914367
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46804
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Rather than computing kVarianceBlocks, which is hard to reason about,
use a sha1_final_with_secret_suffix abstraction. This lets us separate
reasoning in bytes about the minimum and maximum values of |data_size|
and the interaction with HMAC, separately from the core constant-time
SHA-1 update.
It's also faster. I'm guessing it's the more accurate block counts.
Before:
Did 866000 AES-128-CBC-SHA1 (16 bytes) open operations in 2000697us (6.9 MB/sec)
Did 616000 AES-128-CBC-SHA1 (256 bytes) open operations in 2001403us (78.8 MB/sec)
Did 432000 AES-128-CBC-SHA1 (1350 bytes) open operations in 2003898us (291.0 MB/sec)
Did 148000 AES-128-CBC-SHA1 (8192 bytes) open operations in 2006042us (604.4 MB/sec)
Did 83000 AES-128-CBC-SHA1 (16384 bytes) open operations in 2010885us (676.3 MB/sec)
After:
Did 2089000 AES-128-CBC-SHA1 (16 bytes) open operations in 2000049us (16.7 MB/sec) [+141.3%]
Did 851000 AES-128-CBC-SHA1 (256 bytes) open operations in 2000034us (108.9 MB/sec) [+38.2%]
Did 553000 AES-128-CBC-SHA1 (1350 bytes) open operations in 2002169us (372.9 MB/sec) [+28.1%]
Did 178000 AES-128-CBC-SHA1 (8192 bytes) open operations in 2008596us (726.0 MB/sec) [+20.1%]
Did 98000 AES-128-CBC-SHA1 (16384 bytes) open operations in 2001509us (802.2 MB/sec) [+18.6%]
Confirmed with valgrind tooling that this is still constant-time. In
doing so, I ran into a new nuisance with GCC. In loops where we run
constant_time_lt with a counter value, GCC sometimes offsets the loop
counter by the secret. It cancels it out before dereferencing memory,
etc., but valgrind does not know that x + uninit - uninit = x and gets
upset. I've worked around this with a barrier for now.
Change-Id: Ieff8d2cad1b56c07999002e67ce4e6d6aa59e0d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46686
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This CL adds an initial implementation of the ECH server, with pieces of
the client in BoGo as necessary for testing. In particular, the server
supports ClientHelloInner compression with ech_outer_extensions. When
ECH decryption fails, it can send retry_configs back to the client.
This server passes the "ech-accept" and "ech-reject" test cases in
tls-interop-runner[0] when tested against both the cloudflare-go and nss
clients. For reproducibility, I started with the main branch at commit
707604c262d8bcf3e944ed1d5a675077304732ce and updated the endpoint's
script to pass the server's ECHConfig and private key to the boringssl
tool.
Follow-up CLs will update HPKE to the latest draft and catch us up to
draft-10.
[0]: https://github.com/xvzcf/tls-interop-runner
Bug: 275
Change-Id: I49be35af46d1fd5dd9c62252f07d0bae179381ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45285
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This removes the now unnecessary virtual calls. Benchmark differences are
mostly positive but probably noise.
Before:
Did 839000 AES-128-CBC-SHA1 (16 bytes) open operations in 2000497us (6.7 MB/sec)
Did 623000 AES-128-CBC-SHA1 (256 bytes) open operations in 2000409us (79.7 MB/sec)
Did 434000 AES-128-CBC-SHA1 (1350 bytes) open operations in 2002909us (292.5 MB/sec)
Did 146000 AES-128-CBC-SHA1 (8192 bytes) open operations in 2000785us (597.8 MB/sec)
Did 82000 AES-128-CBC-SHA1 (16384 bytes) open operations in 2014268us (667.0 MB/sec)
After:
Did 866000 AES-128-CBC-SHA1 (16 bytes) open operations in 2000697us (6.9 MB/sec) [+3.2%]
Did 616000 AES-128-CBC-SHA1 (256 bytes) open operations in 2001403us (78.8 MB/sec) [-1.2%]
Did 432000 AES-128-CBC-SHA1 (1350 bytes) open operations in 2003898us (291.0 MB/sec) [-0.5%]
Did 148000 AES-128-CBC-SHA1 (8192 bytes) open operations in 2006042us (604.4 MB/sec) [+1.1%]
Did 83000 AES-128-CBC-SHA1 (16384 bytes) open operations in 2010885us (676.3 MB/sec) [+1.4%]
Change-Id: I735e99296ca9a1771518c622b8e7e6979a0d30bc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46685
Reviewed-by: Adam Langley <agl@google.com>
AES_128_GCM is more common than AES_GCM_128 and matches the
specification.
Bug: 275
Change-Id: If3446a38f7bfbe0250d9646e363db29b93e4d231
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46666
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Dan McArdle <dmcardle@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We have loads of variations of these. Align them in one set. This avoids
the HOST_* macros defined by md32_common.h, so it'll be a little easier
to make it a more conventional header.
Change-Id: Id47fe7b51a8f961bd87839f8146d8a5aa8027aa6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46425
Reviewed-by: Adam Langley <agl@google.com>
It's a little confusing to have load_word_le but actually use size_t
instead of crypto_word_t.
NOTE: on some platforms, notably NaCl, crypto_word_t is larger than
size_t. (Do we still need to support this?) We don't have a good testing
story here, so I tested it by hacking up a 32-bit x86 build to think it
was OPENSSL_64_BIT.
Change-Id: Ia0ce469e86803f22655fe2d9659a6a5db766429f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46424
Reviewed-by: Adam Langley <agl@google.com>
MSAN doesn't like the counters starting at whatever value malloc
found to be free.
Change-Id: I0968e61e0025db35b82291fde5d1e193aef77c1e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46444
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
It's now a year past the February 2020 deadline for removing it. Judging
from b/72831885, it looks like the root cause was addressed.
Change-Id: I8c8b358ef4f4146b41aab2a7163c000fa7306025
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46407
Reviewed-by: Adam Langley <agl@google.com>
Although it is strictly fine to call SHA512_Final in SHA384_Final
(array sizes in C parameters are purely decorational, according to the
language), GCC 11 reportedly checks now and gets upset about the size
mismatch. Use an unsized helper function so all our code matches the
specified bounds.
Unfortunately, the bounds in all the functions are a bit misleading
because SHA512_Final really outputs based on sha->md_len (which Init
function you called) rather than which Final function. I've fixed this
places within a library where we mismatched and added asserts to the
smaller functions. SHA512_Final is assert-less because I've seen lots of
code use SHA384_Init / SHA512_Update / SHA512_Final.
This doesn't fix the SHA256 variant since that is generated by a pile of
macros in a multiply-included file. This is probably a good opportunity
to make that code less macro-heavy.
Update-Note: There is a small chance the asserts will trip something,
but hopefully not since I've left SHA512_Final alone.
Bug: 402
Change-Id: I4c9d579a63ee0a0dea103c19ef219c13bb9aa62c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46405
Reviewed-by: Adam Langley <agl@google.com>
It's now 2021. Hopefully we can at least assume anyone building with
-std=c11 also has a corresponding set of headers. Plus, even if you
don't, Clang seems to provide a header. (So C11 atomics work in
clang-cl.) Also apparently atomics are optional, so this checks
__STDC_NO_ATOMICS__.
This does *not* set C11 as the minimum version. If you build with
-std=c99, we'll silently use the non-atomics implementation. That's a
little magical, so I've kept OPENSSL_C11_ATOMIC as a way to assert that
you really want C11 atomics. Mostly it turns into a -std=c11 && !MSVC
self-assert.
Update-Note: If something fails to compile, we'll revert this and adjust
the check, or add an opt-out, or give up. Also, if building with
-std=c99, consider -std=c11.
Change-Id: I1a8074c367a765c5a0f087db8c250e050df2dde8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46344
Reviewed-by: Adam Langley <agl@google.com>
PKCS#10 CSRs don't contain extensions but attributes, which are kind of
like extensions, but defined separately. There is an attribute type from
PKCS#9 to embed a list of X.509 extensions inside an attribute, as well
as a Microsoft variant.
X509_REQ_set_extension_nids allowed callers globally reconfigure the set
of attributes recognized as aliases of this extensions attribute. This
is not used by anyone and not thread-safe. Remove it and only support
the two default attribute types.
From there, document the remaining functions.
Update-Note: This removes a pair of unused functions.
Change-Id: Ic1fc41163996c0c980ba8320b417e444d484aa39
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46326
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This fixes a bug in ASN1_TYPE_get. Partly imported from upstream's
261ec72d58af64327214a78ca1c54b169ad93c28, though I don't believe
ASN1_TYPE_set was broken per se. There's also a lot more than in that
commit.
I've added a test to ensure we maintain the unused bits invariant
anyway, in case external code relies on it. (The invariant comes from
the pointer being NULL-initialized and from ASN1_primitive_free zeroing
*pval on free.)
Change-Id: I4c0c57519a7628041d81c26cd850317e01409556
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46324
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This API does not come from OpenSSL, but OpenSSL does not appear to have
any way to get this information. There is X509_get0_pubkey_bitstr, but
that only works for X509 objects, not X509_PUBKEY.
Change-Id: Ifc8be554a4d8cbf830c32b95b953f092980804df
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46304
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See also 8129ac6ac4c0ca3a488c225cde580ede7dabe874 and
81198bf323ea9deda907714170d329ca7d2ff01f from upstream.
In trying to figure out why ASan (which normally catches overlapping
memcpys) didn't flag this, I noticed that we actually don't have tests
for empty inputs. I've added them to cipher_tests.txt where missing and
fixed a bad assert in ofb.c.
ASan still doesn't flag this because LLVM even requires memcpy handle
dst == src. Still, fixing it is less effort than getting a clear answer
from GCC and MSVC. Though this puts us in the frustrating position of
trying to follow a C rule that our main toolchain and sanitizer disavow.
https://bugs.llvm.org/show_bug.cgi?id=11763https://reviews.llvm.org/D86993
Change-Id: I53c64a84834ddf5cddca0b3d53a29998f666ea2f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46285
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
All our EVP_CIPHERs are deterministic, so there's usually no point in
testing only one direction. Some of the ECB tests were missing free
decryption tests. CTR is the same in both directions, but we ought to
test the API agrees. OFB vectors are doubled up, so we can merge them
together. Plus there are typos in the OFB-AES192.Decrypt tests, also
present upstream, so we weren't actually testing everything we should.
(I haven't removed the direction-specific logic altogether since the
tests imported from nist_cavp rely on it. Though there may be something
to be said for running them both ways since they don't actually double
them up...)
Change-Id: I36a77d342afa436e89ad244a87567e1a4c6ee9dc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46284
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
In doing so, this switches make_errors.go to take library names as
parameters rather than detecting it from the CWD. (I considered
detecting it, but then we'd need to map evp -> crypto/whatever and
crypto/whatever -> evp in both directions.)
Since crypto/hpke currently sits in the EVP namespace, I've gone ahead
and added that, so it should be easier to define new errors in
crypto/hpke. I've not added crypto/cipher, etc., yet. Moving those will
be a breaking change (consumers that put ERR_LIB_CIPHER and ERR_LIB_EVP
in a switch/case need patches).
Bug: 398
Change-Id: Ibae2afd46e076891fa517c377b540b2e492516f0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46264
Reviewed-by: Adam Langley <agl@google.com>
The build scripts distinguish between normal files and bcm.c fragments
based on whether code is in a subdirectory inside crypto/fipsmodule.
Bug: 401
Change-Id: Ieba88178e4f8e19f020e56e2567d5736a34bb43f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46224
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The representation here is a bit more messy than necessary. In doing so,
clean up the variable names and smooth away two rough edges:
- X509_ALGOR_get0 would leave *out_param_value uninitialized if
*out_param_type is V_ASN1_UNDEF. Instead, set it to NULL, so callers
do not accidentally use an uninitialized pointer.
- X509_PUBKEY_set0_param, if key is NULL, would leave the key alone. No
one calls this function externally and none of the (since removed)
callers in OpenSSL rely on this behavior. A NULL check here adds a
discontinuity at the empty string that seems unnecessary here:
changing the algorithm without changing the key isn't useful.
(Note the API doesn't support changing the key without the algorithm.)
Note for reviewing: the representation of ASN1_TYPE is specified
somewhat indirectly. ASN1_TYPE uses the ASN1_ANY ASN1_ITEM, which has
utype V_ASN1_ANY. Then you look at asn1_d2i_ex_primitive and asn1_ex_c2i
which peel off the ASN1_TYPE layer and parse directly into the value
field, with a fixup for NULL. Hopefully we can rework this someday...
Change-Id: I628c4e20f8ea2fd036132242337f4dcac5ba5015
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46165
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In order to provide evidence to auditors that high-level functions end
up calling into the FIPS module, provide counters that allow for such
monitoring.
Change-Id: I55d45299f3050bf58077715ffa280210db156116
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46124
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
ASN1_OBJECTs are awkward. Sometimes they are static, when returned from
OBJ_nid2obj, and sometimes they are dynamic, when parsed from
crypto/asn1.
Most structures in crypto/asn1 need to support unknown OIDs and thus
must own their ASN1_OBJECTs. But they also may be initialized with
static ones in various APIs, such as X509_ALGOR_set0. To make that work,
ASN1_OBJECT_free detects static ASN1_OBJECTs and is a no-op.
Functions like X509_ALGOR_set0 take ownership, so OpenSSL has them take
a non-const ASN1_OBJECT*. To match, OBJ_nid2obj then returns a non-const
ASN1_OBJECT*, to signal that it is freeable.
However, this means OBJ_nid2obj's mutability doesn't match its return
type. In the fork, we switched OBJ_nid2obj to return const. But, in
doing so, we had to make X509_ALGOR_set0 and X509_PUBKEY_set0_param take
const ASN1_OBJECT, even though they would actually take ownership of
dynamic ASN1_OBJECTs. There are also a few internal casts with a TODO to
be const-correct.
Neither situation is ideal. (Perhaps a more sound model would be to copy
static ASN1_OBJECTs before putting them in most structs. But that would
not match current usage.) But I think aligning with OpenSSL is the
lesser evil here, since it avoids misleading set0 functions. Managing
ownership of ASN1_OBJECTs is much more common than mutating them. To
that end, I've added a note that ASN1_OBJECTs you didn't create must be
assumed immutable[*].
Update-Note: The change to OBJ_nid2obj should be compatible. The changes
to X509_PUBKEY_set0_param and X509_ALGOR_set0 may require fixing some
pointer types.
[*] This is *almost* honored by all of our functions. The exception is
c2i_ASN1_OBJECT, which instead checks the DYNAMIC flag as part of the
object reuse business. This would come up if we ever embedded
ASN1_OBJECTs directly in structs.
Change-Id: I1e6c700645c12b43323dd3887adb74e795c285b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46164
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL classified their behavior as a bug and are fixing it for the
next release. In principle it'd be more compatible to emulate OpenSSL's
bug and undo it when we update OPENSSL_VERSION_NUMBER, but use of
PKCS12_parse is rare and this behavior is confusing, so let's leave it
as-is.
Bug: 250
Change-Id: I5f9825490a8afde67272dfaf476b35dbde94b59c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46064
Reviewed-by: Adam Langley <agl@google.com>
We aim to eventually make the entire X509 structure opaque, but let's
start small.
Update-Note: I believe this is now safe to do. If there are compile
failures, switch to X509_get0_notBefore, X509_getm_notBefore, and
X509_set1_notBefore, or revert this if I'm wrong and too many callers
still need updating.
Change-Id: I6e9d91630a10ac777e13ebcdeb543b3cbeea6383
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45965
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
PKCS#12 is overly general and, among other things, supports disabling
encryption. In practice, the unencrypted form is not widely implemented.
Moreover, even in contexts where cleartext is fine, an unencrypted
PKCS#12 file still requires a password for the mandatory MAC component.
They're not very useful.
However, cryptography.io uses them. Previously, we added support for
parsing these. This CL adds support for creating them too, because now
cryptography.io now also depends on that.
Change-Id: Ib7c4e29615047b6c73f887fea7c80f8844999bb7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46045
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
pkcs12_test.cc was getting a bit long. Along the way, embed_test_data.go
needed a fix to work around a syntax quirk of C++.
Change-Id: Ic4a19f77d177ebd607918feb253a08f1f9037981
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46044
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Update-Note: This removes a function that appears to be unused. It also
hardcodes the use of MD5, so please do not use it.
Change-Id: I67909c6360e4737fc22742592f88b907eb818e96
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45964
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
I got the values flipped around. Also cryptography.io wants
EC_GROUP_get_asn1_flag to check a curve's encoding. We (mostly) only
support named curves, so just return OPENSSL_EC_NAMED_CURVE.
Change-Id: I544e76b7380ecd8dceb1df3db4dd4cf5cb322352
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46024
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This improves compatibility with cryptography.io. cryptography.io
doesn't actually care what we return, since the code won't run, but
feigning success seems better than failure. If some application does try
to run this function and checks, returning an error will probably crash
it.
Change-Id: I7a8164753a2f1a7b31dbeb10c7030c5e5fea2bc9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46004
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Per RFC5280, section 5.1.1.2,
[signatureAlgorithm] MUST contain the same algorithm identifier as the
signature field in the sequence tbsCertList (Section 5.1.2.2).
This aligns with a check we already do on the X.509 side.
Update-Note: Invalid CRLs with inconsistent inner and outer signature
algorithms will now be rejected.
Change-Id: I9ef495a9b26779399c932903871391aacd8f2618
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45946
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This may as well be computed from block_size. This reduces the
per-EVP_CIPHER_CTX memory usage slightly.
Update-Note: It doesn't look like anyone is reading into this field. If
they are, we can ideally fix it, or revert this if absolutely necessary.
Change-Id: Ieef9177bed1671efca23d4f94d3d528f82568fc6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45884
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The X509 version APIs confuse everyone, including our own tests
(v3name_test.cc was using the wrong value). Introduce constants. Also
document which version to use for each type. It is extra confusing that,
although certificates and CRLs use the same Version enum, RFC5280
defines X.509 v3 certificates with X.509 v2 CRLs. (There are no v3
CRLs.)
I'll send a similar patch to OpenSSL to keep upstream aligned.
Change-Id: If096138eb004156d43df87a6d74f695b04f67480
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45944
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is a no-op, but relying on ASN1_INTEGER_get's internal handling of
NULL is confusing.
Change-Id: I49ab38c170d2f39fd3a4780b8c44b103a2ba4615
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45924
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Daniel McArdle
Adam Langley
Adam Langley
Bradley Hess