#153: Fix introduced LGTM alert. Remove DAPProperties setters. Cipher…

… back to Signature. Swap -token to -token-key.

src/main/java/pro/javacard/gp/DAPProperties.java

@@ -7,7 +7,6 @@
 
 import static pro.javacard.gp.GPCommandLineInterface.OPT_DAP_DOMAIN;
 import static pro.javacard.gp.GPCommandLineInterface.OPT_TO;
-import static pro.javacard.gp.GPTool.fail;
 
 public class DAPProperties {
     private AID targetDomain = null;
@@ -18,6 +17,9 @@ public DAPProperties(OptionSet args, GlobalPlatform gp) throws CardException, GP
         // Override target and check for DAP
         if (args.has(OPT_TO)) {
             targetDomain = AID.fromString(args.valueOf(OPT_TO));
+            if (gp.getRegistry().getDomain(targetDomain) == null) {
+                throw new GPException("Specified target domain is invalid: " + targetDomain);
+            }
             if (gp.getRegistry().getDomain(targetDomain).getPrivileges().has(GPRegistryEntry.Privilege.DAPVerification))
                 required = true;
         }
@@ -33,7 +35,7 @@ public DAPProperties(OptionSet args, GlobalPlatform gp) throws CardException, GP
             dapDomain = AID.fromString(args.valueOf(OPT_DAP_DOMAIN));
             GPRegistryEntry.Privileges p = gp.getRegistry().getDomain(dapDomain).getPrivileges();
             if (!(p.has(GPRegistryEntry.Privilege.DAPVerification) || p.has(GPRegistryEntry.Privilege.MandatedDAPVerification))) {
-                fail("Specified DAP domain does not have (Mandated)DAPVerification privilege: " + p.toString());
+                throw new GPException("Specified DAP domain does not have (Mandated)DAPVerification privilege: " + p.toString());
             }
         }
     }
@@ -42,23 +44,11 @@ public AID getTargetDomain() {
         return targetDomain;
     }
 
-    public void setTargetDomain(AID targetDomain) {
-        this.targetDomain = targetDomain;
-    }
-
     public AID getDapDomain() {
         return dapDomain;
     }
 
-    public void setDapDomain(AID dapDomain) {
-        this.dapDomain = dapDomain;
-    }
-
     public boolean isRequired() {
         return required;
     }
-
-    public void setRequired(boolean required) {
-        this.required = required;
-    }
 }

src/main/java/pro/javacard/gp/DelegatedManagementHandler.java → src/main/java/pro/javacard/gp/DMTokenGenerator.java

@@ -3,17 +3,18 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.crypto.Cipher;
 import javax.smartcardio.CommandAPDU;
 import java.io.ByteArrayOutputStream;
 import java.security.PrivateKey;
