Add codefactor to dictionary

Turns out that this does not work on the omni-ring, despite documention saying it should

.gitignore

@@ -1,2 +1,4 @@
 *.jar
 *.cap
+*/.venv/*
+*.pyc

.vscode/settings.json

@@ -3,5 +3,16 @@
     "license.extension": ".txt",
     "license.default": "GPL-2.0",
     "license.filename": "LICENSE",
-    "license.year": "auto"
+    "license.year": "auto",
+    "cSpell.words": [
+        "APDU",
+        "apdubuf",
+        "CodeFactor",
+        "Infineon",
+        "minlen",
+        "nopad",
+        "offcdata",
+        "pkcs",
+        "smartpgp"
+    ]
 }

README.md

@@ -1,5 +1,7 @@
 # SmartPGP applet
 
+[![CodeFactor](https://www.codefactor.io/repository/github/c0de-fox/smartpgp/badge)](https://www.codefactor.io/repository/github/c0de-fox/smartpgp)
+
 SmartPGP is a free and open source implementation of the [OpenPGP card
 3.4 specification](https://gnupg.org/ftp/specs/OpenPGP-smart-card-application-3.4.pdf) in JavaCard.
 
@@ -17,6 +19,17 @@ of them depend on underlying hardware support and available
 - Command and response chaining
 - AES 128/256 bits deciphering primitive
 
+## Warnings
+
+### ROCA
+
+[Infineon SLE78](https://www.infineon.com/cms/en/product/security-smart-card-solutions/security-controllers/contactless-and-dual-interface-security-controllers/) chips are vulnerable to [ROCA](https://crocs.fi.muni.cz/public/papers/rsa_ccs17).
+This attack is only relevant if you used on-device key generation. It allows an adversary to obtain your private key, using only your public key.
+
+There isn't much that can be done to rectify this, other than generating the private RSA keys off of your device and importing them. [Other work arounds](https://crocs.fi.muni.cz/public/papers/rsa_ccs17#detection_tools_mitigation_and_workarounds). (Using the [OpenCrypto JCMathLib](https://github.com/OpenCryptoProject/JCMathLib) to handle the cryptographic functions may work too)
+
+Use [this tool](https://github.com/crocs-muni/roca#install-with-pip) to determine if your public keys are vulnerable.
+
 ## Default values
 
 The SmartPGP applet is configured with the following default values:

build.xml

@@ -11,7 +11,7 @@
     <target name="convert">
         <javacard>
             <cap jckit="${JC303}" output="SmartPGPApplet.cap" sources="src" aid="d27600012401" version="1.0">
-                <applet class="dev.c0de.smartpgp.SmartPGPApplet" aid="d276000124010304c0deedb522560000"/>
+                <applet class="dev.c0de.smartpgp.SmartPGPApplet" aid="d276000124010304c0de000000000000"/>
             </cap>
         </javacard>
     </target>

src/dev/c0de/smartpgp/Common.java

@@ -32,9 +32,11 @@ public final class Common {
     protected final RandomData random;
 
     protected Common() {
+        /* Get an instance of cryptography Ciphers */
         cipher_aes_cbc_nopad = Cipher.getInstance(Cipher.ALG_AES_BLOCK_128_CBC_NOPAD, false);
         cipher_rsa_pkcs1 = Cipher.getInstance(Cipher.ALG_RSA_PKCS1, false);
 
+        /* Get an instance of the Secure Random Number Generator */
         random = RandomData.getInstance(RandomData.ALG_SECURE_RANDOM);
     }
 
@@ -158,8 +160,8 @@ public final class Common {
         }
     }
 
