more test coverage

Author: slobodanadamovic

x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/CertGenUtils.java

@@ -478,17 +478,24 @@ public static KeyUsage buildKeyUsage(Collection<String> keyUsages) {
         }
 
         int usageBits = 0;
-        for (String keyUsage : keyUsages) {
-            Integer keyUsageValue = KEY_USAGE_MAPPINGS.get(keyUsage.trim());
+        for (String keyUsageName : keyUsages) {
+            Integer keyUsageValue = findKeyUsageByName(keyUsageName);
             if (keyUsageValue == null) {
-                throw new IllegalArgumentException("Unknown keyUsage: " + keyUsage);
+                throw new IllegalArgumentException("Unknown keyUsage: " + keyUsageName);
             }
             usageBits |= keyUsageValue;
         }
         return new KeyUsage(usageBits);
     }
 
     public static boolean isValidKeyUsage(String keyUsage) {
-        return KEY_USAGE_MAPPINGS.containsKey(keyUsage);
+        return findKeyUsageByName(keyUsage) != null;
+    }
+
+    private static Integer findKeyUsageByName(String keyUsageName) {
+        if (keyUsageName == null) {
+            return null;
+        }
+        return KEY_USAGE_MAPPINGS.get(keyUsageName.trim());
     }
 }

x-pack/plugin/security/cli/src/main/java/org/elasticsearch/xpack/security/cli/HttpCertificateCommand.java

@@ -84,6 +84,7 @@
 
 import static org.elasticsearch.xpack.security.cli.CertGenUtils.buildKeyUsage;
 import static org.elasticsearch.xpack.security.cli.CertGenUtils.generateSignedCertificate;
+import static org.elasticsearch.xpack.security.cli.CertGenUtils.isValidKeyUsage;
 
 /**
  * This command is the "elasticsearch-certutil http" command. It provides a guided process for creating
@@ -1048,7 +1049,7 @@ private static List<String> readKeyUsage(Terminal terminal, List<String> default
                     terminal.println("Key usage cannot be empty");
                     return null;
                 }
-                if (CertGenUtils.isValidKeyUsage(keyUsage) == false) {
+                if (isValidKeyUsage(keyUsage) == false) {
                     terminal.println("Invalid key usage: " + keyUsage);
                     terminal.println("The key usage should be one of the following values: ");
                     for (String keyUsageName : CertGenUtils.KEY_USAGE_MAPPINGS.keySet()) {

x-pack/plugin/security/cli/src/test/java/org/elasticsearch/xpack/security/cli/CertGenUtilsTests.java

@@ -13,6 +13,7 @@
 import org.bouncycastle.asn1.x509.GeneralName;
 import org.bouncycastle.asn1.x509.GeneralNames;
 import org.bouncycastle.asn1.x509.KeyPurposeId;
+import org.bouncycastle.asn1.x509.KeyUsage;
 import org.elasticsearch.common.network.InetAddresses;
 import org.elasticsearch.common.network.NetworkAddress;
 import org.elasticsearch.core.SuppressForbidden;
@@ -38,7 +39,11 @@
 import javax.security.auth.x500.X500Principal;
 
 import static org.elasticsearch.xpack.security.cli.CertGenUtils.KEY_USAGE_BITS;
+import static org.elasticsearch.xpack.security.cli.CertGenUtils.KEY_USAGE_MAPPINGS;
 import static org.elasticsearch.xpack.security.cli.CertGenUtils.buildKeyUsage;
+import static org.elasticsearch.xpack.security.cli.CertGenUtils.isValidKeyUsage;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -190,6 +195,69 @@ public void testIssuerCertSubjectDN() throws Exception {
 
     }
 
+    public void testBuildKeyUsage() {
+        // sanity check that lookup maps are containing the same keyUsage entries
+        assertThat(KEY_USAGE_BITS.keySet(), containsInAnyOrder(KEY_USAGE_MAPPINGS.keySet().toArray()));
+
+        // passing null or empty list of keyUsage names should return null
+        assertThat(buildKeyUsage(null), is(nullValue()));
+        assertThat(buildKeyUsage(List.of()), is(nullValue()));
+
+        // invalid names should throw IAE
+        var e = expectThrows(IllegalArgumentException.class, () -> buildKeyUsage(List.of(randomAlphanumericOfLength(5))));
+        assertThat(e.getMessage(), containsString("Unknown keyUsage"));
+
+        {
+            final List<String> keyUsages = randomNonEmptySubsetOf(KEY_USAGE_MAPPINGS.keySet());
+            final KeyUsage keyUsage = buildKeyUsage(keyUsages);
+            for (String usageName : keyUsages) {
+                final Integer usage = KEY_USAGE_MAPPINGS.get(usageName);
+                assertThat(" mapping for keyUsage [" + usageName + "] is missing", usage, is(notNullValue()));
+                assertThat("expected keyUsage [" + usageName + "] to be set in [" + keyUsage + "]", keyUsage.hasUsages(usage), is(true));
+            }
+
+            final Set<String> keyUsagesNotSet = KEY_USAGE_MAPPINGS.keySet()
+                .stream()
+                .filter(u -> keyUsages.contains(u) == false)
+                .collect(Collectors.toSet());
+
+            for (String usageName : keyUsagesNotSet) {
+                final Integer usage = KEY_USAGE_MAPPINGS.get(usageName);
+                assertThat(" mapping for keyUsage [" + usageName + "] is missing", usage, is(notNullValue()));
+                assertThat(
+                    "expected keyUsage [" + usageName + "] not to be set in [" + keyUsage + "]",
+                    keyUsage.hasUsages(usage),
+                    is(false)
+                );
+            }
+
+        }
+
+        {
+            // test that duplicates and whitespaces are ignored
+            KeyUsage keyUsage = buildKeyUsage(
+                List.of("digitalSignature   ", "    nonRepudiation", "\tkeyEncipherment", "keyEncipherment\n")
+            );
+            assertThat(keyUsage.hasUsages(KEY_USAGE_MAPPINGS.get("digitalSignature")), is(true));
+            assertThat(keyUsage.hasUsages(KEY_USAGE_MAPPINGS.get("nonRepudiation")), is(true));
+            assertThat(keyUsage.hasUsages(KEY_USAGE_MAPPINGS.get("digitalSignature")), is(true));
+            assertThat(keyUsage.hasUsages(KEY_USAGE_MAPPINGS.get("keyEncipherment")), is(true));
+        }
+    }
+
+    public void testIsValidKeyUsage() {
+        assertThat(isValidKeyUsage(randomFrom(KEY_USAGE_MAPPINGS.keySet())), is(true));
+        assertThat(isValidKeyUsage(randomAlphanumericOfLength(5)), is(false));
+
+        // keyUsage names are case-sensitive
+        assertThat(isValidKeyUsage("DigitalSignature"), is(false));
+
+        // white-spaces are ignored
+        assertThat(isValidKeyUsage("keyAgreement "), is(true));
+        assertThat(isValidKeyUsage("keyCertSign\n"), is(true));
+        assertThat(isValidKeyUsage("\tcRLSign  "), is(true));
+    }
+
     public static void assertExpectedKeyUsage(X509Certificate certificate, List<String> expectedKeyUsage) {
         final boolean[] keyUsage = certificate.getKeyUsage();
         assertThat("Expected " + KEY_USAGE_BITS.size() + " bits for key usage", keyUsage.length, equalTo(KEY_USAGE_BITS.size()));