| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is essentially unused. The only consumer, www/kore,-acme is in the
process of being fixed. It is also incomplete: in particular, the verifier
doesn't learn about extensions added to the list, making the entire
exercise rather pointless. So let's ditch that crap.
This was the last consumer of the horror that is OBJ_bsearch_().
The even worse OBJ_bsearch_ex_() is still being "used" by M2Crypto...
This prepares the removal of X509V3_EXT_{add{,_list,_alias},cleanup}().
and removes another piece of thread-unsafe global state.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
X509_check_trust() is of course used by the verifier. Unfortunately
M2Crypto exposes it. The only other part of the X509_TRUST API that
are still needed are the X509_TRUST_* macros in x509.h, as they are
used via *_set_trust and indirectly via the purpose stuff. The rest
will be removed.
X509_TRUST_add() was defanged recently, in particular it no longer
hangs strdup()'ed strings off the global struct. Nothing ever cleaned
these up. TRUST_cleanup() attempted to do so, but since it checked
the dynamic/dynamic strings flags in the wrong order, that cleanup
call ended up doing nothing, so that code was removed at some point.
As a consequence, the struct can now be made const. Use a CTASSERT()
to ensure size assumptions on X509_TRUST_COUNT, X509_TRUST_MAX, and
X509_TRUST_MIN hold true.
Remove the global variable underlying X509_TRUST_set_default()'s
functionality and move its accessor down to all the other functions
that will be deleted.
Inline a few things in X509_check_trust(), so we can excise the
internals of X509_TRUST_get0(), X509_TRUST_get_by_id(). Since the
default trust function can no longer be changed, call obj_trust()
directly.
ok jsing
|
| |
|
|
|
|
|
| |
Now they are next to the trstandard[] table and listed in the order they
appear in the table.
|
| |
|
|
|
|
|
| |
Hoist obj_trust() to the top and move the static default_trust() next
to its setter.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Split the two codepaths in x509_vfy_purpose_inherit() into its two callers.
What remains is gross, but at least a reader has a chance of following all
this nonsense without leaving a significant amount of hair behind.
In short, purpose and trust are only overridden if they're not already
set. Otherwise silently ignore valid purpose and trust identifiers that
were passed in and succeed. Error on almost all invalid trust or purpose
ids, except 0, because... well... who knows, really?
ok jsing
|
|
|
|
| |
Noticed by anton
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Nothing uses this function, except two internal callers. So split its guts
temporarily into a helper function and disable the gross general case.
The internal helper can be simplified by observing that def_purpose == 0:
Overriding 0 by 0 doesn't do anything, so drop that bit. Rename ptmp into
purp, and inline X509_PURPOSE_get_by_id(), i.e., make appropriate checks and
subtract X509_PURPOSE_MIN. The fallback to X509_PURPOSE_get_by_id(0) will
always fail since X509_PURPOSE_MIN == 1. So ditch that call. In particular,
X509_STORE_CTX_set_purpose(ctx, X509_PURPOSE_ANY) fails in current because
of this. That's nonsense. So remove the purp->trust == X509_TRUST_DEFAULT
check as only change of behavior. This matches what OpenSSL do nowadays.
They now set def_purpose = purpose if purpose != 0 and def_purpose == 0,
so in all real-world uses of this function they will just fetch the same
purpose again and do not check for default trust the second time around.
Finally, X509_TRUST_get_by_id() is only used to ensure that a non-zero (or
overridden) trust is between X509_TRUST_MIN and X509_TRUST_MAX. So expand
that into its explicit form.
ok jsing
|
|
|
|
|
|
| |
They are now unused and will join the exodus to the attic in the next bump.
ok jsing
|
|
|
|
| |
CID 477172
|
|
|
|
|
| |
Make a few checks against 0 explicit to reduce noise in an upcoming diff
and tiny KNF tweaks.
|
| |
|
|
|
|
|
|
|
|
| |
This is pretty much identical to the X509_PURPOSE case: remove the stack
used for extending and overriding the trust table and make X509_TRUST_add()
always fail. Simplify some other bits accordingly.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Another complication of dubious value that nobody's ever used. crl_init(),
crl_free() and the meth_data are dead weight, as are their accessors.
Inline def_crl_verify() in X509_CRL_verify() so that the latter becomes
the trivial wrapper of ASN1_item_verify() that one would expect it to be.
It is quite unclear what kind of customization would make sense here...
def_crl_lookup() is renamed into crl_lookup() and its two callers,
X509_CRL_lookup_by_{serial,cert}(), are moved below it so that we
don't need a prototype.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Another bit of global state without lock protection. The by now familiar
complications of a stack to make this user configurable, which, of course,
no one ever did. The table is not currently const, and the API exposes its
entries directly, so anyone can modify it. This fits very well with the
safety guarantees of Rust's 'static lifetime, which is how rust-openssl
exposes it (for no good reason).
Remove the stack and make the X509_PURPOSE_add() API always fail.
Simplify the other bits accordingly.
In addition, this API inflicts the charming difference between purpose
identifiers and purpose indexes (the former minus one) onto the user.
Neither of the two obvious solutions to avoid this trap seems to have
crossed the implementer's mind.
ok jsing
|
| |
|
|
|
|
| |
requested by jsing
|
|
|
|
|
|
|
|
|
| |
If all you have is OBJ_bsearch_(), everything looks like a nail. This
changes a binary search over a list of 12 elements with a lookup via
a switch.
switch suggested by claudio
ok jsing
|
|
|
|
|
| |
This is an internal function and you can't hold the required mutex
to call it anyway since that's internal, too.
|
| |
|
| |
|
|
|
|
|
| |
This way we don't need to cast from BY_DIR * to char * and back in
its only consumer, the lovely by_dir.
|
| |
|
|
|
|
|
|
|
| |
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
|