From fa4c213ea0e5a37f4a140682d58087c4749f2d54 Mon Sep 17 00:00:00 2001 From: Stepan Broz Date: Fri, 14 Feb 2025 11:24:10 +0100 Subject: [PATCH] Do CA checks to verify authentication by default This is a follow-up to b665b287ff955bbbd9539252ff9f9e2754c3fb48. An attacker that is able to login into a token could bypass authentication by using its own certificate with any valid signature. This change makes the default "ca, signature" with the only way to disable CA check by using "no_ca". This, however, also makes the "none" option disabling CRL and OCSP checks only. --- etc/pam_pkcs11.conf.example.in | 29 +++++++++++++++-------------- src/common/cert_vfy.c | 8 ++++---- src/common/cert_vfy.h | 2 +- src/pam_pkcs11/pam_config.c | 15 ++++++++++----- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/etc/pam_pkcs11.conf.example.in b/etc/pam_pkcs11.conf.example.in index 7ca45b7..0322dfa 100644 --- a/etc/pam_pkcs11.conf.example.in +++ b/etc/pam_pkcs11.conf.example.in @@ -92,21 +92,22 @@ pam_pkcs11 { support_threads = false; # Sets the Certificate verification policy. - # "none" Performs no verification, except (!) the signature - # "ca" Does CA check - # "crl_online" Downloads the CRL form the location given by the - # CRL distribution point extension of the certificate - # "crl_offline" Uses the locally stored CRLs - # "crl_auto" Is a combination of online and offline; it first - # tries to download the CRL from a possibly given CRL - # distribution point and if this fails, uses the local - # CRLs - # "signature" Does a signature check to ensure that private - # and public key matches - # "no_signature" The only value that disables signature check. + # "none" Performs only (!) CA and signature checks + # "ca" Does CA check + # "no_ca" The only value that disables CA check + # "crl_online" Downloads the CRL form the location given by the + # CRL distribution point extension of the certificate + # "crl_offline" Uses the locally stored CRLs + # "crl_auto" Is a combination of online and offline; it first + # tries to download the CRL from a possibly given CRL + # distribution point and if this fails, uses the local + # CRLs + # "signature" Does a signature check to ensure that private + # and public key matches + # "no_signature" The only value that disables signature check # # You can use a combination of ca,crl, and signature flags, or just - # use "none". + # use "none". Use "none,no_ca,no_signature" to disable all checks. cert_policy = ca,signature; # What kind of token? @@ -140,7 +141,7 @@ pam_pkcs11 { support_threads = false; ca_dir = /etc/pam_pkcs11/cacerts; crl_dir = /etc/pam_pkcs11/crls; - cert_policy = signature; + cert_policy = ca,signature; } # Which mappers ( Cert to login ) to use? diff --git a/src/common/cert_vfy.c b/src/common/cert_vfy.c index d16cf3f..dfd8d23 100644 --- a/src/common/cert_vfy.c +++ b/src/common/cert_vfy.c @@ -408,7 +408,7 @@ static X509_STORE * setup_store(cert_policy *policy) { } } /* add needed hash dir pathname entries */ - if ( (policy->ca_policy) && (is_dir(policy->ca_dir)>0) ) { + if ( (!policy->no_ca_policy) && (is_dir(policy->ca_dir)>0) ) { const char *pt=policy->ca_dir; if ( strstr(pt,"file:///")) pt+=8; /* strip url if needed */ DBG1("Adding hash dir '%s' to CACERT checks",policy->ca_dir); @@ -434,7 +434,7 @@ static X509_STORE * setup_store(cert_policy *policy) { } } /* and add file entries to lookup */ - if ( (policy->ca_policy) && (is_file(policy->ca_dir)>0) ) { + if ( (!policy->no_ca_policy) && (is_file(policy->ca_dir)>0) ) { const char *pt=policy->ca_dir; if ( strstr(pt,"file:///")) pt+=8; /* strip url if needed */ DBG1("Adding file '%s' to CACERT checks",policy->ca_dir); @@ -467,7 +467,7 @@ int verify_certificate(X509 * x509, cert_policy *policy) X509_STORE_CTX *ctx = NULL; /* if neither ca nor crl check are requested skip */ - if ( (policy->ca_policy==0) && (policy->crl_policy==CRLP_NONE) ) { + if ( (!policy->no_ca_policy==0) && (policy->crl_policy==CRLP_NONE) ) { DBG("Neither CA nor CRL check requested. CertVrfy() skipped"); return 1; } @@ -489,7 +489,7 @@ int verify_certificate(X509 * x509, cert_policy *policy) #if 0 X509_STORE_CTX_set_purpose(ctx, purpose); #endif - if (policy->ca_policy) { + if (!policy->no_ca_policy) { rv = X509_verify_cert(ctx); if (rv != 1) { X509_STORE_CTX_free(ctx); diff --git a/src/common/cert_vfy.h b/src/common/cert_vfy.h index 00a64c6..9de9c91 100644 --- a/src/common/cert_vfy.h +++ b/src/common/cert_vfy.h @@ -46,7 +46,7 @@ typedef enum { } ocsp_policy_t; struct cert_policy_st { - int ca_policy; + int no_ca_policy; int crl_policy; int no_signature_policy; const char *ca_dir; diff --git a/src/pam_pkcs11/pam_config.c b/src/pam_pkcs11/pam_config.c index 6590478..5dd6364 100644 --- a/src/pam_pkcs11/pam_config.c +++ b/src/pam_pkcs11/pam_config.c @@ -85,7 +85,7 @@ static void display_config (void) { DBG1("crl_dir %s",configuration.policy.crl_dir); DBG1("nss_dir %s",configuration.policy.nss_dir); DBG1("support_threads %d",configuration.support_threads); - DBG1("ca_policy %d",configuration.policy.ca_policy); + DBG1("no_ca_policy %d",configuration.policy.no_ca_policy); DBG1("crl_policy %d",configuration.policy.crl_policy); DBG1("no_signature_policy %d",configuration.policy.no_signature_policy); DBG1("ocsp_policy %d",configuration.policy.ocsp_policy); @@ -179,7 +179,7 @@ static void parse_config_file(void) { if ( !strcmp(policy_list->data,"none") ) { configuration.policy.crl_policy=CRLP_NONE; configuration.policy.ocsp_policy=OCSP_NONE; - configuration.policy.ca_policy=0; + configuration.policy.no_ca_policy=0; configuration.policy.no_signature_policy=0; break; } else if ( !strcmp(policy_list->data,"crl_auto") ) { @@ -191,7 +191,9 @@ static void parse_config_file(void) { } else if ( !strcmp(policy_list->data,"ocsp_on") ) { configuration.policy.ocsp_policy=OCSP_ON; } else if ( !strcmp(policy_list->data,"ca") ) { - configuration.policy.ca_policy=1; + // ignore this setting for legacy reasons + } else if ( !strcmp(policy_list->data,"no_ca") ) { + configuration.policy.no_ca_policy=1; } else if ( !strcmp(policy_list->data,"signature") ) { // ignore this setting for legacy reasons } else if ( !strcmp(policy_list->data,"no_signature") ) { @@ -322,7 +324,7 @@ struct configuration_st *pk_configure( int argc, const char **argv ) { if (strstr(argv[i],"cert_policy=") ) { if (strstr(argv[i],"none")) { configuration.policy.crl_policy=CRLP_NONE; - configuration.policy.ca_policy=0; + configuration.policy.no_ca_policy=0; configuration.policy.no_signature_policy=0; configuration.policy.ocsp_policy=OCSP_NONE; } @@ -339,7 +341,10 @@ struct configuration_st *pk_configure( int argc, const char **argv ) { configuration.policy.ocsp_policy=OCSP_ON; } if (strstr(argv[i],"ca")) { - configuration.policy.ca_policy=1; + // ignore this setting for legacy reasons + } + if (strstr(argv[i],"no_ca")) { + configuration.policy.no_ca_policy=1; } if (strstr(argv[i],"signature")) { // ignore this setting for legacy reasons