summaryrefslogtreecommitdiff
path: root/src/lib/libcrypto/x509 (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Hide global _it variables in x509v3.hbeck2024-07-0813-13/+45
| | | | ok tb@
* Pretend to clarify the way ipv6_asc() worksjca2024-07-081-4/+5
| | | | | | | | | Give example IPv6 addresses to clarify what is meant with 1, 2 or 3 zero length elements. tb made me look. perverted, twisted, crippled
* libcrypto: constify most error string tablestb2024-06-241-9/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* x509_conf: rename the merr label into errtb2024-06-241-8/+8
|
* x_all.c: remove a bunch of unnecessary parenthesestb2024-06-191-23/+19
|
* v3_generic_extension() use ASN1_STRING_set0()tb2024-06-181-4/+4
| | | | This aligns it with do_ext_i2d()
* v3_generic_extension() rename the X509_EXTENSIONtb2024-06-181-4/+5
| | | | now that ext is free, we can use it like everywhere else
* Rename 'ext' to 'name' in v3_generic_extension()tb2024-06-181-4/+4
| | | | In this code 'ext' is usually used for an X509_EXTENSION object.
* Make local BIT_STRING_BITNAME variables consttb2024-06-182-5/+5
| | | | | | | | 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.
* x509_conf: rename ext_struc into ext_structtb2024-06-181-16/+16
| | | | requested by jsing on review
* x509_conf: rename all ext_nid to nidtb2024-06-181-19/+19
| | | | There are no nid variables in this file, so no need to disambiguate.
* do_ext_i2d(): move empty line to the proper placetb2024-06-181-2/+2
|
* do_ext_i2d(): malloc -> calloctb2024-06-181-2/+2
| | | | requested by jsing on review
* do_ext_i2d(): populate ext_oct with ASN1_STRING_set0()tb2024-06-181-3/+2
| | | | ok jsing
* do_ext_i2d(): avoid leaks and add some missing error checkingtb2024-06-181-4/+10
| | | | | | | | | | 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
* Indent labels in x509_conf.ctb2024-06-181-3/+3
|
* do_ext_i2d(): make various NULL checks explicittb2024-06-181-5/+5
| | | | ok jsing
* do_ext_i2d(): unwrap a linetb2024-06-181-3/+2
|
* Replace x with x509_exts in X509V3_add1_i2d() and X509V3_get_d2i()tb2024-06-171-14/+16
| | | | requested by jsing on review
* Rewrite X509V3_get_d2i()tb2024-06-171-56/+47
| | | | | | | | | | | | | | | | | 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
* piuid, psuid -> issuerUID, subjectUIDtb2024-06-121-3/+3
|
* Fix non-xsc path in x509_verify_potential_parent()tb2024-06-071-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Clean up and fix X509V3_EXT_add1_i2d()tb2024-05-281-57/+89
| | | | | | | | | | | | | | | | | | | | 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
* x509_v3.c: indent labelstb2024-05-231-4/+4
|
* x509_v3.c: remove an unnecessary elsetb2024-05-231-3/+3
|
* x509_v3.c: consistently call STACK_OF(X509_EXTENSIONS) arguments sktb2024-05-231-12/+12
| | | | (where it doesn't conflict with a local variable)
* x509_v3.c: zap another pointless local variabletb2024-05-231-7/+2
|
* x509_v3.c: add a few empty linestb2024-05-231-1/+9
|
* X509v3_get_ext_by_NID: make obj const, test & assigntb2024-05-231-4/+4
|
* x509_v3.c: remove a pointless local variabletb2024-05-231-5/+3
|
* x509_v3.c: mechanically replace ex with ext and new_ex with new_exttb2024-05-231-42/+42
|
* remove prototypes with no matching functionjsg2024-05-191-2/+1
| | | | feedback and ok tb@
* x509_v3.c: remove superfluous parenthesestb2024-05-161-39/+39
| | | | No change in the generated assembly
* X509_check_akid: zap stray spacetb2024-05-151-2/+2
|
* x509_ext.c: remove unnecessary includestb2024-05-141-5/+1
|
* x509_ext.c: remove lots of extraneous parenthesestb2024-05-141-23/+23
| | | | No change in the generated assembly
* Move X509V3_add_standard_extensions out of the waytb2024-05-111-8/+8
| | | | | | 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.
* Make two NULL checks more explicittb2024-05-111-3/+3
|
* Unwrap a linetb2024-05-111-3/+2
|
* Tiny style tweaks in X509_REQ_add_extension_nid()tb2024-05-091-6/+5
| | | | | | Test & assign and use ret instead of rv. ok jsing
* Streamline X509_REQ_check_private_key() a bittb2024-05-091-16/+17
| | | | | | | Use better variable names, split the success from the error path and return directly rather than using an ok variable. ok jsing
* Zap some extra parentheses in X509_REQ_get_pubkey()tb2024-05-091-4/+4
| | | | ok jsing
* Clean up X509_to_X509_REQ()tb2024-05-091-21/+18
| | | | | | | | | | 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
* Further simplify X509_REQ_get_extensions()tb2024-05-091-6/+4
| | | | | | | 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
* Simplify X509_REQ_get_extensions()tb2024-05-081-22/+13
| | | | | | | | | | | | | 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
* Simplify X509_REQ_extension_nid()tb2024-05-081-11/+3
| | | | | | | | | | | | | 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
* Defang X509_REQ_{s,g}et_extension_nids()tb2024-05-081-15/+20
| | | | | | | | | 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
* Add a comment on abuse of EXFLAG_INVALIDtb2024-04-091-1/+8
| | | | | | | We added things we probably shouldn't have, and so did BoringSSL and OpenSSL. Terrible API is terrible. discussed with jsing
* Remove notBefore and notAfter cacheing.beck2024-04-084-63/+16
| | | | | | | | | | | | | | | | | | | | | | 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@
* Make X509_VERIFY_PARAM_set1_policies() less badtb2024-03-291-22/+34
| | | | | | | | | | | 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