Improve logging for extension checkers

DEVSIX-8828

Author: iText-CI

sign/src/main/java/com/itextpdf/signatures/validation/CertificateChainValidator.java

@@ -56,7 +56,7 @@ public class CertificateChainValidator {
             "Certificate {0} is trusted, revocation data checks are not required.";
     static final String CERTIFICATE_TRUSTED_FOR_DIFFERENT_CONTEXT = "Certificate {0} is trusted for {1}, "
             + "but it is not used in this context. Validation will continue as usual.";
-    static final String EXTENSION_MISSING = "Required extension {0} is missing or incorrect.";
+    static final String EXTENSION_MISSING = "Required extension validation failed: {0}";
     static final String ISSUER_MISSING = "Certificate {0} isn't trusted and issuer certificate isn't provided.";
     static final String EXPIRED_CERTIFICATE = "Certificate {0} is expired.";
     static final String NOT_YET_VALID_CERTIFICATE = "Certificate {0} is not yet valid.";
@@ -245,7 +245,7 @@ private void validateRequiredExtensions(ValidationReport result, ValidationConte
                 }
                 if (!requiredExtension.existsInCertificate(certificate)) {
                     result.addReportItem(new CertificateReportItem(certificate, EXTENSIONS_CHECK,
-                            MessageFormatUtil.format(EXTENSION_MISSING, requiredExtension.getExtensionOid()),
+                            MessageFormatUtil.format(EXTENSION_MISSING, requiredExtension.getMessage()),
                             ReportItemStatus.INVALID));
                 }
             }

sign/src/main/java/com/itextpdf/signatures/validation/extensions/CertificateExtension.java

@@ -23,6 +23,7 @@ This file is part of the iText (R) project.
 package com.itextpdf.signatures.validation.extensions;
 
 import com.itextpdf.commons.bouncycastle.asn1.IASN1Primitive;
+import com.itextpdf.commons.utils.MessageFormatUtil;
 import com.itextpdf.signatures.CertificateUtil;
 
 import java.io.IOException;
