| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
|
|
|
|
|
|
|
| |
That we are still calling this (NB without error check because heritage),
made coverity unhappy.
CID 471705
|
|
|
|
|
|
|
|
| |
None of these function pointers were ever set. Now that the structure is
opaque they won't ever be, so time for them to hit the bitbucket. Infinite
extensibility of the toolkit results in complications, bugs, and dead code.
ok jsing
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
| |
These are only ever set to one particular function which is either local
to this file or part of the public API and we never added the public API
to set them to something else. Prefix the local functions touched in this
commit with x509_vfy_. More cleanup to follow.
ok joshua jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
| |
The struct underlying the X509_STORE type is opaque ars and nothing uses
the accessors that OpenSSL added blindly for these. Therefore we didn't
add them in the first place. So this rips out several dozens of lines of
dead code.
ok beck joshua jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
sk_deep_copy() is bad code. It is less bad than the upstream code, but
still bad: it passes strdup() through a void pointer and assigns it to
a function pointer of different type before calling the latter. That's
not kosher in more than one way.
There is no need for such gymnastics. If we need a deep copy for a type,
we should implement it as appropriate for that type.
Also, we should not expect and even less so allow holes in a STACK_OF().
The only way the vpm->hosts can be populated is by way of this deep_copy
function or x509_param_set_hosts_internal(), which pushes only after a
non-NULL check. Invariants: they're useful.
ok jsing
|
|
|
|
|
|
|
| |
I had to read this for other purposes and it exceeded my muppetry
tolerance.
ok tb@
|
| |
|
|
|
|
|
|
|
| |
This converts to proper single exit and undoes a number of unnecessarily
silly muppet antics.
ok beck
|
|
|
|
|
|
|
|
|
|
|
|
| |
Contrast "#define EVP_PKT_EXP 0x1000 /* <= 512 bit key */" with the diff:
- /* /8 because it's 1024 bits we look for, not bytes */
- if (EVP_PKEY_size(pk) <= 1024 / 8)
- ret |= EVP_PKT_EXP;
EVP_PKT_EXP will be nuked at the next opportunity.
discussed with jsing
|
| |
|
|
|
|
| |
ok beck jsing
|
|
|
|
|
|
|
| |
This doesn't do much right now, but is part of the tangle that is adding
RSA-PSS support.
ok beck jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
timegm(3) is not available on some operating systems we support in
portable. We currently use musl's implementation, for which gcc-13
decided to emit warnings (which seem incorrect in general and are
irrelevant in this case anyway). Instead of patching this up and
diverge from upstream, we can avoid reports about compiler warnings
by simply not depending on this function.
Rework the caching of notBefore and notAfter by replacing timegm(3)
with asn1_time_tm_to_time_t(3). Also make this API properly error
checkable since at the time x509v3_cache_extensions(3) is called,
nothing is known about the cert, in particular not whether it isn't
malformed one way or the other.
suggested by and ok beck
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These 'builder' functions, usually used together, can result in corrupt
ASIdentifiers on failure. In general, no caller should ever try to recover
from OpenSSL API failure. There are simply too many traps. We can still
make an effort to leave the objects in unmodified state on failure. This
is tricky because ownership transfer happens. Unfortunately a really
clean version of this seems impossible, maybe a future iteration will
bring improvements...
The nasty bit here is that the caller of X509v3_asid_add_id_or_range()
can't know from the return value whether ownership of min and max was
transferred or not. An inspection of (*choice)->u.range is required.
If a caller frees min and max after sk_ASIdOrRange_push() failed, there
is a double free.
All these complications could have been avoided if the API interface
had simply used uint32_t instead of ASN1_INTEGERs. The entire RFC 3779
API was clearly written without proper review. I don't know if there
ever was an actual consumer before rpki-client. If it existed, nobody
with the requisite skill set looked at it in depth.
ok beck for the general direction
with a lot of input and ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
X509_ALGOR_set0() is annoyingly unergonomic since it takes an ASN1_OBJECT
rather than a nid. This means that almost all callers call OBJ_obj2nid()
and they often do this inline without error checking so that the resulting
X509_ALGOR object is corrupted and may lead to incorrect encodings.
Provide an internal alternative X509_ALGOR_set0_by_nid() that takes a nid
instead of an ASN1_OBJECT and performs proper error checking. This will be
used to convert callers of X509_ALGOR_set0() in the library.
ok jsing
|
|
|
|
|
|
|
|
| |
If the topmost cert is invalid, this should result in a validation failure.
Do the same dance as elsewhere permitting the verify callback to intercept
the error but ensuring that we throw an error.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
X509_ALGOR_set_md() is a void function that cannot easily be error checked.
The caller has to jump through hoops to make sure this function doesn't
fail. Prepare replacing this internally with X509_ALGOR_set_evp_md(), which
allows error checking. There is one slight change of behavior: if the EVP_MD
object passed in does not have an OID known to the library, then this new
API fails.
It is unclear what the library should do with such an object and people
who use EVP_MD_meth_new() need to know what they are doing anyway and they
are better off teaching the lib about the OID if they're going to be
messing with certs.
Oh, and the prototype is in x509_local.h because the rest of this API is
in x509.h despite being implemented in asn1/.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
| |
Our checking here was a bit too aggressive, and did not permit an
IP address in a URI. IP's in a URI are allowed for things like CRLdp's
AIA, SAN URI's etc.). The check for this was also slightly flawed as
we would permit an IP if memory allocation failed while checking for
an IP.
Correct both issues.
ok tb@
|
|
|
|
|
|
|
| |
This code is a complete bug fest and using it with any other AFI is
downright dangerous. Such don't arise in this context in practice.
ok claudio jsing
|
|
|
|
| |
requested by jsing@
|
|
|
|
| |
OK tb@
|
|
|
|
|
|
|
| |
Reported by Viktor Szakats in
https://github.com/libressl/portable/issues/910
ok job
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The stack of subject alternative names from the issuer is parsed using
X509V3_EXT_d2i(), so it must be freed with sk_GENERAL_NAME_pop_free().
It's not worth doing complicated ownership handling when the individual
alternative names can be copied with GENERAL_NAME_dup().
Previously, ialt and its remaining members would be leaked when the call
to sk_GENERAL_NAME_push() failed halfway through.
This is only reachable via the issuer:copy x509v3.cnf(5) directive.
ok jsing
|
|
|
|
|
|
|
|
| |
On failure invalidate the cert with EXFLAG_INVALID. It's unlikely that
a cert would make it through to the end of this function without setting
the flag, but it's bad style anyway.
ok jsing
|
|
|
|
|
|
|
|
| |
These headers are now reduced to #include <openssl/ec.h> and are provided
for compatiblity only. There's no point in using them. At the same time
garbage collect the last uses of OPENSSL_NO_{ECDSA,ECDH} in our tree.
ok jsing
|
|
|
|
| |
ok & "happy pirate day" beck
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Per RFC 5280, 4.2: A certificate MUST NOT include more than one instance
of a particular extension.
This implements such a check in x509v3_cache_extensions() by sorting the
list of extensions and looking for duplicate neighbors. This sidesteps
complications from extensions we do not know about and keeps algorithmic
complexity reasonable. If the check fails, EXFLAG_INVALID is set on the
certificate, which means that the verifier will not validate it.
ok jsing
|
|
|
|
| |
This file is already enough of an eyesore without them.
|
|
|
|
|
|
|
|
|
| |
Only allow version v1-v3, disallow issuerUID and subjectUID in v1 certs
and require that if X509v3 extensions are present that the cert be v3.
Initial diff from job
ok job jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
| |
must_be_ca can no longer be 0 after the proxy cert code got nuked,
so change this to an if. must_be_ca is now -1 for a leaf, or 1 for
a non leaf.
ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is an un-revert with nits of the previously landed change
to do this which broke libtls. libtls has now been changed to
not use this function.
This change ensures that if something is returned it is "text"
(UTF-8) and a C string not containing a NUL byte. Historically
callers to this function assume the result is text and a C string
however the OpenSSL version simply hands them the bytes from an
ASN1_STRING and expects them to know bad things can happen which
they almost universally do not check for. Partly inspired by
goings on in boringssl.
ok jsing@ tb@
|
|
|
|
|
|
|
|
|
|
| |
Back in the day when essentially every struct was open to all applications,
X509_VERIFY_PARAM_ID provided a modicum of opacity. This indirection is now
no longer needed with X509_VERIFY_PARAM being opaque itself, so stop using
X509_VERIFY_PARAM_ID and merge it into X509_VERIFY_PARAM. This is a first
small step towards cleaning up the X509_VERIFY_PARAM mess.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Without this, hostflags set on the SSL_CTX would not propagate to newly
created SSL. This is surprising behavior that was changed in OpenSSL 1.1
by Christian Heimes after the issue was flagged by Quentin Pradet:
https://bugs.python.org/issue43522
This is a version of the fix that landed in OpenSSL.
There used to be a workaround in place in urllib3, but that was removed at
some point. We haven't fixed this earlier since it wasn't reported. It only
showed up after recent fallout of extraordinarily strict library checking
in urllib3 coming from their own interpretation of the implications of
PEP 644.
ok jsing
|
|
|
|
|
|
|
|
| |
This is needed for an upcoming regress test that needs to access the
hostflag. This is public API in OpenSSL but since nothing seems to be
using this, this accessor will be kept internal-only for the time being.
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
It is higly confusing to call the list of untrusted certs chain, when
you're later going to call X509_STORE_CTX_get0_chain() to get a completely
unrelated chain by the verifier. Other X509_STORE_CTX APIs call this list
of certs 'untrusted', so go with that. At the same time, rename the x509
into leaf, which is more explicit.
suggested by/ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When v3err.c was merged into x509_err.c nearly three years ago, it was
overlooked that the code needed two distinct pairs of ERR_FUNC/ERR_REASON,
one for ERR_LIB_X509 and one for ERR_LIB_X509V3. The result is that the
reason strings for the X509_R_* codes would be overwritten by the ones for
X509V3_R_* with the same value while the reason strings for all X509V3_R_*
would be left undefined.
Fix this by an #undef/#define dance for ERR_LIB_X509V3 once we no longer
the ERR_FUNC/ERR_REASON pair for ERR_LIB_X509.
reported by job
ok jsing
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
These helpers used to contain messy pointer bashing some with weird logic
for NUL termination. This can be written more safely and cleanly using
CBB/CBS, so do that. The result is nearly but not entirely identical to
code used elsewhere due to some strange semantics. Apart from errors pushed
on the stack due to out-of-memory conditions, care was taken to preserve
error codes.
ok jsing
|