-    protected static final short writeAlgorithmInformation(final byte key_tag,
+    protected static final short writeAlgorithmInformation(final byte key_tag, final byte[] buf, short off) {
-                                                           final byte[] buf, short off) {
+
         for(short m = 2; m <= 4; ++m) {
             for(byte form = Constants.RSA_IMPORT_SUPPORTS_FORMAT_1 ? 1 : 3; form <= 3; form += 2) {
                 buf[off++] = key_tag;

src/dev/c0de/smartpgp/Constants.java

@@ -30,40 +30,46 @@ public final class Constants {
 
     protected static final short APDU_MAX_LENGTH = (short)0x400;
 
+    /* See section 4.3.2 of the specification; Default is NONE; Standard UTF-8 PWs*/
     protected static final byte[] KEY_DERIVATION_FUNCTION_DEFAULT = {
         (byte)0x81, (byte)0x01, (byte)0x00
     };
 
-    protected static final byte USER_PIN_RETRY_COUNT = 3;
+    protected static final byte USER_PIN_RETRY_COUNT = 3; /* Card gets locked after this many incorrect attempts */
-    protected static final byte USER_PIN_MIN_SIZE = 0x06;
+    protected static final byte USER_PIN_MIN_SIZE = 0x06; /* 6 chars is minimum as defined by spec */
-    protected static final byte USER_PIN_MAX_SIZE = 0x7f; /* max is 0x7f because PIN format 2 */
+    protected static final byte USER_PIN_MAX_SIZE = 0x7f; /* 127 chars is maximum as defined by spec */
+
+    /* UTF-8 bytes for the default user pin: 123456 is the value defined by the specification */
     protected static final byte[] USER_PIN_DEFAULT = {
         (byte)0x31, (byte)0x32, (byte)0x33, (byte)0x34,
         (byte)0x35, (byte)0x36
     };
 
+    /* Is the USER_PIN required for signing actions; default: true */
     protected static final boolean USER_PIN_DEFAULT_FORCE_VERIFY_SIGNATURE = true;
 
-    protected static final byte USER_PUK_RETRY_COUNT = 3;
+    protected static final byte USER_PUK_RETRY_COUNT = 3; /*  */
-    protected static final byte USER_PUK_MIN_SIZE = 0x08;
+    protected static final byte USER_PUK_MIN_SIZE = 0x08; /* 8 chars is minimum as defined by spec */
-    protected static final byte USER_PUK_MAX_SIZE = 0x7f; /* max is 0x7f because PIN format 2 */
+    protected static final byte USER_PUK_MAX_SIZE = 0x7f; /* 127 chars is maximum as defined by spec */
 
-    protected static final byte ADMIN_PIN_RETRY_COUNT = 3;
+    protected static final byte ADMIN_PIN_RETRY_COUNT = 3; /* Card gets reset after this many failed attempts */
-    protected static final byte ADMIN_PIN_MIN_SIZE = 0x08;
+    protected static final byte ADMIN_PIN_MIN_SIZE = 0x08; /* 8 chars is minimum as defined by spec */
-    protected static final byte ADMIN_PIN_MAX_SIZE = 0x7f; /* max is 0x7f because PIN format 2 */
+    protected static final byte ADMIN_PIN_MAX_SIZE = 0x7f; /* 127 chars is maximum as defined by spec */
     protected static final byte[] ADMIN_PIN_DEFAULT = {
         (byte)0x31, (byte)0x32, (byte)0x33, (byte)0x34,
         (byte)0x35, (byte)0x36, (byte)0x37, (byte)0x38
     };
 
 
-    protected static final byte FINGERPRINT_SIZE = 20;
+    protected static final byte FINGERPRINT_SIZE = 20; /* size of each fingerprint in bytes */
-    protected static final byte GENERATION_DATE_SIZE = 4;
+    protected static final byte GENERATION_DATE_SIZE = 4; /* number of bytes to store the date+time */
 
-    protected static final byte NAME_MAX_LENGTH = 39;
+    protected static final byte NAME_MAX_LENGTH = 39; /* max number of chars in cardholder name */
-    protected static final byte LANG_MIN_LENGTH = 2;
+    protected static final byte LANG_MIN_LENGTH = 2; /* 2 char language codes */
-    protected static final byte LANG_MAX_LENGTH = 8;
+    protected static final byte LANG_MAX_LENGTH = 8; /* spec allows up to 4 languages */
-    protected static final byte[] LANG_DEFAULT = { (byte)0x65, (byte)0x6e };
+    protected static final byte[] LANG_DEFAULT = { (byte)0x65, (byte)0x6e }; /* utf-8: EN */
+
+    /* Unsure if the following bytes and shorts actually need to be in card memory */
 
     protected static final byte SEX_NOT_KNOWN = (byte)0x30;
     protected static final byte SEX_MALE = (byte)0x31;
@@ -108,6 +114,8 @@ public final class Constants {
     protected static final short TAG_KEY_DERIVATION_FUNCTION = (short)0x00f9;
     protected static final short TAG_ALGORITHM_INFORMATION = (short)0x00fa;
 
+    /* The bytes and shorts below appear to be needed */
+
     protected static final byte CRT_TAG_AUTHENTICATION_KEY = (byte)0xa4;
     protected static final byte CRT_TAG_SIGNATURE_KEY = (byte)0xb6;
     protected static final byte CRT_TAG_DECRYPTION_KEY = (byte)0xb8;
@@ -150,9 +158,9 @@ public final class Constants {
         (byte)0xC0, /* 1st byte: "methods supported" see ISO 7816-4 */
         (byte)0x01, /* 2nd byte: "data coding byte" idem */
 
-        /* 3rd byte: command chaining + extended length; Set to 0x80 if
+        /* 3rd byte: command chaining + extended length; Set to 0xC0 if
-                     extended length is not supported by card or reader */
+                     extended length is supported by card and reader */
-        (byte)0xC0,
+        (byte)0x80,
 
         (byte)0x05, /* status indicator byte : operational state */
         (byte)0x90, /* SW1 */
@@ -169,9 +177,9 @@ public final class Constants {
                0x02 | /* support PSO:DEC/ENC AES */
                0x01), /* support KDF-DO */
         (byte)0x00, /* SM 0x01 = 128 bits, 0x02 = 256 bits, 0x03 = SCP11b */
-        (byte)0x00, (byte)0x20, /* max length get challenge */
+        (byte)0x00, (byte)0x20, /* max length of get challenge response in bytes (decimal: 32) */
-        (byte)0x04, (byte)0x80, /* max length of carholder certificate in Bytes (decimal: 1152) */
+        (byte)0x04, (byte)0x80, /* max length of cardholder certificate in bytes (decimal: 1152) */
-        (byte)0x00, (byte)0xff, /* max length of special DOs (private, login, url, KDF-DO) */
+        (byte)0x00, (byte)0xff, /* max length of special DOs (private, login, url, KDF-DO) in bytes (decimal: 255) */
         (byte)0x00, /* PIN format 2 is not supported */
         (byte)0x00  /* MSE not supported */
     };
@@ -217,6 +225,7 @@ public final class Constants {
     protected static final byte ALGORITHM_ATTRIBUTES_MIN_LENGTH = 6;
     protected static final byte ALGORITHM_ATTRIBUTES_MAX_LENGTH = 13;
 
+    /* FIXME: Can I modify these to get safer private key generation? */
     protected static final byte[] ALGORITHM_ATTRIBUTES_DEFAULT = {
         (byte)0x01, /* RSA */
         (byte)0x08, (byte)0x00, /* 2048 bits modulus */

src/dev/c0de/smartpgp/PGPKey.java

@@ -179,6 +179,14 @@ public final class PGPKey {
         return Util.getShort(attributes, (short)3);
     }
 
+    /*
+     * !!! WARNING !!! - Read this if your JavaCard is Infineon SLE78 
+     * The API called by this function is flawed and vulnerable to ROCA.
+     * Malicious actors are able to determine the private key using ONLY the public key.
+     * 
+     * It's HIGHLY recommended that you do NOT use this API; Instead, you
+     * should generate your private key off-device, then import it later
+     */
     private final KeyPair generateRSA() {
         final PrivateKey priv = (PrivateKey)KeyBuilder.buildKey(KeyBuilder.TYPE_RSA_CRT_PRIVATE, rsaModulusBitSize(), false);
         final RSAPublicKey pub = (RSAPublicKey)KeyBuilder.buildKey(KeyBuilder.TYPE_RSA_PUBLIC, rsaModulusBitSize(), false);

src/dev/c0de/smartpgp/Persistent.java

@@ -87,8 +87,7 @@ public final class Persistent {
     protected final OwnerPIN admin_pin; /* PW3 */
     protected byte admin_pin_length;
 
-
+    /* Called at Applet Install time; Reserve memory of persistent data objects */
-
     protected Persistent() {
         login = new byte[Constants.specialDoMaxLength()];
         login_length = 0;

src/dev/c0de/smartpgp/SmartPGPApplet.java

@@ -39,11 +39,13 @@ public final class SmartPGPApplet extends Applet implements ExtendedLength {
         transients = new Transients();
     }
 
+    /* Called on applet install */
     public static final void install(byte[] buf, short off, byte len) {
+        // Reserve memory immediately 
         new SmartPGPApplet().register(buf, (short)(off + 1), buf[off]);
     }
 
-    private final PGPKey currentTagOccurenceToKey() {
+    private final PGPKey currentTagOccurrenceToKey() {
         switch(transients.currentTagOccurrence()) {
         case 0:
             return data.pgp_keys[Persistent.PGP_KEYS_OFFSET_AUT];
@@ -421,7 +423,7 @@ public final class SmartPGPApplet extends Applet implements ExtendedLength {
             break;
 
         case Constants.TAG_CARDHOLDER_CERTIFICATE:
-            k = currentTagOccurenceToKey();
+            k = currentTagOccurrenceToKey();
 
             if(k == null) {
                 ISOException.throwIt(Constants.SW_REFERENCE_DATA_NOT_FOUND);
@@ -471,7 +473,7 @@ public final class SmartPGPApplet extends Applet implements ExtendedLength {
             return 0;
         }
 
-        final PGPKey k = currentTagOccurenceToKey();
+        final PGPKey k = currentTagOccurrenceToKey();
 
         if(k == null) {
             ISOException.throwIt(Constants.SW_REFERENCE_DATA_NOT_FOUND);
@@ -1063,7 +1065,7 @@ public final class SmartPGPApplet extends Applet implements ExtendedLength {
 
             case Constants.TAG_CARDHOLDER_CERTIFICATE:
                 assertAdmin();
-                k = currentTagOccurenceToKey();
+                k = currentTagOccurrenceToKey();
                 if(k == null) {
                     ISOException.throwIt(Constants.SW_REFERENCE_DATA_NOT_FOUND);
                     return;