Slair1 commented on a change in pull request #4852:
URL: https://github.com/apache/cloudstack/pull/4852#discussion_r600518130



##########
File path: 
plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java
##########
@@ -79,57 +79,58 @@ public void checkClientTrusted(final X509Certificate[] 
certificates, final Strin
         if (LOG.isDebugEnabled()) {
             printCertificateChain(certificates, s);
         }
-        if (!authStrictness) {
-            return;
-        }
-        if (certificates == null || certificates.length < 1 || certificates[0] 
== null) {
-            throw new CertificateException("In strict auth mode, 
certificate(s) are expected from client:" + clientAddress);
-        }
-        final X509Certificate primaryClientCertificate = certificates[0];
-
-        // Revocation check
-        final BigInteger serialNumber = 
primaryClientCertificate.getSerialNumber();
-        if (serialNumber == null || crlDao.findBySerial(serialNumber) != null) 
{
-            final String errorMsg = String.format("Client is using revoked 
certificate of serial=%x, subject=%s from address=%s",
-                    primaryClientCertificate.getSerialNumber(), 
primaryClientCertificate.getSubjectDN(), clientAddress);
-            LOG.error(errorMsg);
-            throw new CertificateException(errorMsg);
-        }
 
-        // Validity check
-        if (!allowExpiredCertificate) {
-            try {
-                primaryClientCertificate.checkValidity();
-            } catch (final CertificateExpiredException | 
CertificateNotYetValidException e) {
-                final String errorMsg = String.format("Client certificate has 
expired with serial=%x, subject=%s from address=%s",
+        final X509Certificate primaryClientCertificate = (certificates != null 
&& certificates.length > 0 && certificates[0] != null) ? certificates[0] : null;
+
+        if (authStrictness) {
+            if (primaryClientCertificate == null) {
+                throw new CertificateException("In strict auth mode, 
certificate(s) are expected from client:" + clientAddress);
+            }
+
+            // Revocation check
+            final BigInteger serialNumber = 
primaryClientCertificate.getSerialNumber();
+            if (serialNumber == null || crlDao.findBySerial(serialNumber) != 
null) {
+                final String errorMsg = String.format("Client is using revoked 
certificate of serial=%x, subject=%s from address=%s",

Review comment:
       @sureshanaparti  I actually didn't change the functionality for 
authStrictness compared to before this PR.  It looks like I made a lot of 
changes, but i really just indented everything within "if (authStrictness)".  
The previous code just exited the function right away if authStrictness was 
false.  I instead put the bulk of the method inside that IF so we could still 
run the cert renewal at the end.
   
   I can make a further enhancement to log this - that would be easy.  should I 
go ahead and do so?
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to