| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| |
|
|
| |
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
|
| |
|
|
|
|
| |
Streamline some checks and use more idiomatic sk_push() error check
ok jsing
|
| | |
|
| |
|
|
|
|
| |
No need for an inconsistently named local variable and a ternary operator.
ok jsing
|
| |
|
|
| |
No change in generated assembly
|
| |
|
|
| |
No change in generated assembly
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
The toolkit aspect bites again. Lots of invalid CRLs and CSRs are produced
because people neither read the RFCs nor does the toolkit check anything it
is fed. Reviewers apparently also aren't capable of remembering that they
have three copy-pasted versions of the same API and that adding a version
check to one of the might suggest adding one for the other two.
This requires ruby-openssl 20240326p0 to pass
ok beck job jsing
|
| |
|
|
| |
quoth the muppet "yes I know this is horrible!"
|
| |
|
|
| |
ok tb@
|
| |
|
|
|
|
| |
Reported by David Benjamin (BoringSSL)
OK tb@
|
| |
|
|
| |
looked over by jsing
|
| |
|
|
|
|
|
|
| |
After peeling off enough layers, the entire wacky abstraction turns out
to be nothing but dispatching from a trust_id to a trust handler and
passing the appropriate nid and the cert.
ok beck jsing
|
| |
|
|
| |
ok tb@
|
