Skip to content

Commit 5a6850e

Browse files
bernd-edlingerbernd-edlinger
committed
Fix empty AKID/SKID handling
in all kinds of self-signed and non-self-signed X509, CSR, CRLs. The default, when not defined by config for self-signed X509 is: subjectKeyIdentifier=hash authorityKeyIdentifier=none For non-self-signed X509 it is: subjectKeyIdentifier=hash authorityKeyIdentifier=keyid,issuer For CSR and CRLs the default is: subjectKeyIdentifier=none authorityKeyIdentifier=none
1 parent 15fb99d commit 5a6850e

File tree

9 files changed

+89
-54
lines changed

9 files changed

+89
-54
lines changed

‎apps/ca.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1702,6 +1702,8 @@ static int do_body(X509 **xret, EVP_PKEY *pkey, X509 *x509,
17021702
BIO_printf(bio_err,
17031703
"Warning: Signature key and public key of cert do not match\n");
17041704
}
1705+
if (!add_X509_default_keyids(ret, pkey, &ext_ctx))
1706+
goto end;
17051707

17061708
/* Lets add the extensions, if there are any */
17071709
if (ext_sect) {

‎apps/cmp.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1812,7 +1812,8 @@ static int setup_request_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine)
18121812
if (opt_reqexts != NULL || opt_policies != NULL) {
18131813
if ((exts = sk_X509_EXTENSION_new_null()) == NULL)
18141814
goto oom;
1815-
X509V3_set_ctx(&ext_ctx, NULL, NULL, csr, NULL, X509V3_CTX_REPLACE);
1815+
X509V3_set_ctx(&ext_ctx, NULL, NULL, csr, NULL,
1816+
csr == NULL ? X509V3_CTX_REPLACE : 0);
18161817
X509V3_set_nconf(&ext_ctx, conf);
18171818
if (opt_reqexts != NULL
18181819
&& !X509V3_EXT_add_nconf_sk(conf, &ext_ctx, opt_reqexts, &exts)) {

‎apps/include/apps.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ int init_gen_str(EVP_PKEY_CTX **pctx,
258258
const char *algname, ENGINE *e, int do_param,
259259
OSSL_LIB_CTX *libctx, const char *propq);
260260
int cert_matches_key(const X509 *cert, const EVP_PKEY *pkey);
261+
int add_X509_default_keyids(X509 *x, EVP_PKEY *pkey, X509V3_CTX *ext_ctx);
261262
int do_X509_sign(X509 *x, int force_v1, EVP_PKEY *pkey, const char *md,
262263
STACK_OF(OPENSSL_STRING) *sigopts, X509V3_CTX *ext_ctx);
263264
int do_X509_verify(X509 *x, EVP_PKEY *pkey, STACK_OF(OPENSSL_STRING) *vfyopts);

‎apps/lib/apps.c‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2248,28 +2248,24 @@ static int do_sign_init(EVP_MD_CTX *ctx, EVP_PKEY *pkey,
22482248
}
22492249

22502250
static int adapt_keyid_ext(X509 *cert, X509V3_CTX *ext_ctx,
2251-
const char *name, const char *value, int add_default)
2251+
const char *name, const char *value)
22522252
{
22532253
const STACK_OF(X509_EXTENSION) *exts = X509_get0_extensions(cert);
22542254
X509_EXTENSION *new_ext = X509V3_EXT_nconf(NULL, ext_ctx, name, value);
2255-
int idx, rv = 0;
2255+
ASN1_OCTET_STRING *encoded;
2256+
int disabled, idx, rv = 0;
22562257

22572258
if (new_ext == NULL)
22582259
return rv;
22592260

22602261
idx = X509v3_get_ext_by_OBJ(exts, X509_EXTENSION_get_object(new_ext), -1);
22612262
if (idx >= 0) {
2262-
X509_EXTENSION *found_ext = X509v3_get_ext(exts, idx);
2263-
ASN1_OCTET_STRING *encoded = X509_EXTENSION_get_data(found_ext);
2264-
int disabled = ASN1_STRING_length(encoded) <= 2; /* indicating "none" */
2265-
2266-
if (disabled) {
2267-
X509_delete_ext(cert, idx);
2268-
X509_EXTENSION_free(found_ext);
2269-
} /* else keep existing key identifier, which might be outdated */
2263+
/* keep existing key identifier, which might be outdated */
22702264
rv = 1;
22712265
} else {
2272-
rv = !add_default || X509_add_ext(cert, new_ext, -1);
2266+
encoded = X509_EXTENSION_get_data(new_ext);
2267+
disabled = ASN1_STRING_length(encoded) <= 2; /* indicating "none" */
2268+
rv = disabled || X509_add_ext(cert, new_ext, -1);
22732269
}
22742270
X509_EXTENSION_free(new_ext);
22752271
return rv;
@@ -2285,30 +2281,40 @@ int cert_matches_key(const X509 *cert, const EVP_PKEY *pkey)
22852281
return match;
22862282
}
22872283

2288-
/* Ensure RFC 5280 compliance, adapt keyIDs as needed, and sign the cert info */
2284+
/* Add default keyIDs as needed */
2285+
int add_X509_default_keyids(X509 *cert, EVP_PKEY *pkey, X509V3_CTX *ext_ctx)
2286+
{
2287+
int self_sign = 0;
2288+
int rv = 0;
2289+
2290+
/*
2291+
* Add default SKID before AKID such that AKID can make use of it
2292+
* in case the certificate is self-signed
2293+
*/
2294+
if (X509_get_extension_flags(cert) & EXFLAG_SI)
2295+
self_sign = cert_matches_key(cert, pkey);
2296+
/* Prevent X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER */
2297+
if (!adapt_keyid_ext(cert, ext_ctx, "subjectKeyIdentifier", "hash"))
2298+
goto end;
2299+
/* Prevent X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER */
2300+
if (!adapt_keyid_ext(cert, ext_ctx, "authorityKeyIdentifier",
2301+
self_sign ? "none" : "keyid, issuer"))
2302+
goto end;
2303+
rv = 1;
2304+
end:
2305+
return rv;
2306+
}
2307+
2308+
/* Ensure RFC 5280 compliance, and sign the cert info */
22892309
int do_X509_sign(X509 *cert, int force_v1, EVP_PKEY *pkey, const char *md,
22902310
STACK_OF(OPENSSL_STRING) *sigopts, X509V3_CTX *ext_ctx)
22912311
{
22922312
EVP_MD_CTX *mctx = EVP_MD_CTX_new();
2293-
int self_sign;
22942313
int rv = 0;
22952314

22962315
if (!force_v1) {
22972316
if (!X509_set_version(cert, X509_VERSION_3))
22982317
goto end;
2299-
2300-
/*
2301-
* Add default SKID before AKID such that AKID can make use of it
2302-
* in case the certificate is self-signed
2303-
*/
2304-
/* Prevent X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER */
2305-
if (!adapt_keyid_ext(cert, ext_ctx, "subjectKeyIdentifier", "hash", 1))
2306-
goto end;
2307-
/* Prevent X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER */
2308-
self_sign = cert_matches_key(cert, pkey);
2309-
if (!adapt_keyid_ext(cert, ext_ctx, "authorityKeyIdentifier",
2310-
"keyid, issuer", !self_sign))
2311-
goto end;
23122318
}
23132319
/* May add further measures for ensuring RFC 5280 compliance, see #19805 */
23142320

‎apps/req.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,8 @@ int req_main(int argc, char **argv)
830830
BIO_printf(bio_err,
831831
"Warning: Signature key and public key of cert do not match\n");
832832
}
833+
if (!x509v1 && !add_X509_default_keyids(new_x509, issuer_key, &ext_ctx))
834+
goto end;
833835
X509V3_set_nconf(&ext_ctx, req_conf);
834836

835837
/* Add extensions */

‎apps/x509.c‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -848,23 +848,6 @@ int x509_main(int argc, char **argv)
848848
}
849849
}
850850

