Commit dc01d6cfa1 for openssl.org
commit dc01d6cfa1211a2ca9ff793e4381c9ad268ec996
Author: Daniel Kubec <kubec@openssl.org>
Date: Sat Jan 24 20:50:42 2026 +0100
ASN.1: Raise additional errors in crl_set_issuers()
Additional ASN.1 parsing errors are now raised to the error stack,
allowing invalid CRLs to be rejected early with detailed error messages.
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
MergeDate: Tue Feb 3 09:02:15 2026
(Merged from https://github.com/openssl/openssl/pull/29750)
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 7314d30dbd..a0c93a750c 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -8,7 +8,6 @@
*/
#include <stdio.h>
-#include "internal/cryptlib.h"
#include <openssl/asn1t.h>
#include <openssl/x509.h>
#include "crypto/x509.h"
@@ -80,7 +79,6 @@ ASN1_SEQUENCE_enc(X509_CRL_INFO, enc, crl_inf_cb) = {
static int crl_set_issuers(X509_CRL *crl)
{
-
int i, j;
GENERAL_NAMES *most_recent_issuer, *gtmp;
STACK_OF(X509_REVOKED) *revoked;
@@ -91,6 +89,8 @@ static int crl_set_issuers(X509_CRL *crl)
*/
if (crl->crl.lastUpdate == NULL) {
crl->flags |= EXFLAG_INVALID;
+ ERR_raise_data(ERR_LIB_ASN1, ASN1_R_ILLEGAL_TIME_VALUE,
+ "lastUpdate in CRL is not well-formed");
return 0;
}
@@ -121,6 +121,8 @@ static int crl_set_issuers(X509_CRL *crl)
*/
if ((rev_date = X509_REVOKED_get0_revocationDate(rev)) == NULL) {
crl->flags |= EXFLAG_INVALID;
+ ERR_raise_data(ERR_LIB_ASN1, ASN1_R_ILLEGAL_TIME_VALUE,
+ "revocationDate in CRL is not well-formed");
return 0;
}
@@ -139,6 +141,8 @@ static int crl_set_issuers(X509_CRL *crl)
*/
if (crl->idp == NULL || !crl->idp->indirectCRL) {
crl->flags |= EXFLAG_INVALID;
+ ERR_raise_data(ERR_LIB_ASN1, ASN1_R_INVALID_VALUE,
+ "CRL Certificate Issuer extension requires Indirect CRL flag to be set");
GENERAL_NAMES_free(gtmp);
return 0;
}
@@ -171,7 +175,6 @@ static int crl_set_issuers(X509_CRL *crl)
rev->reason = CRL_REASON_NONE;
/* Check for critical CRL entry extensions and validate time. */
-
exts = rev->extensions;
for (j = 0; j < sk_X509_EXTENSION_num(exts); j++) {
@@ -186,6 +189,8 @@ static int crl_set_issuers(X509_CRL *crl)
if (nid == NID_invalidity_date) {
if ((inv_date = X509V3_EXT_d2i(ext)) == NULL) {
crl->flags |= EXFLAG_INVALID;
+ ERR_raise_data(ERR_LIB_ASN1, ASN1_R_ILLEGAL_TIME_VALUE,
+ "invalidityDate in CRL is not well-formed");
return 0;
}
ASN1_GENERALIZEDTIME_free(inv_date);
diff --git a/test/crltest.c b/test/crltest.c
index 0d81ac69db..efcb7692ea 100644
--- a/test/crltest.c
+++ b/test/crltest.c
@@ -628,27 +628,23 @@ static int test_crl_cert_issuer_ext(void)
return test;
}
-/*
- * This function clears the error stack before parsing and delegates the actual
- * decoding to CRL_from_strings().
- */
-static X509_CRL *crl_clear_err_parse(const char **pem)
-{
- ERR_clear_error();
- return CRL_from_strings(pem);
-}
-
static int test_crl_date_invalid(void)
{
X509_CRL *tmm = NULL, *tss = NULL, *utc = NULL;
int test = 0;
- test = TEST_ptr_null((tmm = crl_clear_err_parse(kInvalidDateMM)))
- && TEST_true(err_chk(ERR_LIB_ASN1, ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT))
- && TEST_ptr_null((tss = crl_clear_err_parse(kInvalidDateSS)))
- && TEST_true(err_chk(ERR_LIB_ASN1, ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT))
- && TEST_ptr_null((utc = crl_clear_err_parse(kInvalidDateUTC)))
- && TEST_true(err_chk(ERR_LIB_ASN1, ASN1_R_WRONG_TAG));
+ test = TEST_ptr_null((tmm = CRL_from_strings(kInvalidDateMM)))
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT)
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_ILLEGAL_TIME_VALUE)
+ && TEST_err_s("invalidityDate in CRL is not well-formed")
+ && TEST_ptr_null((tss = CRL_from_strings(kInvalidDateSS)))
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_GENERALIZEDTIME_IS_TOO_SHORT)
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_ILLEGAL_TIME_VALUE)
+ && TEST_err_s("invalidityDate in CRL is not well-formed")
+ && TEST_ptr_null((utc = CRL_from_strings(kInvalidDateUTC)))
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_WRONG_TAG)
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_ILLEGAL_TIME_VALUE)
+ && TEST_err_s("invalidityDate in CRL is not well-formed");
X509_CRL_free(tmm);
X509_CRL_free(utc);
diff --git a/test/errtest.c b/test/errtest.c
index 323184d93b..a6c27ef3ab 100644
--- a/test/errtest.c
+++ b/test/errtest.c
@@ -420,6 +420,36 @@ err:
return res;
}
+static int test_error_reason(void)
+{
+ int test;
+
+ ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE);
+ ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+ ERR_raise(ERR_LIB_CRYPTO, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+
+ test = TEST_err_r(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE)
+ && TEST_err_r(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR)
+ && TEST_err_r(ERR_LIB_CRYPTO, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
+
+ return test;
+}
+
+static int test_error_string(void)
+{
+ int test;
+
+ ERR_raise_data(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE, "malloc failure");
+ ERR_raise_data(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR, "internal error");
+
+ test = TEST_err_r(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE)
+ && TEST_err_r(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR)
+ && TEST_err_s("malloc failure")
+ && TEST_err_s("internal error");
+
+ return test;
+}
+
int setup_tests(void)
{
ADD_TEST(preserves_system_error);
@@ -431,5 +461,7 @@ int setup_tests(void)
ADD_TEST(test_marks);
ADD_ALL_TESTS(test_save_restore, 2);
ADD_TEST(test_clear_error);
+ ADD_TEST(test_error_reason);
+ ADD_TEST(test_error_string);
return 1;
}
diff --git a/test/testutil.h b/test/testutil.h
index de4023f761..1a43a96faf 100644
--- a/test/testutil.h
+++ b/test/testutil.h
@@ -379,6 +379,23 @@ int test_mem_ne(const char *, int, const char *, const char *,
int test_true(const char *file, int line, const char *s, int b);
int test_false(const char *file, int line, const char *s, int b);
+/*
+ * Checks whether a specific error reason is present in the error stack.
+ * This function iterates over the current thread's error queue extracting all
+ * pending errors. If any of them match the specified reason code (as returned
+ * by ERR_GET_REASON()), the function returns 1 to indicate that the
+ * corresponding error was found.
+ */
+int test_err_r(const char *file, int line, int lib, int reason);
+
+/*
+ * Checks whether a specific string is present in the error data stack.
+ * This function iterates over the current thread's error queue extracting all
+ * pending errors. If any of them match the specified string the function
+ * returns 1 to indicate that the corresponding error was found.
+ */
+int test_err_s(const char *file, int line, const char *data);
+
/*
* Comparisons between BIGNUMs.
* BIGNUMS can be compared against other BIGNUMs or zero.
@@ -520,6 +537,9 @@ void test_perror(const char *s);
#define TEST_true(a) test_true(__FILE__, __LINE__, #a, (a) != 0)
#define TEST_false(a) test_false(__FILE__, __LINE__, #a, (a) != 0)
+#define TEST_err_r(a, b) test_err_r(__FILE__, __LINE__, a, b)
+#define TEST_err_s(a) test_err_s(__FILE__, __LINE__, a)
+
#define TEST_BN_eq(a, b) test_BN_eq(__FILE__, __LINE__, #a, #b, a, b)
#define TEST_BN_ne(a, b) test_BN_ne(__FILE__, __LINE__, #a, #b, a, b)
#define TEST_BN_lt(a, b) test_BN_lt(__FILE__, __LINE__, #a, #b, a, b)
@@ -661,13 +681,5 @@ X509_CRL *CRL_from_strings(const char **pem);
* into |*out| so we can free it.
*/
BIO *glue2bio(const char **pem, char **out);
-/*
- * Checks whether a specific error reason is present in the error stack.
- * This function iterates over the current thread's error queue using
- * ERR_get_error_all(), extracting all pending errors. If any of them match
- * the specified reason code (as returned by ERR_GET_REASON()), the function
- * returns 1 to indicate that the corresponding error was found.
- */
-int err_chk(int lib, int reason);
#endif /* OSSL_TESTUTIL_H */
diff --git a/test/testutil/load.c b/test/testutil/load.c
index d75e14983e..c3ab1d2700 100644
--- a/test/testutil/load.c
+++ b/test/testutil/load.c
@@ -130,6 +130,7 @@ X509_CRL *CRL_from_strings(const char **pem)
return NULL;
}
+ ERR_clear_error();
crl = PEM_read_bio_X509_CRL(b, NULL, NULL, NULL);
OPENSSL_free(p);
@@ -151,28 +152,10 @@ X509 *X509_from_strings(const char **pem)
return NULL;
}
+ ERR_clear_error();
x = PEM_read_bio_X509(b, NULL, NULL, NULL);
OPENSSL_free(p);
BIO_free(b);
return x;
}
-
-int err_chk(int lib, int reason)
-{
-#if defined(OPENSSL_NO_ERR) || defined(OPENSSL_SMALL_FOOTPRINT)
- return 1;
-#endif
-#if defined(OPENSSL_NO_DEPRECATED_3_0) || defined(OPENSSL_NO_HTTP)
- return 1;
-#endif
-
- unsigned long e;
-
- while ((e = ERR_get_error_all(NULL, NULL, NULL, NULL, NULL))) {
- if (ERR_GET_LIB(e) == lib && ERR_GET_REASON(e) == reason)
- return 1;
- }
-
- return 0;
-}
diff --git a/test/testutil/tests.c b/test/testutil/tests.c
index ee83f8a4a9..7299726e79 100644
--- a/test/testutil/tests.c
+++ b/test/testutil/tests.c
@@ -357,6 +357,97 @@ int test_mem_ne(const char *file, int line, const char *st1, const char *st2,
return 1;
}
+#if defined(OPENSSL_NO_ERR) || defined(OPENSSL_NO_DEPRECATED_3_0) || defined(OPENSSL_SMALL_FOOTPRINT)
+
+int test_err_r(const char *file, int line, int lib, int reason)
+{
+ return 1;
+}
+
+int test_err_s(const char *file, int line, const char *data)
+{
+ return 1;
+}
+
+#else
+
+struct test_err_expect_ctx {
+ struct test_err_expect {
+ char *file, *fn, *data;
+ unsigned long code;
+ int line;
+ } errs[ERR_NUM_ERRORS];
+ int err_count;
+};
+
+static int err_r_cb(int lib, int reason, struct test_err_expect *e)
+{
+ if (ERR_GET_LIB(e->code) == lib && ERR_GET_REASON(e->code) == reason)
+ return 1;
+ return 0;
+}
+
+static int err_s_cb(const char *data, struct test_err_expect *e)
+{
+ if (e->data != NULL && strcmp(e->data, data) == 0)
+ return 1;
+ return 0;
+}
+
+static int test_err_helper(int lib, int reason, const char *str)
+{
+ struct test_err_expect_ctx ctx;
+ int result = 0;
+
+ for (ctx.err_count = 0;; ctx.err_count++) {
+ struct test_err_expect *e = &ctx.errs[ctx.err_count];
+ const char *file = NULL, *fn = NULL, *data = NULL;
+
+ if ((e->code = ERR_get_error_all(&file, &e->line, &fn, &data, NULL)) == 0)
+ break;
+
+ /*
+ * Usage after free won't actually happen in tests, but for greater
+ * robustness, paying for 3 allocations per record in the test is not a
+ * problem and we also gain robustness against future changes in the
+ * error stack.
+ */
+
+ e->file = file != NULL ? OPENSSL_strdup(file) : NULL;
+ e->fn = fn != NULL ? OPENSSL_strdup(fn) : NULL;
+ e->data = data != NULL ? OPENSSL_strdup(data) : NULL;
+ }
+
+ for (int i = 0; i < ctx.err_count; i++) {
+ struct test_err_expect *e = &ctx.errs[i];
+
+ if ((str != NULL ? err_s_cb(str, e) : err_r_cb(lib, reason, e)) == 1)
+ result = 1;
+
+ ERR_new();
+ ERR_set_debug(e->file, e->line, e->fn);
+ ERR_set_error(ERR_GET_LIB(e->code), ERR_GET_REASON(e->code), e->data);
+
+ OPENSSL_free(e->file);
+ OPENSSL_free(e->fn);
+ OPENSSL_free(e->data);
+ }
+
+ return result;
+}
+
+int test_err_r(const char *file, int line, int lib, int reason)
+{
+ return test_err_helper(lib, reason, NULL);
+}
+
+int test_err_s(const char *file, int line, const char *data)
+{
+ return test_err_helper(0, 0, data);
+}
+
+#endif
+
#define DEFINE_BN_COMPARISONS(opname, op, zero_cond) \
int test_BN_##opname(const char *file, int line, \
const char *s1, const char *s2, \
diff --git a/test/x509_internal_test.c b/test/x509_internal_test.c
index ecd6d7651e..155a566263 100644
--- a/test/x509_internal_test.c
+++ b/test/x509_internal_test.c
@@ -519,13 +519,54 @@ static int tests_X509_check_crypto(void)
test = TEST_ptr((rsa_n_neg = X509_from_strings(kRSAModulusNeg)))
&& TEST_ptr_null((pub = X509_get_pubkey(rsa_n_neg)))
- && TEST_true(err_chk(ERR_LIB_EVP, EVP_R_DECODE_ERROR));
+ && TEST_err_r(ERR_LIB_EVP, EVP_R_DECODE_ERROR);
EVP_PKEY_free(pub);
X509_free(rsa_n_neg);
return test;
}
+/* https://github.com/openssl/openssl/issues/11722 */
+static const char *kDistributionPointWrongTag[] = {
+ "-----BEGIN CERTIFICATE-----\n",
+ "MIIDiTCCAnGgAwIBAgIFAO3UVNIwDQYJKoZIhvcNAQELBQAwfTEWMBQGA1UEBhMN\n",
+ "VW5pdGVkTmF0aW9uczEQMA4GA1UECAwHTmV3WW9yazEQMA4GA1UEBwwHTmV3WW9y\n",
+ "azEcMBoGA1UECgwTU29mdHdhcmVFbmdpbmVlcmluZzEQMA4GA1UECwwHVGVzdGlu\n",
+ "ZzEPMA0GA1UEAwwGdW4ub3JnMCIYDzIwMTcwMTIzMDkzMDAwWhgPMjAxODEyMjMw\n",
+ "OTMwMDBaMGwxCzAJBgNVBAYTAlVOMRAwDgYDVQQIEwdOZXdZb3JrMRAwDgYDVQQH\n",
+ "EwdOZXdZb3JrMQ8wDQYDVQQKEwZzdWJPcmcxEzARBgNVBAsTCnN1Yk9yZ1VuaXQx\n",
+ "EzARBgNVBAMTCnN1Yi51bi5vcmcwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK\n",
+ "AoIBAQDBpo9Viz3OQa6vmsyULUFLgznPkB3OX5Yf5XFBnZWoAu2d9qNypidARWUs\n",
+ "DomjtevXKrQApnmuWlxqLV5Q4JGbggeq6UsAwnoXCnH7zhQqInczr0U7rzlUYsZq\n",
+ "H+5haEfV9OewOIFHStAVXyTOqJuEfZOQKZbhhx7TDYg9IpoQisvKB4HbPfi6a6BV\n",
+ "YPAqNFlSfnYsF7sHNztTJXxyRY/KjrBeKVWs3n2+QQaydI+seiDD1GKBhApHrrWo\n",
+ "XtaP4VFbSyPszlRnW0ICAVrMItmt1rJBJlARVRq+gpU0gifmMheNBTWt8js6Ms/i\n",
+ "XeSzBkrQtFsnbVE75qeTrybOxqTXAgMBAAGjHTAbMBkGA1UdHwEB/wQPMA2BCwBS\n",
+ "ZWFzb24uLi4AMA0GCSqGSIb3DQEBCwUAA4IBAQCzwoxTrHgICZeYE7owZxV39BZh\n",
+ "MAHYYzS16/EXdXPZvZFQkL+wMBGkPC82s/3D/4kjHUwDxmmu2jBR8k+vEiV5VMnw\n",
+ "ZcoS22KFNVskk+CBfP0G5/d+ZfFMuW1tE3B1sO7RvYT1MtYt+DryRZ7vvLv7MlQb\n",
+ "sE+le0VjCfZHAZ+D3GqhYNNy+qhKYaHQDg/tfA/J28yyYm1EMzUd//Bao9BbnRxi\n",
+ "p4x2WfCFGB/ZP9BV0VA2KH3qF5M1RAETch/YbWqOIn+LxKomhvQSwQ4DEmRHRu69\n",
+ "loi+aH8qoQ4hb91EeaNb3OCV3azSH8I8RGGZDM2I2fZmgFwZ+5w7rgFjKe6b\n",
+ "-----END CERTIFICATE-----\n",
+ NULL
+};
+
+static int tests_x509_check_dpn(void)
+{
+ X509 *cert = NULL;
+ STACK_OF(DIST_POINT) *crls = NULL;
+ int test, nid = NID_crl_distribution_points;
+
+ test = TEST_ptr((cert = X509_from_strings(kDistributionPointWrongTag)))
+ && TEST_ptr_null((crls = X509_get_ext_d2i(cert, nid, NULL, NULL)))
+ && TEST_err_r(ERR_LIB_ASN1, ASN1_R_WRONG_TAG);
+
+ sk_DIST_POINT_pop_free(crls, DIST_POINT_free);
+ X509_free(cert);
+ return test;
+}
+
int setup_tests(void)
{
ADD_TEST(test_standard_exts);
@@ -533,5 +574,6 @@ int setup_tests(void)
ADD_TEST(tests_X509_PURPOSE);
ADD_TEST(tests_X509_check_time);
ADD_TEST(tests_X509_check_crypto);
+ ADD_TEST(tests_x509_check_dpn);
return 1;
}