@@ -34,8 +35,14 @@ This file is part of the iText (R) project.
  */
 public class CertificateExtension {
 
+    public static final String EXCEPTION_OCCURRED = " but an exception occurred {0}:{1}.";
+    public static final String EXTENSION_NOT_FOUND = " but no extension with that id was found.";
+    public static final String FOUND_VALUE = " but found value ";
+    public static final String EXPECTED_EXTENSION_ID_AND_VALUE = "Expected extension with id {0} and value {1}"
+            + " {1} {2}";
     private final String extensionOid;
     private final IASN1Primitive extensionValue;
+    private String errorMessage = "";
 
     /**
      * Create new instance of {@link CertificateExtension} using provided extension OID and value.
@@ -66,6 +73,15 @@ public String getExtensionOid() {
         return extensionOid;
     }
 
+    /**
+     * Returns a message with extra information about the check.
+     * @return a message with extra information about the check.
+     */
+    public String getMessage() {
+        return MessageFormatUtil.format(EXPECTED_EXTENSION_ID_AND_VALUE,
+                getExtensionOid(), getExtensionValue().toString(), errorMessage);
+    }
+
     /**
      * Check if this extension is present in the provided certificate.
      * <p>
@@ -81,9 +97,22 @@ public boolean existsInCertificate(X509Certificate certificate) {
         try {
             providedExtensionValue = CertificateUtil.getExtensionValue(certificate, extensionOid);
         } catch (IOException | RuntimeException e) {
+            errorMessage = MessageFormatUtil.format(EXCEPTION_OCCURRED,
+                    e.getClass().getName(),e.getMessage());
             return false;
         }
-        return Objects.equals(providedExtensionValue, extensionValue);
+        if (providedExtensionValue == null) {
+            if (extensionValue == null) {
+                return true;
+            }
+            errorMessage = EXTENSION_NOT_FOUND;
+            return false;
+        }
+        if (Objects.equals(providedExtensionValue, extensionValue)) {
+            return true;
+        }
+        errorMessage = FOUND_VALUE + MessageFormatUtil.format(" but found value {0}.", extensionValue.toString());
+        return false;
     }
 
     @Override
@@ -102,4 +131,5 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hash((Object) extensionOid, extensionValue);
     }
+
 }

sign/src/main/java/com/itextpdf/signatures/validation/extensions/DynamicBasicConstraintsExtension.java

@@ -24,10 +24,9 @@ This file is part of the iText (R) project.
 
 import com.itextpdf.bouncycastleconnector.BouncyCastleFactoryCreator;
 import com.itextpdf.commons.bouncycastle.IBouncyCastleFactory;
+import com.itextpdf.commons.utils.MessageFormatUtil;
 import com.itextpdf.kernel.crypto.OID;
-import com.itextpdf.signatures.CertificateUtil;
 
-import java.io.IOException;
 import java.security.cert.X509Certificate;
 
 /**
@@ -36,6 +35,9 @@ This file is part of the iText (R) project.
  */
 public class DynamicBasicConstraintsExtension extends DynamicCertificateExtension {
     private static final IBouncyCastleFactory FACTORY = BouncyCastleFactoryCreator.getFactory();
+    public static final String ERROR_MESSAGE =
+            "Expected extension 2.5.29.19 to have a value of at least {0} but found {1}";
+    private String errorMessage;
 
     /**
      * Create new instance of {@link DynamicBasicConstraintsExtension}.
@@ -55,6 +57,16 @@ public DynamicBasicConstraintsExtension() {
      */
     @Override
     public boolean existsInCertificate(X509Certificate certificate) {
-        return certificate.getBasicConstraints() >= getCertificateChainSize() - 1;
+        if (certificate.getBasicConstraints() >= getCertificateChainSize() - 1) {
+            return true;
+        }
+        errorMessage = MessageFormatUtil.format(ERROR_MESSAGE,
+                getCertificateChainSize() - 1, certificate.getBasicConstraints());
+        return false;
+    }
+
+    @Override
+    public String getMessage() {
+        return errorMessage;
     }
 }

sign/src/main/java/com/itextpdf/signatures/validation/extensions/ExtendedKeyUsageExtension.java

@@ -43,8 +43,13 @@ public class ExtendedKeyUsageExtension extends CertificateExtension {
     public static final String CLIENT_AUTH = "1.3.6.1.5.5.7.3.2";
 
     private static final IBouncyCastleFactory FACTORY = BouncyCastleFactoryCreator.getFactory();
+    public static final String EXPECTED_KEY_USAGES = "Expected extended key usages:";
+    public static final String ACTUAL = "But found :";
+    public static final String NO_EXTENDED_KEY_USAGES_WERE_FOUND = " But no extended key usages were found.";
+    public static final String ERROR_OCCURRED_DURING_RETRIEVAL = " But error occurred during retrieval ";
 
     private final List<String> extendedKeyUsageOids;
+    private String errorMessage = "";
 
     /**
      * Create new {@link ExtendedKeyUsageExtension} instance.
@@ -71,14 +76,29 @@ public boolean existsInCertificate(X509Certificate certificate) {
         try {
             providedExtendedKeyUsage = (List<String>) certificate.getExtendedKeyUsage();
         } catch (CertificateParsingException | RuntimeException e) {
+            errorMessage = ERROR_OCCURRED_DURING_RETRIEVAL + e.getClass().getName() + " " + e.getMessage();
             return false;
         }
 
         if (providedExtendedKeyUsage == null) {
+            errorMessage = NO_EXTENDED_KEY_USAGES_WERE_FOUND;
             return false;
         }
-        return providedExtendedKeyUsage.contains(ANY_EXTENDED_KEY_USAGE_OID) ||
-                new HashSet<>(providedExtendedKeyUsage).containsAll(extendedKeyUsageOids);
+
+        if (providedExtendedKeyUsage.contains(ANY_EXTENDED_KEY_USAGE_OID) ||
+                new HashSet<>(providedExtendedKeyUsage).containsAll(extendedKeyUsageOids)) {
+            return true;
+        }
+
+        StringBuilder sb = new StringBuilder(ACTUAL);
+        char sep = '(';
+        for (String usage : providedExtendedKeyUsage) {
+            sb.append(sep).append(usage);
+            sep = ',';
+        }
+        sb.append(')');
+        errorMessage = sb.toString();
+        return false;
     }
 
     private static IKeyPurposeId[] createKeyPurposeIds(List<String> extendedKeyUsageOids) {
@@ -89,4 +109,17 @@ private static IKeyPurposeId[] createKeyPurposeIds(List<String> extendedKeyUsage
         }
         return keyPurposeIds;
     }
+
+    @Override
+    public String getMessage() {
+        StringBuilder sb = new StringBuilder(EXPECTED_KEY_USAGES);
+        char sep = '(';
+        for (String usage : extendedKeyUsageOids) {
+            sb.append(sep).append(usage);
+            sep = ',';
+        }
+        sb.append(')');
+        sb.append(errorMessage);
+        return sb.toString();
+    }
 }

sign/src/main/java/com/itextpdf/signatures/validation/extensions/KeyUsageExtension.java

@@ -24,6 +24,8 @@ This file is part of the iText (R) project.
 
 import com.itextpdf.bouncycastleconnector.BouncyCastleFactoryCreator;
 import com.itextpdf.commons.bouncycastle.IBouncyCastleFactory;
+import com.itextpdf.commons.utils.MessageFormatUtil;
+import com.itextpdf.io.util.EnumUtil;
 import com.itextpdf.kernel.crypto.OID;
 
 import java.security.cert.X509Certificate;
@@ -36,14 +38,24 @@ This file is part of the iText (R) project.
 public class KeyUsageExtension extends CertificateExtension {
 
     private static final IBouncyCastleFactory FACTORY = BouncyCastleFactoryCreator.getFactory();
+    public static final String EXPECTED_VALUE =
+            "Key usage expected: ({0})";
+    public static final String ACTUAL_VALUE = "\nbut found {0}";
+    public static final String MISSING_VALUE = "\nbut nothing found.";
 
     private final int keyUsage;
     private final boolean resultOnMissingExtension;
+    private String messagePreAmble;
+    private String message;
 
     /**
      * Create new {@link KeyUsageExtension} instance using provided {@code int} flag.
      *
      * @param keyUsage {@code int} flag which represents bit values for key usage value
+     *                             bit strings are stored with the big-endian byte order and padding on the end,
+     *                             the big endian notation causes a shift in actual integer values for
+     *                             bits 1-8 becoming 0-7 and bit 1
+     *                             the 7 bits padding makes for bit 0 to become bit 7 of the first byte
      */
     public KeyUsageExtension(int keyUsage) {
         this(keyUsage, false);
@@ -52,14 +64,20 @@ public KeyUsageExtension(int keyUsage) {
     /**
      * Create new {@link KeyUsageExtension} instance using provided {@code int} flag.
      *
-     * @param keyUsage {@code int} flag which represents bit values for key usage value
+     * @param keyUsage                 {@code int} flag which represents bit values for key usage value
+     *                                 bit strings are stored with the big-endian byte order and padding on the end,
+     *                                 the big endian notation causes a shift in actual integer values for bits 1-8
+     *                                 becoming 0-7 and bit 1
+     *                                 the 7 bits padding makes for bit 0 to become bit 7 of the first byte
      * @param resultOnMissingExtension parameter which represents return value for
      * {@link #existsInCertificate(X509Certificate)} method in case of the extension not being present in a certificate
      */
     public KeyUsageExtension(int keyUsage, boolean resultOnMissingExtension) {
         super(OID.X509Extensions.KEY_USAGE, FACTORY.createKeyUsage(keyUsage).toASN1Primitive());
         this.keyUsage = keyUsage;
         this.resultOnMissingExtension = resultOnMissingExtension;
+        messagePreAmble =MessageFormatUtil.format(EXPECTED_VALUE , convertKeyUsageMaskToString(keyUsage));
+        message = messagePreAmble;
     }
 
     /**
@@ -74,7 +92,7 @@ public KeyUsageExtension(List<KeyUsage> keyUsages) {
     /**
      * Create new {@link KeyUsageExtension} instance using provided key usage enum list.
      *
-     * @param keyUsages key usages {@link List} which represents key usage values
+     * @param keyUsages                key usages {@link List} which represents key usage values
      * @param resultOnMissingExtension parameter which represents return value for
      * {@link #existsInCertificate(X509Certificate)} method in case of the extension not being present in a certificate
      */
@@ -115,44 +133,62 @@ public boolean existsInCertificate(X509Certificate certificate) {
         boolean[] providedKeyUsageFlags = certificate.getKeyUsage();
         if (providedKeyUsageFlags == null) {
             // By default, we want to return true if extension is not specified. Configurable.
+            message = messagePreAmble + MISSING_VALUE;
             return resultOnMissingExtension;
         }
-        for (int i = 0; i < providedKeyUsageFlags.length; ++i) {
-            int power = providedKeyUsageFlags.length - i - 2;
-            if (power < 0) {
-                // Bits are encoded backwards, for the last bit power is -1 and in this case we need to go over byte
-                power = 16 + power;
-            }
-            if ((keyUsage & (1 << power)) != 0 && !providedKeyUsageFlags[i]) {
-                return false;
+        int bitmap = 0;
+        // bit strings are stored with the big-endian byte order and padding on the end,
+        // the big endian notation causes a shift in actual integer values for bits 1-8 becoming 0-7 and bit 1
+        // the 7 bits padding makes for bit 0 to become bit 7 of the first byte
+        for (int i = 0; i < providedKeyUsageFlags.length - 1; ++i) {
+            if (providedKeyUsageFlags[i]) {
+                bitmap +=  1 << (8-i-1);
             }
         }
+        if (providedKeyUsageFlags[8]) {
+            bitmap +=  0x8000;
+        }
+        if ((bitmap & keyUsage) != keyUsage) {
+            message = new StringBuilder(messagePreAmble).append(
+                    MessageFormatUtil.format(ACTUAL_VALUE, convertKeyUsageMaskToString(bitmap)))
+                    .toString();
+            return false;
+        }
         return true;
     }
+    @Override
+    public String getMessage() {
+        return message;
+    }
 
-    private static int convertKeyUsageSetToInt(List<KeyUsage> keyUsages) {
-        KeyUsage[] possibleKeyUsage = new KeyUsage[] {
-                KeyUsage.DIGITAL_SIGNATURE,
-                KeyUsage.NON_REPUDIATION,
-                KeyUsage.KEY_ENCIPHERMENT,
-                KeyUsage.DATA_ENCIPHERMENT,
-                KeyUsage.KEY_AGREEMENT,
-                KeyUsage.KEY_CERT_SIGN,
-                KeyUsage.CRL_SIGN,
-                KeyUsage.ENCIPHER_ONLY,
-                KeyUsage.DECIPHER_ONLY
-        };
-        int result = 0;
-        for (int i = 0; i < possibleKeyUsage.length; ++i) {
-            if (keyUsages.contains(possibleKeyUsage[i])) {
-                int power = possibleKeyUsage.length - i - 2;
-                if (power < 0) {
-                    // Bits are encoded backwards, for the last bit power is -1 and in this case we need to go over byte
-                    power = 16 + power;
-                }
-                result |= (1 << power);
+    private static String convertKeyUsageMaskToString(int keyUsageMask) {
+        StringBuilder result = new StringBuilder();
+        String separator = "";
+        // bit strings are stored with the big-endian byte order and padding on the end,
+        // the big endian notation causes a shift in actual integer values for bits 1-8 becoming 0-7 and bit 1
+        // the 7 bits padding makes for bit 0 to become bit 7 of the first byte
+        for (KeyUsage usage: EnumUtil.getAllValuesOfEnum(KeyUsage.class)) {
+            if (((1 << (8-usage.ordinal()-1)) & keyUsageMask) > 0 ||
+                    (usage == KeyUsage.DECIPHER_ONLY &&  (keyUsageMask & 0x8000) == 0x8000)) {
+                result.append(separator)
+                        .append(usage);
+                separator = ", ";
+            }
+        }
+        return result.toString();
+    }
+    private static int convertKeyUsageSetToInt(Iterable<KeyUsage> keyUsages) {
+        int keyUsageMask = 0;
+        // bit strings are stored with the big-endian byte order and padding on the end,
+        // the big endian notation causes a shift in actual integer values for bits 1-8 becoming 0-7 and bit 1
+        // the 7 bits padding makes for bit 0 to become bit 7 of the first byte
+        for (KeyUsage usage: keyUsages) {
+            if (usage == KeyUsage.DECIPHER_ONLY) {
+                keyUsageMask += 0x8000;
+                continue;
             }
+            keyUsageMask +=  1 << ( 8 - usage.ordinal() - 1);
         }
-        return result;
+        return keyUsageMask;
     }
 }