851-
X509V3_set_ctx(&ext_ctx, issuer_cert, x, NULL, NULL, X509V3_CTX_REPLACE);
852-
/* prepare fallback for AKID, but only if issuer cert equals subject cert */
853-
if (CAfile == NULL) {
854-
if (!X509V3_set_issuer_pkey(&ext_ctx, privkey))
855-
goto err;
856-
}
857-
if (extconf != NULL && !x509toreq) {
858-
X509V3_set_nconf(&ext_ctx, extconf);
859-
if (!X509V3_EXT_add_nconf(extconf, &ext_ctx, extsect, x)) {
860-
BIO_printf(bio_err,
861-
"Error adding extensions from section %s\n", extsect);
862-
goto err;
863-
}
864-
}
865-
866-
/* At this point the contents of the certificate x have been finished. */
867-
868851
pkey = X509_get0_pubkey(x);
869852
if ((print_pubkey != 0 || modulus != 0) && pkey == NULL) {
870853
BIO_printf(bio_err, "Error getting public key\n");
@@ -882,6 +865,7 @@ int x509_main(int argc, char **argv)
882865
}
883866
if ((rq = x509_to_req(x, ext_copy, ext_names)) == NULL)
884867
goto err;
868+
X509V3_set_ctx(&ext_ctx, NULL, NULL, rq, NULL, X509V3_CTX_REPLACE);
885869
if (extconf != NULL) {
886870
X509V3_set_nconf(&ext_ctx, extconf);
887871
if (!X509V3_EXT_REQ_add_nconf(extconf, &ext_ctx, extsect, rq)) {
@@ -916,9 +900,34 @@ int x509_main(int argc, char **argv)
916900
goto err;
917901
}
918902

903+
X509V3_set_ctx(&ext_ctx, xca, x, NULL, NULL, X509V3_CTX_REPLACE);
904+
if (!add_X509_default_keyids(x, CAkey, &ext_ctx))
905+
goto err;
906+
if (extconf != NULL) {
907+
X509V3_set_nconf(&ext_ctx, extconf);
908+
if (!X509V3_EXT_add_nconf(extconf, &ext_ctx, extsect, x)) {
909+
BIO_printf(bio_err,
910+
"Error adding extensions from section %s\n", extsect);
911+
goto err;
912+
}
913+
}
919914
if (!do_X509_sign(x, 0, CAkey, digest, sigopts, &ext_ctx))
920915
goto err;
921916
} else if (privkey != NULL) {
917+
X509V3_set_ctx(&ext_ctx, x, x, NULL, NULL, X509V3_CTX_REPLACE);
918+
/* prepare fallback for AKID, but only if issuer cert equals subject cert */
919+
if (!X509V3_set_issuer_pkey(&ext_ctx, privkey))
920+
goto err;
921+
if (!add_X509_default_keyids(x, privkey, &ext_ctx))
922+
goto err;
923+
if (extconf != NULL) {
924+
X509V3_set_nconf(&ext_ctx, extconf);
925+
if (!X509V3_EXT_add_nconf(extconf, &ext_ctx, extsect, x)) {
926+
BIO_printf(bio_err,
927+
"Error adding extensions from section %s\n", extsect);
928+
goto err;
929+
}
930+
}
922931
if (!do_X509_sign(x, 0, privkey, digest, sigopts, &ext_ctx))
923932
goto err;
924933
}

