Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling certificate verification using CRL-DP extension #1003

Merged
merged 3 commits into from
Apr 15, 2024
Merged

Enabling certificate verification using CRL-DP extension #1003

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
264ea74
Enable certification verification using CRL-DP
fmarco76 Apr 10, 2024
332041b
Rename enableOCSP to enableRevocationCheck
fmarco76 Apr 11, 2024
ff5589a
Add revocation check fallback for PKIX based verification
fmarco76 Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changes/v5.6.0/Parameter-Changes.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
= Paramater Changes =

== Rename enableOCSP to enableRevocationCheck ==

The `enableOCSP` param has been deprecated. Use `enableRevocationCheck` instead.
22 changes: 13 additions & 9 deletions native/src/main/native/org/mozilla/jss/ssl/common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -936,24 +936,28 @@ JSSL_verifyCertPKIXInternal(CERTCertificate *cert,
CERTVerifyLog *log, SECCertificateUsage *usage,
CERTCertList *trustedCertList)
{
/* Put the first set of possible flags internally here first. Later
* there could be a more complete list to choose from; for now we only
* support our hard core fetch AIA OCSP policy. Note that we disable
* CRL fetching as Dogtag doesn't support it. Additionally, enable OCSP
* checking on the chained CA certificates. Since NSS/PKIX's
* CERT_GetClassicOCSPEnabledHardFailurePolicy doesn't do what we want,
* we construct the policy ourselves. */
/* Since NSS/PKIX's CERT_GetClassicOCSPEnabledHardFailurePolicy doesn't
* do what we want, we construct the policy ourselves.
*
* When enabled the checking on the chained CA certificates.
* With this policy the verification process does:
* - if one between AIA and CRL-DP is present then it will be used;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion to rephrase : "- if only an AIA or an CRL-DP is present, the respective validation method is used;"

* - if AIA and CRL-DP are both presents only AIA is used and in case
* freshin formation cannot be retrieved it fails the validation;
Copy link
Contributor

@edewata edewata Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a typo: fresh information.

With this PR what would be the verification order, CRL-DP first or AIA first?

If the AIA check is done first but the OCSP responder is not available, can we fallback to CRL-DP?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFTR, @fmarco76 explained that the order is defined in ocsp_Enabled_Hard_Policy_Method_Preference, and currently it's configured to do OCSP first (so CRL is implicitly the second).

Also, it seems like it's not possible to define a single validation policy that can fallback from OCSP to CRL without affecting individual OCSP or CRL validations. A possible workaround is to define separate policies based on which extensions are present in the cert itself, but this likely requires more significant changes.

Regardless, the description above correctly describes the policy based on the changes in this PR, so feel free to update/merge. Thanks!

* - it no AIA and CRL-DP are present no revocation check is performed.*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo. "if" instead of "it"

PRUint64 ocsp_Enabled_Hard_Policy_LeafFlags[2] = {
/* crl */
CERT_REV_M_DO_NOT_TEST_USING_THIS_METHOD,
CERT_REV_M_TEST_USING_THIS_METHOD |
CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO,
/* ocsp */
CERT_REV_M_TEST_USING_THIS_METHOD |
CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO
};

