This flag is set when an ASN1_STRING is created from a codepath that is
aware it is an "mstring" (CHOICE of multiple string or string-like
types). With setters like X509_set_notBefore, it is very easy to
accidentally lose the flag on some field that normally has it.
The only place the flag is checked is X509_time_adj_ex. X509_time_adj_ex
usually transparently picks UTCTime vs GeneralizedTime, as in the X.509
CHOICE type. But if writing to an existing object AND if the object
lacks the flag, it will lock to whichever type the object was
previously. It is likely any caller hitting this codepath is doing so
unintentionally and has a latent bug that won't trip until 2050.
In fact, one of the ways callers might accidentally lose the
ASN1_STRING_FLAG_MSTRING flag is by using X509_time_adj_ex!
X509_time_adj_ex(NULL) does not use an mstring-aware constructor. This
CL avoids needing such a notion in the first place.
Looking through callers, the one place that wants the old behavior is a
call site within OpenSSL, to set the producedAt field in OCSP. That
field is a GeneralizedTime, rather than a UTCTime/GeneralizedTime
CHOICE. We dropped that code, but I'm making a note of it to remember
when filing upstream.
Update-Note: ASN1_STRING_FLAG_MSTRING is no longer defined and
X509_time_adj_ex now behaves more predictably. Callers that actually
wanted to lock to a specific type should call ASN1_UTCTIME_adj or
ASN1_GENERALIZEDTIME_adj instead.
Change-Id: Ib9e1c9dbd0c694e1e69f938da3992d1ffc9bd060
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48668
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This covers most of the ASN.1 time functions and a handful more of
x509.h. Also remove some code under #if 0.
I'm running out of a easy ones to do, which is probably a good thing.
Change-Id: I085b1e2a54d191a7a5f18c801b3c135cfda7bd88
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48665
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
These macros aren't consumed by anything anymore.
Change-Id: Id9616fa0962ae0dbf27bc884c6883dcad9755eb2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48229
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's 9689a6aeed4ef7a2357cb95191b4313175440e4c.
X509_VERIFY_PARAM_ID made sense as a separate structure when
X509_VERIFY_PARAM was public, but now the struct is unexported.
Change-Id: I93bac64d33b76aa020fae07bba71b04f1505fdc4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48128
Reviewed-by: Adam Langley <agl@google.com>
Upstream ultimately preferred a different naming convention, and
type-specific constants. Align with them.
Update-Note: This renames some BoringSSL-specific constants that we
recently added. It doesn't look like anyone's used them yet.
Change-Id: I580e0872a5f09fb1c5bab9127c35f1ed852680c0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47164
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>
X509_STORE_CTX_init is documented upstream to allow a NULL store and has
logic to account for it. However, attempting to use such an
X509_STORE_CTX crashes in X509_verify_cert due to the
additional_untrusted logic we added.
Moreover, before that change, it still crashes because
X509_STORE_CTX_get1_issuer (the default get_issuer hook) assumes
ctx->ctx (the store) is non-null. This was also true in upstream but
later fixed in https://github.com/openssl/openssl/pull/6001. However,
without a store, there is no trust anchor, so this is not very useful.
Reject NULL stores in X509_STORE_CTX_init and remove the logic allowing
for a NULL one.
Thanks to Danny Halawi for catching this.
Update-Note: X509_STORE_CTX_init will now fail when the store is NULL,
rather than report success, only to crash later in X509_verify_cert.
Breakage should thus be limited to code which was passing in a NULL
store but never used the resulting X509_STORE_CTX.
Change-Id: I9db0289612cc245a8d62d6fa647d6b56b2daabda
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42728
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
OpenSSL renamed the preferred spelling of X509_set_notBefore to
X509_set1_notBefore, etc., in 568ce3a583a17c33feacbf5028ece9f7f0680478.
Add the set1 names and update uses within the library.
Change-Id: Ib211e356da9de963990ad2b330249383ccfef7e5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42524
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Adam Langley