Skip to content

Commit de97a1a

Browse files
bob-beckjogme
bob-beck
authored and
jogme
committed
Don't update the certificate in cache_extensions if we lost the race.
ossl_x509v3_cache_extensions() checks for an unprocessed cert under a read lock, computes the cached values, then takes a write lock to install them. Re-check EXFLAG_SET after acquiring the write lock in case another thread has updated the certificate in the meantime, and if so, do not install our values over theirs. Since losing the race requires freeing the temporary objects, convert the function to a single exit that frees whatever was not installed. This also fixes a pre-existing leak of the temporaries when acquiring the write lock failed. Reviewed-by: Mounir Idrassi <mounir.idrassi@idrix.fr> Reviewed-by: Neil Horman <nhorman@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> Merge-date: Sat Sep 26 12:18:18 2026 Merged-from: #32614
1 parent c35fa56 commit de97a1a

1 file changed

Lines changed: 50 additions & 15 deletions

File tree

‎crypto/x509/v3_purp.c‎

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -456,29 +456,37 @@ int ossl_x509v3_cache_extensions(const X509 *const_x)
456456
uint32_t tmp_ex_kusage;
457457
uint32_t tmp_ex_xkusage;
458458
uint32_t tmp_ex_nscert;
459-
ASN1_OCTET_STRING *tmp_skid;
460-
AUTHORITY_KEYID *tmp_akid;
461-
STACK_OF(GENERAL_NAME) *tmp_altname;
462-
NAME_CONSTRAINTS *tmp_nc;
459+
ASN1_OCTET_STRING *tmp_skid = NULL;
460+
AUTHORITY_KEYID *tmp_akid = NULL;
461+
STACK_OF(GENERAL_NAME) *tmp_altname = NULL;
462+
NAME_CONSTRAINTS *tmp_nc = NULL;
463463
STACK_OF(DIST_POINT) *tmp_crldp = NULL;
464464
X509_SIG_INFO tmp_siginf;
465+
#ifndef OPENSSL_NO_RFC3779
466+
STACK_OF(IPAddressFamily) *tmp_rfc3779_addr = NULL;
467+
struct ASIdentifiers_st *tmp_rfc3779_asid = NULL;
468+
#endif
469+
int ret = 0;
465470

466471
#ifdef tsan_ld_acq
467472
/* Fast lock-free check, see end of the function for details. */
468-
if (tsan_ld_acq((TSAN_QUALIFIER int *)&const_x->ex_cached))
469-
return (const_x->ex_flags & EXFLAG_INVALID) == 0;
473+
if (tsan_ld_acq((TSAN_QUALIFIER int *)&const_x->ex_cached)) {
474+
ret = (const_x->ex_flags & EXFLAG_INVALID) == 0;
475+
goto done;
476+
}
470477
#endif
471478

472479
if (!CRYPTO_THREAD_read_lock(const_x->lock))
473-
return 0;
480+
goto done;
474481
tmp_ex_flags = const_x->ex_flags;
475482
tmp_ex_pcpathlen = const_x->ex_pcpathlen;
476483
tmp_ex_kusage = const_x->ex_kusage;
477484
tmp_ex_nscert = const_x->ex_nscert;
478485

479486
if ((tmp_ex_flags & EXFLAG_SET) != 0) { /* Cert has already been processed */
480487
CRYPTO_THREAD_unlock(const_x->lock);
481-
return (tmp_ex_flags & EXFLAG_INVALID) == 0;
488+
ret = (tmp_ex_flags & EXFLAG_INVALID) == 0;
489+
goto done;
482490
}
483491

484492
ERR_set_mark();
@@ -656,13 +664,11 @@ int ossl_x509v3_cache_extensions(const X509 *const_x)
656664
tmp_ex_flags |= EXFLAG_INVALID;
657665

658666
#ifndef OPENSSL_NO_RFC3779
659-
STACK_OF(IPAddressFamily) *tmp_rfc3779_addr
660-
= X509_get_ext_d2i(const_x, NID_sbgp_ipAddrBlock, &i, NULL);
667+
tmp_rfc3779_addr = X509_get_ext_d2i(const_x, NID_sbgp_ipAddrBlock, &i, NULL);
661668
if (tmp_rfc3779_addr == NULL && i != -1)
662669
tmp_ex_flags |= EXFLAG_INVALID;
663670