PRUint64 ocsp_Enabled_Hard_Policy_ChainFlags[2] = {
/* crl */
CERT_REV_M_DO_NOT_TEST_USING_THIS_METHOD,
CERT_REV_M_TEST_USING_THIS_METHOD |
CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO,
/* ocsp */
CERT_REV_M_TEST_USING_THIS_METHOD |
CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO
Expand Down
14 changes: 11 additions & 3 deletions tomcat/src/main/java/org/dogtagpki/jss/tomcat/Http11NioProtocol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,20 @@ public void setServerCertNickFile(String serverCertNickFile) {
tomcatjss.setServerCertNickFile(serverCertNickFile);
}

public boolean getEnabledOCSP() {
return tomcatjss.getEnableOCSP();
public boolean getEnableOCSP() {
return tomcatjss.getEnableRevocationCheck();
}

public void setEnableOCSP(boolean enableOCSP) {
tomcatjss.setEnableOCSP(enableOCSP);
tomcatjss.setEnableRevocationCheck(enableOCSP);
}

public boolean getEnableRevocationCheck() {
return tomcatjss.getEnableRevocationCheck();
}

public void setEnableRevocationCheck(boolean enableRevocationCheck) {
tomcatjss.setEnableRevocationCheck(enableRevocationCheck);
}

public String getOcspResponderURL() {
Expand Down
46 changes: 27 additions & 19 deletions tomcat/src/main/java/org/dogtagpki/jss/tomcat/TomcatJSS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public class TomcatJSS implements SSLSocketListener {
boolean requireClientAuth;
boolean wantClientAuth;

boolean enableOCSP;
boolean enableRevocationCheck;
String ocspResponderURL;
String ocspResponderCertNickname;
int ocspCacheSize = 1000; // entries
Expand Down Expand Up @@ -183,12 +183,12 @@ public boolean getWantClientAuth() {
return wantClientAuth;
}

public boolean getEnableOCSP() {
return enableOCSP;
public boolean getEnableRevocationCheck() {
return enableRevocationCheck;
}

public void setEnableOCSP(boolean enableOCSP) {
this.enableOCSP = enableOCSP;
public void setEnableRevocationCheck(boolean enableRevocationCheck) {
this.enableRevocationCheck = enableRevocationCheck;
}

public String getOcspResponderURL() {
Expand Down Expand Up @@ -269,7 +269,11 @@ public void loadJSSConfig(Properties config) {

String enableOCSPProp = config.getProperty("enableOCSP");
if (enableOCSPProp != null)
setEnableOCSP(Boolean.parseBoolean(enableOCSPProp));
setEnableRevocationCheck(Boolean.parseBoolean(enableOCSPProp));

String enableRevocationCheckProp = config.getProperty("enableRevocationCheck");
if (enableRevocationCheckProp != null)
setEnableRevocationCheck(Boolean.parseBoolean(enableRevocationCheckProp));
Comment on lines +274 to +276
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, we should also check enableOCSP param for backward compatibility.


String ocspResponderURLProp = config.getProperty("ocspResponderURL");
if (ocspResponderURLProp != null)
Expand Down Expand Up @@ -328,31 +332,35 @@ public void loadTomcatConfig(Document document) throws XPathExpressionException
}

String certDbProp = connector.getAttribute("certdbDir");
if (certDbProp != null)
if (StringUtils.isNotEmpty(certDbProp))
setCertdbDir(certDbProp);

String passwordClassProp = connector.getAttribute("passwordClass");
if (passwordClassProp != null)
if (StringUtils.isNotEmpty(passwordClassProp))
setPasswordClass(passwordClassProp);

String passwordFileProp = connector.getAttribute("passwordFile");
if (passwordFileProp != null)
if (StringUtils.isNotEmpty(passwordFileProp))
setPasswordFile(passwordFileProp);

String serverCertNickFileProp = connector.getAttribute("serverCertNickFile");
if (serverCertNickFileProp != null)
if (StringUtils.isNotEmpty(serverCertNickFileProp))
setServerCertNickFile(serverCertNickFileProp);

String enableOCSPProp = connector.getAttribute("enableOCSP");
if (enableOCSPProp != null)
setEnableOCSP(Boolean.parseBoolean(enableOCSPProp));
if (StringUtils.isNotEmpty(enableOCSPProp))
setEnableRevocationCheck(Boolean.parseBoolean(enableOCSPProp));

String enableRevocationCheckProp = connector.getAttribute("enableRevocationCheck");
if (StringUtils.isNotEmpty(enableRevocationCheckProp))
setEnableRevocationCheck(Boolean.parseBoolean(enableRevocationCheckProp));

String ocspResponderURLProp = connector.getAttribute("ocspResponderURL");
if (ocspResponderURLProp != null)
if (StringUtils.isNotEmpty(ocspResponderURLProp))
setOcspResponderURL(ocspResponderURLProp);

String ocspResponderCertNicknameProp = connector.getAttribute("ocspResponderCertNickname");
if (ocspResponderCertNicknameProp != null)
if (StringUtils.isNotEmpty(ocspResponderCertNicknameProp))
setOcspResponderCertNickname(ocspResponderCertNicknameProp);

String ocspCacheSizeProp = connector.getAttribute("ocspCacheSize");
Expand Down Expand Up @@ -469,7 +477,7 @@ public void init() throws KeyDatabaseException, CertDatabaseException, GeneralSe
logger.debug("wantClientAuth: {}", wantClientAuth);

if (requireClientAuth || wantClientAuth) {
configureOCSP();
configureRevocationCheck();
}

// 12 hours = 43200 seconds
Expand Down Expand Up @@ -549,12 +557,12 @@ public CryptoToken getToken(String tag) throws NoSuchTokenException {
return null;
}

public void configureOCSP() throws GeneralSecurityException, ConfigurationException {
public void configureRevocationCheck() throws GeneralSecurityException, ConfigurationException {

logger.info("configuring OCSP");
logger.info("configuring Revocation Check");

logger.debug("enableOCSP: {}", enableOCSP);
if (!enableOCSP) {
logger.debug("enableCertificateCheck: {}", enableRevocationCheck);
if (!enableRevocationCheck) {
return;
}

Expand Down
Loading