‎crypto/cmp/cmp_msg.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ static int add_extensions(STACK_OF(X509_EXTENSION) **target,
157157
for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
158158
X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, i);
159159
ASN1_OBJECT *obj = X509_EXTENSION_get_object(ext);
160+
ASN1_OCTET_STRING *encoded = X509_EXTENSION_get_data(ext);
160161
int idx = X509v3_get_ext_by_OBJ(*target, obj, -1);
161162

162163
/* Does extension exist in target? */
@@ -167,6 +168,15 @@ static int add_extensions(STACK_OF(X509_EXTENSION) **target,
167168
idx = X509v3_get_ext_by_OBJ(*target, obj, -1);
168169
} while (idx != -1);
169170
}
171+
switch (OBJ_obj2nid(obj)) {
172+
case NID_subject_key_identifier:
173+
case NID_authority_key_identifier:
174+
if (ASN1_STRING_length(encoded) <= 2) /* indicating "none" */
175+
continue;
176+
break;
177+
default:
178+
break;
179+
}
170180
if (!X509v3_add_ext(target, ext, -1))
171181
return 0;
172182
}
@@ -352,6 +362,8 @@ OSSL_CRMF_MSG *OSSL_CMP_CTX_setup_CRM(OSSL_CMP_CTX *ctx, int for_KUR, int rid)
352362
if (ctx->p10CSR != NULL
353363
&& (exts = X509_REQ_get_extensions(ctx->p10CSR)) == NULL)
354364
goto err;
365+
if (exts == NULL && (exts = sk_X509_EXTENSION_new_null()) == NULL)
366+
goto err;
355367
if (!ctx->SubjectAltName_nodefault && !HAS_SAN(ctx) && refcert != NULL
356368
&& (default_sans = X509V3_get_d2i(X509_get0_extensions(refcert),
357369
NID_subject_alt_name, NULL, NULL))

‎crypto/x509/v3_akid.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,8 @@ static AUTHORITY_KEYID *v2i_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
179179
X509_PUBKEY_free(pubkey);
180180
} else {
181181
i = X509_get_ext_by_NID(issuer_cert, NID_subject_key_identifier, -1);
182-
if (i >= 0 && (ext = X509_get_ext(issuer_cert, i)) != NULL) {
182+
if (i >= 0 && (ext = X509_get_ext(issuer_cert, i)) != NULL)
183183
ikeyid = X509V3_EXT_d2i(ext);
184-
if (ASN1_STRING_length(ikeyid) == 0) /* indicating "none" */ {
185-
ASN1_OCTET_STRING_free(ikeyid);
186-
ikeyid = NULL;
187-
}
188-
}
189184
}
190185
if (keyid == 2 && ikeyid == NULL) {
191186
ERR_raise(ERR_LIB_X509V3, X509V3_R_UNABLE_TO_GET_ISSUER_KEYID);

‎crypto/x509/v3_conf.c‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,18 @@ int X509V3_EXT_add_nconf_sk(CONF *conf, X509V3_CTX *ctx, const char *section,
340340
val->name, val->value)) == NULL)
341341
return 0;
342342
if (sk != NULL) {
343-
if (ctx->flags == X509V3_CTX_REPLACE)
343+
if (ctx->flags & X509V3_CTX_REPLACE)
344344
delete_ext(*sk, ext);
345-
if (X509v3_add_ext(sk, ext, -1) == NULL) {
346-
X509_EXTENSION_free(ext);
347-
return 0;
345+
if ((ctx->flags & X509V3_CTX_REPLACE) == 0
346+
|| (i != akid && i != skid)
347+
|| ASN1_STRING_length(X509_EXTENSION_get_data(ext)) > 2) {
348+
if (X509v3_add_ext(sk, ext, -1) == NULL) {
349+
X509_EXTENSION_free(ext);
350+
return 0;
351+
}
352+
} else if (*sk != NULL && sk_X509_EXTENSION_num(*sk) == 0) {
353+
sk_X509_EXTENSION_free(*sk);
354+
*sk = NULL;
348355
}
349356
}
350357
X509_EXTENSION_free(ext);

0 commit comments

Comments
 (0)