+import java.security.Signature;
+
+public class DMTokenGenerator {
+    private static final Logger logger = LoggerFactory.getLogger(DMTokenGenerator.class);
+    private static final String acceptedSignatureAlgorithm = "SHA1withRSA";
 
-public class DelegatedManagementHandler {
-    private static final Logger logger = LoggerFactory.getLogger(DelegatedManagementHandler.class);
-    private static final String acceptedSignatureAlgorithm = "RSA/ECB/PKCS1Padding";
     private PrivateKey key;
 
-    public DelegatedManagementHandler(PrivateKey key) {
+    public DMTokenGenerator(PrivateKey key) {
         this.key = key;
     }
 
@@ -56,13 +57,17 @@ private static byte[] getTokenData(CommandAPDU apdu) {
 
     private static byte[] signData(PrivateKey privateKey, byte[] apduData) {
         try {
-            Cipher cipher = Cipher.getInstance(acceptedSignatureAlgorithm);
-            cipher.init(Cipher.ENCRYPT_MODE, privateKey);
-
-            return cipher.doFinal(apduData);
+            Signature signature = Signature.getInstance(acceptedSignatureAlgorithm);
+            signature.initSign(privateKey);
+            signature.update(apduData);
+            return signature.sign();
         } catch (Exception e) {
             throw new RuntimeException("Could not create signature with instance " + acceptedSignatureAlgorithm, e);
         }
     }
 
+    public boolean hasKey() {
+        return key != null;
+    }
+
 }

src/main/java/pro/javacard/gp/GPCommandLineInterface.java

@@ -104,7 +104,8 @@ abstract class GPCommandLineInterface {
     protected final static String OPT_ACR_DELETE = "acr-delete";
     protected final static String OPT_ACR_RULE = "acr-rule";
     protected final static String OPT_ACR_CERT_HASH = "acr-hash";
-    protected final static String OPT_TOKEN = "token";
+    protected final static String OPT_TOKEN_KEY = "token-key";
+    // TODO - include token "as is" with -token
 
     protected static OptionSet parseArguments(String[] argv) throws IOException {
         OptionSet args = null;
@@ -161,7 +162,7 @@ protected static OptionSet parseArguments(String[] argv) throws IOException {
         parser.accepts(OPT_STORE_DATA_BLOB, "STORE DATA blob").withRequiredArg().describedAs("data");
         parser.accepts(OPT_STORE_DATA, "Send STORE DATA commands").withRequiredArg().describedAs("data");
 
-        parser.accepts(OPT_TOKEN, "Path to private key used in Delegated Management token generation").withRequiredArg().describedAs("path");
+        parser.accepts(OPT_TOKEN_KEY, "Path to private key used in Delegated Management token generation").withRequiredArg().describedAs("path");
 
         parser.accepts(OPT_MAKE_DEFAULT, "Make AID the default").withRequiredArg().describedAs("AID");
         parser.accepts(OPT_RENAME_ISD, "Rename ISD").withRequiredArg().describedAs("new AID");

src/main/java/pro/javacard/gp/GPTool.java

@@ -237,7 +237,11 @@ public static void main(String[] argv) throws Exception {
                         // FIXME: would like to get AID from oracle as well.
                     }
 
-                    gp.setTokenKey(privateKeyOrNull(args));
+                    if (args.has(OPT_TOKEN_KEY)) {
+                        gp.setDMTokenGenerator(new DMTokenGenerator(getRSAPrivateKey(args.valueOf(OPT_TOKEN_KEY).toString())));
+                    } else {
+                        gp.setDMTokenGenerator(new DMTokenGenerator(null));
+                    }
 
                     // Don't do sanity checks, just run asked commands
                     if (args.has(OPT_FORCE))
@@ -860,8 +864,6 @@ private static void calculateDapPropertiesAndLoadCap(OptionSet args, GlobalPlatf
                     // Do nothing. Here for findbugs
             }
             throw e;
-        } catch (CardException e) {
-            throw e;
         }
     }
 
@@ -875,18 +877,17 @@ private static void loadCapAccordingToDapRequirement(OptionSet args, GlobalPlatf
         }
     }
 
-    private static PrivateKey privateKeyOrNull(OptionSet args) {
-        if (args.has(OPT_TOKEN)) {
-            try (FileInputStream fin = new FileInputStream(new File(args.valueOf(OPT_TOKEN).toString()))) {
-                PrivateKey key = GPCrypto.pem2PrivateKey(fin);
-                if (key instanceof RSAPrivateKey) {
-                    return key;
-                }
-            } catch (Exception e) {
-                throw new RuntimeException("Could not extract RSA private key from supplied path");
+    private static PrivateKey getRSAPrivateKey(String path) {
+        try (FileInputStream fin = new FileInputStream(new File(path))) {
+            PrivateKey key = GPCrypto.pem2PrivateKey(fin);
+            if (key instanceof RSAPrivateKey) {
+                return key;
+            } else {
+                throw new RuntimeException("Supplied key at path is not instance of RSAPrivateKey");
             }
+        } catch (Exception e) {
+            throw new RuntimeException("Could not extract RSAPrivateKey from supplied path");
         }
-        return null;
     }
 
     // FIXME: get rid

src/main/java/pro/javacard/gp/GlobalPlatform.java

@@ -104,7 +104,7 @@ public class GlobalPlatform extends CardChannel implements AutoCloseable {
     private SecureChannelWrapper wrapper = null;
     private CardChannel channel;
     private GPRegistry registry = null;
-    private PrivateKey tokenKey;
+    private DMTokenGenerator tokenGenerator;
     private boolean dirty = true; // True if registry is dirty.
 
     /**
@@ -235,8 +235,8 @@ public void setSpec(GPSpec spec) {
         this.spec = spec;
     }
 
-    public void setTokenKey(PrivateKey key) {
-        this.tokenKey = key;
+    public void setDMTokenGenerator(DMTokenGenerator tokenGenerator) {
+        this.tokenGenerator = tokenGenerator;
     }
 
     public AID getAID() {
@@ -550,12 +550,11 @@ private ResponseAPDU transmitLV(CommandAPDU command) throws CardException {
     }
 
     private ResponseAPDU transmitDM(CommandAPDU command) throws CardException {
-        if (tokenKey == null && command.getINS() == INS_DELETE) {
+        if (!tokenGenerator.hasKey() && command.getINS() == INS_DELETE) {
             //Only add token bytes to INS_DELETE if key exists for token calculation, since token is optional
             return transmitLV(command);
         }
-        DelegatedManagementHandler dmHandler = new DelegatedManagementHandler(tokenKey);
-        command = dmHandler.applyToken(command);
+        command = tokenGenerator.applyToken(command);
         return transmitLV(command);
     }
 
@@ -625,7 +624,7 @@ private void loadCapFile(CAPFile cap, AID targetDomain, boolean includeDebug, bo
         ByteArrayOutputStream loadBlock = new ByteArrayOutputStream();
         try {
             // Add DAP block, if signature present
-            if (dap != null) {
+            if (dap != null && dapDomain != null) {
                 loadBlock.write(0xE2);
                 loadBlock.write(GPUtils.encodeLength(dapDomain.getLength() + dap.length + GPUtils.encodeLength(dap.length).length + 3)); // two tags, two lengths FIXME: proper size
                 loadBlock.write(0x4F);

src/test/java/pro/javacard/gp/TestDelegatedManagementHandler.java → src/test/java/pro/javacard/gp/TestDMTokenGenerator.java

@@ -13,7 +13,7 @@
 import static pro.javacard.gp.GlobalPlatform.CLA_GP;
 import static pro.javacard.gp.GlobalPlatform.INS_INSTALL;
 
-public class TestDelegatedManagementHandler {
+public class TestDMTokenGenerator {
 
     private PrivateKey key;
 
@@ -29,15 +29,15 @@ public void setUp() {
     @Test
     public void testApplyToken() {
         CommandAPDU command = new CommandAPDU(CLA_GP, INS_INSTALL, 0x02, 0x00, new byte[]{0});
-        DelegatedManagementHandler dmHandler = new DelegatedManagementHandler(key);
+        DMTokenGenerator dmHandler = new DMTokenGenerator(key);
         command = dmHandler.applyToken(command);
         Assert.assertTrue(command.getData().length > 1);
     }
 
     @Test
     public void testApplyEmptyToken() {
         CommandAPDU command = new CommandAPDU(CLA_GP, INS_INSTALL, 0x02, 0x00, new byte[]{0});
-        DelegatedManagementHandler dmHandler = new DelegatedManagementHandler(null);
+        DMTokenGenerator dmHandler = new DMTokenGenerator(null);
         command = dmHandler.applyToken(command);
         Assert.assertArrayEquals(command.getData(), new byte[]{0, 0});
     }