From da6c691d6d417ad413fdc1e7a7a183d005e7fefd Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Tue, 25 Aug 2020 16:46:18 +0200 Subject: [PATCH] check_chain_extensions(): Add check that on empty Subject the SAN must be marked critical Reviewed-by: Kurt Roeckx Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/12478) --- crypto/x509/v3_purp.c | 3 +++ crypto/x509/x509_txt.c | 2 ++ crypto/x509/x509_vfy.c | 10 ++++------ include/openssl/x509_vfy.h | 7 ++++--- include/openssl/x509v3.h | 1 + 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index bced482df4f..2d4098b6292 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -608,6 +608,9 @@ int x509v3_cache_extensions(X509 *x) case NID_subject_key_identifier: x->ex_flags |= EXFLAG_SKID_CRITICAL; break; + case NID_subject_alt_name: + x->ex_flags |= EXFLAG_SAN_CRITICAL; + break; default: break; } diff --git a/crypto/x509/x509_txt.c b/crypto/x509/x509_txt.c index d4bf31685e1..85782a2f865 100644 --- a/crypto/x509/x509_txt.c +++ b/crypto/x509/x509_txt.c @@ -200,6 +200,8 @@ const char *X509_verify_cert_error_string(long n) return "Empty Subject Alternative Name extension"; case X509_V_ERR_CA_BCONS_NOT_CRITICAL: return "Basic Constraints of CA cert not marked critical"; + case X509_V_ERR_EMPTY_SUBJECT_SAN_NOT_CRITICAL: + return "Subject empty and Subject Alt Name extension not critical"; case X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL: return "Authority Key Identifier marked critical"; case X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL: diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 48c0a2d58d0..966733dbb70 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -549,12 +549,10 @@ static int check_chain_extensions(X509_STORE_CTX *ctx) || x->altname == NULL ) && X509_NAME_entry_count(X509_get_subject_name(x)) == 0) ctx->error = X509_V_ERR_SUBJECT_NAME_EMPTY; - /* - * TODO check: If subject naming information is present only in - * the subjectAltName extension, - * then the subject name MUST be an empty sequence - * and the subjectAltName extension MUST be critical. - */ + if (X509_NAME_entry_count(X509_get_subject_name(x)) == 0 + && x->altname != NULL + && (x->ex_flags & EXFLAG_SAN_CRITICAL) == 0) + ctx->error = X509_V_ERR_EMPTY_SUBJECT_SAN_NOT_CRITICAL; /* Check SAN is non-empty according to RFC 5280 section 4.2.1.6 */ if (x->altname != NULL && sk_GENERAL_NAME_num(x->altname) <= 0) ctx->error = X509_V_ERR_EMPTY_SUBJECT_ALT_NAME; diff --git a/include/openssl/x509_vfy.h b/include/openssl/x509_vfy.h index c568b0541c9..53dff234ce1 100644 --- a/include/openssl/x509_vfy.h +++ b/include/openssl/x509_vfy.h @@ -228,9 +228,10 @@ X509_LOOKUP_ctrl_with_libctx((x), X509_L_ADD_STORE, (name), 0, NULL, \ # define X509_V_ERR_MISSING_AUTHORITY_KEY_IDENTIFIER 85 # define X509_V_ERR_MISSING_SUBJECT_KEY_IDENTIFIER 86 # define X509_V_ERR_EMPTY_SUBJECT_ALT_NAME 87 -# define X509_V_ERR_CA_BCONS_NOT_CRITICAL 88 -# define X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL 89 -# define X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL 90 +# define X509_V_ERR_EMPTY_SUBJECT_SAN_NOT_CRITICAL 88 +# define X509_V_ERR_CA_BCONS_NOT_CRITICAL 89 +# define X509_V_ERR_AUTHORITY_KEY_IDENTIFIER_CRITICAL 90 +# define X509_V_ERR_SUBJECT_KEY_IDENTIFIER_CRITICAL 91 /* Certificate verify flags */ # ifndef OPENSSL_NO_DEPRECATED_1_1_0 diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h index 93f9347ac8b..a3ef7ced3a1 100644 --- a/include/openssl/x509v3.h +++ b/include/openssl/x509v3.h @@ -379,6 +379,7 @@ struct ISSUING_DIST_POINT_st { # define EXFLAG_BCONS_CRITICAL 0x10000 # define EXFLAG_AKID_CRITICAL 0x20000 # define EXFLAG_SKID_CRITICAL 0x40000 +# define EXFLAG_SAN_CRITICAL 0x80000 # define KU_DIGITAL_SIGNATURE 0x0080 # define KU_NON_REPUDIATION 0x0040 -- 2.47.3