664-
struct ASIdentifiers_st *tmp_rfc3779_asid
665-
= X509_get_ext_d2i(const_x, NID_sbgp_autonomousSysNum, &i, NULL);
671+
tmp_rfc3779_asid = X509_get_ext_d2i(const_x, NID_sbgp_autonomousSysNum, &i, NULL);
666672
if (tmp_rfc3779_asid == NULL && i != -1)
667673
tmp_ex_flags |= EXFLAG_INVALID;
668674
#endif
@@ -693,7 +699,16 @@ int ossl_x509v3_cache_extensions(const X509 *const_x)
693699
* do all the updating under a write lock
694700
*/
695701
if (!CRYPTO_THREAD_write_lock(const_x->lock))
696-
return 0;
702+
goto done;
703+
704+
/* See if another thread updated this certificate before we got the write lock. */
705+
if ((const_x->ex_flags & EXFLAG_SET) != 0) { /* Cert has already been processed */
706+
CRYPTO_THREAD_unlock(const_x->lock);
707+
ret = (const_x->ex_flags & EXFLAG_INVALID) == 0;
708+
goto done;
709+
}
710+
711+
/* Otherwise, we have the lock, set the cached fields in the cert. */
697712
((X509 *)const_x)->ex_flags = tmp_ex_flags;
698713
((X509 *)const_x)->ex_pathlen = tmp_ex_pathlen;
699714
((X509 *)const_x)->ex_pcpathlen = tmp_ex_pcpathlen;
@@ -706,19 +721,26 @@ int ossl_x509v3_cache_extensions(const X509 *const_x)
706721
((X509 *)const_x)->ex_nscert = tmp_ex_nscert;
707722
ASN1_OCTET_STRING_free(((X509 *)const_x)->skid);
708723
((X509 *)const_x)->skid = tmp_skid;
724+
tmp_skid = NULL;
709725
AUTHORITY_KEYID_free(((X509 *)const_x)->akid);
710726
((X509 *)const_x)->akid = tmp_akid;
727+
tmp_akid = NULL;
711728
sk_GENERAL_NAME_pop_free(((X509 *)const_x)->altname, GENERAL_NAME_free);
712729
((X509 *)const_x)->altname = tmp_altname;
730+
tmp_altname = NULL;
713731
NAME_CONSTRAINTS_free(((X509 *)const_x)->nc);
714732
((X509 *)const_x)->nc = tmp_nc;
733+
tmp_nc = NULL;
715734
sk_DIST_POINT_pop_free(((X509 *)const_x)->crldp, DIST_POINT_free);
716735
((X509 *)const_x)->crldp = tmp_crldp;
736+
tmp_crldp = NULL;
717737
#ifndef OPENSSL_NO_RFC3779
718738
sk_IPAddressFamily_pop_free(((X509 *)const_x)->rfc3779_addr, IPAddressFamily_free);
719739
((X509 *)const_x)->rfc3779_addr = tmp_rfc3779_addr;
740+
tmp_rfc3779_addr = NULL;
720741
ASIdentifiers_free(((X509 *)const_x)->rfc3779_asid);
721742
((X509 *)const_x)->rfc3779_asid = tmp_rfc3779_asid;
743+
tmp_rfc3779_asid = NULL;
722744
#endif
723745
((X509 *)const_x)->siginf = tmp_siginf;
724746

@@ -733,9 +755,22 @@ int ossl_x509v3_cache_extensions(const X509 *const_x)
733755
CRYPTO_THREAD_unlock(const_x->lock);
734756
if (tmp_ex_flags & EXFLAG_INVALID) {
735757
ERR_raise(ERR_LIB_X509V3, X509V3_R_INVALID_CERTIFICATE);
736-
return 0;
758+
goto done;
737759
}
738-
return 1;
760+
761+
ret = 1;
762+
763+
done:
764+
ASN1_OCTET_STRING_free(tmp_skid);
765+
AUTHORITY_KEYID_free(tmp_akid);
766+
sk_GENERAL_NAME_pop_free(tmp_altname, GENERAL_NAME_free);
767+
NAME_CONSTRAINTS_free(tmp_nc);
768+
sk_DIST_POINT_pop_free(tmp_crldp, DIST_POINT_free);
769+
#ifndef OPENSSL_NO_RFC3779
770+
sk_IPAddressFamily_pop_free(tmp_rfc3779_addr, IPAddressFamily_free);
771+
ASIdentifiers_free(tmp_rfc3779_asid);
772+
#endif
773+
return ret;
739774
}
740775

741776
/*-

0 commit comments

Comments
 (0)