| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
|
| |
Give example IPv6 addresses to clarify what is meant with 1, 2 or 3 zero
length elements.
tb made me look.
perverted, twisted, crippled
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These constitute the bulk of the remaining global mutable state in
libcrypto. This commit moves most of them into data.rel.ro, leaving
out ERR_str_{functs,libraries,reasons} (which require a slightly
different approach) and SYS_str_reasons which is populated on startup.
The main observation is that if ERR_load_strings() is called with a 0 lib
argument, the ERR_STRING_DATA argument is not actually modified. We could
use this fact to cast away const on the caller side and be done with it.
We can make this cleaner by adding a helper ERR_load_const_strings() which
explicitly avoids the assignment to str->error overriding the error code
already set in the table.
In order for this to work, we need to sprinkle some const in err/err.c.
CMS called ERR_load_strings() with non-0 lib argument, but this didn't
actually modify the error data since it ored in the value already stored
in the table.
Annoyingly, we need to cast const away once, namely in the call to
lh_insert() in int_err_set_item(). Fixing this would require changing
the public API and is going to be tricky since it requires that the
LHASH_DOALL_FN_* types adjust.
ok jsing
|
| |
|
| |
|
|
|
|
| |
This aligns it with do_ext_i2d()
|
|
|
|
| |
now that ext is free, we can use it like everywhere else
|
|
|
|
| |
In this code 'ext' is usually used for an X509_EXTENSION object.
|
|
|
|
|
|
|
|
| |
There's no reason for them not to be const. This is a piece of a larger
diff that I carry in several of my trees to move more things to rodata
or relro. The full diff requires a change to a public header and it's
very annoying to have to 'make includes' and recompile the entire lib
all the time when hopping from tree to tree.
|
|
|
|
| |
requested by jsing on review
|
|
|
|
| |
There are no nid variables in this file, so no need to disambiguate.
|
| |
|
|
|
|
| |
requested by jsing on review
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
If ASN1_OCTET_STRING_new() failed, ext_der would be leaked, fix this.
If i2d(foo, NULL) succeeded, the same is not guaranteed for the second
with appropriately sized buffer since i2d() may make further allocations
internally. So use the proper error check. Also transfer the ownership of
ext_der to the octet string to avoid a now possible double free.
ok jsing
|
| |
|
|
|
|
| |
ok jsing
|
| |
|
|
|
|
| |
requested by jsing on review
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This API is wrapped by nine *_get{,1}_ext_d2i() functions and they all
have the same defect: if an idx variable is passed in, multiple extensions
are handled incorrectly.
Clean up the mess that was the current implementation by replacing the
reimplementation of X509v3_get_ext_by_NID() with extra twists by actual
calls to the real thing. This way the madness is implemented explicitly
and can be explained in comments. The code still gets shorter.
In brief: always call this API with a known nid, pass crit, and a NULL idx.
If NULL is returned, crit != -1 is an error (malformed cert or allocation
failure).
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The combination of two bugs made this unexpectedly work as intended. To
appreciate this, let's first note that
a) check_issued(..., child, parent) checks if child was issued by parent.
b) X509_check_issued(child, parent) checks if parent was issued by child.
Now like in the real world, b) will only be true in unusual circumstances
(child is known not to be self-issued at this point). X509_check_issued()
fails by returning something different from X509_V_OK, so
return X509_check_issued(child, parent) != X509_V_OK;
will return true if child was issued by parent since then parent was indeed
not issued by child. On the other hand, if child was not issued by parent,
the verifier will notice elsewhere, e.g., in a signature check.
Fix this by reversing the order of child and parent in the above return
line and check for equality instead. This is nearly impossible to detect
in regress.
ok beck
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When looking at this code I noticed a few leaks. Fixing those leaks
was straightforward, but following the code was really hard.
This attempts to make the logic a bit clearer. In short, there are
6 mutually exclusive modes for this function (passed in the variable
aptly called flags). The default mode is to append the extension of
type nid and to error if such an extension already exists. Then there
are other modes with varying degree of madness.
The existing code didn't make X509V3_ADD_REPLACE explicit, which is
confusing. Operations 6-15 would all be treated like X509V3_ADD_REPLACE
due to the way the function was written. Handle the supported operations
via a switch and error for operations 6-15. This and the elimination
of leaks are the only changes of behavior, as validated by relatively
extensive test coverage.
ok jsing
|
| |
|
| |
|
|
|
|
| |
(where it doesn't conflict with a local variable)
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
| |
feedback and ok tb@
|
|
|
|
| |
No change in the generated assembly
|
| |
|
| |
|
|
|
|
| |
No change in the generated assembly
|
|
|
|
|
|
| |
This function is only used by OpenLDAP and it's been a noop since
forever. It has no business to be squeezed in between a number of
other, quite unrelated functions. It's distracting.
|
| |
|
| |
|
|
|
|
|
|
| |
Test & assign and use ret instead of rv.
ok jsing
|
|
|
|
|
|
|
| |
Use better variable names, split the success from the error path and
return directly rather than using an ok variable.
ok jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
| |
Use better variable names. X509_REQ_new() sets the version to the only
specified version, so there is no point to set it. Extract the subject
name, then assign to make it more obvious that we error happens if the
cert has a missing subject. Switch to X509_get0_pubkey() to avoid some
strange dance with a strangely named variable to adjust the refcount.
ok jsing
|
|
|
|
|
|
|
| |
Instead of inlining a poor version of ASN1_TYPE_unpack_sequence() with
missing error checks, just call the real thing. It's safer and simpler.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Now that we know the two OIDs we need to look for when checking for the
extension list attribute in a certification request, we can simplify this
quite a bit. There is one change of behavior. Attribute value sets are not
supposed to be empty and it makes no sense to return an empty stack of
extensions in that case, return NULL instead, matching BoringSSL.
This removes last use of ext_nids and ext_nid_list[], so these two bits
of unprotected global mutable state can now join the party in the attic.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Now that the global ext_nids[] array can no longer be modified by the
application, we can simplify this by returning the two possible NIDs
that we accept in the extension list attribute in PKCS#10 certification
requests.
The year is 2024. This API is entirely unused by the ecosystem. Well not
entirely! One small village of indomitable rare API use still holds out
against the cleansers. You may have guessed it: security/xca.
ok jsing
|
|
|
|
|
|
|
|
|
| |
These fiddle with unprotected global state, so aren't thread safe and
of course there was no good reason to have this API in the first place.
Nothing uses it, so it becomes a noop and will be removed in the next
major bump.
ok jsing
|
|
|
|
|
|
|
| |
We added things we probably shouldn't have, and so did BoringSSL and
OpenSSL. Terrible API is terrible.
discussed with jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This cache was added because our time conversion used timegm()
and gmtime() which aren't very cheap. These calls were noticably
expensive when profiling things like rpki-client which do many
X.509 validations.
Now that we convert times using julien seconds from the unix
epoch, BoringSSL style, instead of a julien days from a
Byzantine date, we no longer use timegm() and gmtime().
Since the julien seconds calculaitons are cheap for conversion,
we don't need to bother caching this, it doesn't have a noticable
performance impact.
While we are at this correct a bug where
x509_verify_asn1_time_to_time_t was not NULL safe.
Tested for performance regressions by tb@ and job@
ok tb@ job@
|
|
|
|
|
|
|
|
|
|
|
| |
If any OBJ_dup() fails along the way, a partially copied policy stack
would remain on the params object. This makes no sense. Implement and
use an sk_ASN1_OBJECT_deep_copy(), that copies the full stack or else
returns NULL.
Remove unnecessary NULL check and streamline some other logic.
ok jsing
|