From 563e31b460d7ae44a376a6354a2a1a777eb7b7df Mon Sep 17 00:00:00 2001 From: Robert Quattlebaum Date: Wed, 18 Mar 2020 14:19:51 -0700 Subject: [PATCH] New Counter/Presence Classes, Minor Refactor This commit primarily does two things: * Moves the counter logic out of `U2FApplet` and into its own class, `Counter`. This will allow it to be more easily refactored in the future. * Moves the user presence logic out of `U2FApplet`, creating a new interface called `Presence`. This will make it easier to implement devices which have a button for testing presence. Two implementations are provided: `OneShotPresence` (the default) and `NullPresence` (used when the install flags are set to 0x01). The functionality should be completely identical at this point, but the code should be easier to maintain and read. --- src/main/java/com/ledger/u2f/Counter.java | 66 ++++++++++++++ .../java/com/ledger/u2f/NullPresence.java | 11 +++ .../java/com/ledger/u2f/OneShotPresence.java | 38 ++++++++ src/main/java/com/ledger/u2f/Presence.java | 6 ++ src/main/java/com/ledger/u2f/U2FApplet.java | 88 ++++++------------- 5 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 src/main/java/com/ledger/u2f/Counter.java create mode 100644 src/main/java/com/ledger/u2f/NullPresence.java create mode 100644 src/main/java/com/ledger/u2f/OneShotPresence.java create mode 100644 src/main/java/com/ledger/u2f/Presence.java diff --git a/src/main/java/com/ledger/u2f/Counter.java b/src/main/java/com/ledger/u2f/Counter.java new file mode 100644 index 0000000..be53295 --- /dev/null +++ b/src/main/java/com/ledger/u2f/Counter.java @@ -0,0 +1,66 @@ +package com.ledger.u2f; + +import javacard.framework.ISO7816; +import javacard.framework.ISOException; +import javacard.framework.JCSystem; +import javacard.framework.Util; + +/** + * Simple incrementing counter. + * + * This class implements a simple unsigned 32-bit counter + * that always counts up and cannot be decremented. Once + * it reaches the value 0xFFFFFFFF, all future operations + * throw ISOException with a value of ISO7816.SW_FILE_FULL. + * + * TODO: This should be rewritten to improve flash wear + * leveling behavior and device longevity. + */ +public class Counter { + private byte[] counter; + private boolean overflow; + + Counter() { + counter = new byte[4]; + overflow = false; + } + + public void inc() { + boolean carry = false; + + if (overflow) { + // Game over + ISOException.throwIt(ISO7816.SW_FILE_FULL); + } + + JCSystem.beginTransaction(); + + for (byte i=0; i<4; i++) { + short addValue = (i == 0 ? (short)1 : (short)0); + short val = (short)((short)(counter[(short)(4 - 1 - i)] & 0xff) + addValue); + if (carry) { + val++; + } + carry = (val > 255); + counter[(short)(4 - 1 - i)] = (byte)val; + } + + if (carry) { + // Game over + overflow = true; + } + + JCSystem.commitTransaction(); + + if (overflow) { + ISOException.throwIt(ISO7816.SW_FILE_FULL); + } + } + + public short writeValue(byte[] dest, short destOffset) { + if (overflow) { + ISOException.throwIt(ISO7816.SW_FILE_FULL); + } + return Util.arrayCopyNonAtomic(counter, (short)0, dest, destOffset, (short)4); + } +} diff --git a/src/main/java/com/ledger/u2f/NullPresence.java b/src/main/java/com/ledger/u2f/NullPresence.java new file mode 100644 index 0000000..ae3ef0b --- /dev/null +++ b/src/main/java/com/ledger/u2f/NullPresence.java @@ -0,0 +1,11 @@ +package com.ledger.u2f; + +/** + * Null user presence detection. + * Suitable for testing only. + */ +public class NullPresence implements Presence { + @Override + public void verify_user_presence() { + } +} diff --git a/src/main/java/com/ledger/u2f/OneShotPresence.java b/src/main/java/com/ledger/u2f/OneShotPresence.java new file mode 100644 index 0000000..9c96f0b --- /dev/null +++ b/src/main/java/com/ledger/u2f/OneShotPresence.java @@ -0,0 +1,38 @@ +package com.ledger.u2f; + +import javacard.framework.ISO7816; +import javacard.framework.ISOException; +import javacard.framework.JCSystem; + +/** + * User presence detection that works based on the + * presentation of the token rather than an interaction + * with the token. + * + * This means that, for a presented token, + * only a single operation requiring user presence can + * be performed until the card is reset. + * + * This is suitable for NFC-based authenticators, and + * USB authenticators that don't have buttons. + * + * Note that in many cases the card can be reset by + * software without any physical user action, rendering + * this protection moot. Without a physical button this + * is the best that can be done. + */ +public class OneShotPresence implements Presence { + private byte[] did_verify_flag; + + OneShotPresence() { + did_verify_flag = JCSystem.makeTransientByteArray((short)1, JCSystem.CLEAR_ON_RESET); + } + + @Override + public void verify_user_presence() { + if (did_verify_flag[0] != 0) { + ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); + } + did_verify_flag[0] = 1; + } +} diff --git a/src/main/java/com/ledger/u2f/Presence.java b/src/main/java/com/ledger/u2f/Presence.java new file mode 100644 index 0000000..01b26da --- /dev/null +++ b/src/main/java/com/ledger/u2f/Presence.java @@ -0,0 +1,6 @@ +package com.ledger.u2f; + +public interface Presence { + + void verify_user_presence(); +} diff --git a/src/main/java/com/ledger/u2f/U2FApplet.java b/src/main/java/com/ledger/u2f/U2FApplet.java index 7de82cb..e70448b 100644 --- a/src/main/java/com/ledger/u2f/U2FApplet.java +++ b/src/main/java/com/ledger/u2f/U2FApplet.java @@ -19,32 +19,19 @@ package com.ledger.u2f; -import javacard.framework.APDU; -import javacard.framework.Applet; -import javacard.framework.ISO7816; -import javacard.framework.ISOException; -import javacard.framework.JCSystem; -import javacard.framework.Util; - -import javacard.security.KeyBuilder; +import javacard.framework.*; +import javacard.security.*; import javacardx.apdu.ExtendedLength; -import javacard.security.ECPrivateKey; -import javacard.security.ECPublicKey; -import javacard.security.Signature; -import javacard.security.CryptoException; public class U2FApplet extends Applet implements ExtendedLength { - private byte flags; - private byte[] counter; - private byte[] scratchPersistent; + private Counter counter; + private Presence presence; private byte[] scratch; private byte[] attestationCertificate; private boolean attestationCertificateSet; - private ECPrivateKey attestationPrivateKey; private ECPrivateKey localPrivateKey; private boolean localPrivateTransient; - private boolean counterOverflowed; private Signature attestationSignature; private Signature localSignature; private FIDOAPI fidoImpl; @@ -113,8 +100,7 @@ public U2FApplet(byte[] parameters, short parametersOffset, byte parametersLengt if (parametersLength != 35) { ISOException.throwIt(ISO7816.SW_WRONG_DATA); } - counter = new byte[4]; - scratchPersistent = JCSystem.makeTransientByteArray((short)1, JCSystem.CLEAR_ON_RESET); + counter = new Counter(); scratch = JCSystem.makeTransientByteArray((short)(SCRATCH_PAD + SCRATCH_PAD_SIZE), JCSystem.CLEAR_ON_DESELECT); try { // ok, let's save RAM @@ -135,9 +121,16 @@ public U2FApplet(byte[] parameters, short parametersOffset, byte parametersLengt } attestationSignature = Signature.getInstance(Signature.ALG_ECDSA_SHA_256, false); localSignature = Signature.getInstance(Signature.ALG_ECDSA_SHA_256, false); - flags = parameters[parametersOffset]; + byte flags = parameters[parametersOffset]; + + if ((flags & INSTALL_FLAG_DISABLE_USER_PRESENCE) == 0) { + presence = new OneShotPresence(); + } else { + presence = new NullPresence(); + } + attestationCertificate = new byte[Util.getShort(parameters, (short)(parametersOffset + 1))]; - attestationPrivateKey = (ECPrivateKey)KeyBuilder.buildKey(KeyBuilder.TYPE_EC_FP_PRIVATE, KeyBuilder.LENGTH_EC_FP_256, false); + ECPrivateKey attestationPrivateKey = (ECPrivateKey) KeyBuilder.buildKey(KeyBuilder.TYPE_EC_FP_PRIVATE, KeyBuilder.LENGTH_EC_FP_256, false); Secp256r1.setCommonCurveParameters(attestationPrivateKey); attestationPrivateKey.setS(parameters, (short)(parametersOffset + 3), (short)32); attestationSignature.init(attestationPrivateKey, Signature.MODE_SIGN); @@ -164,21 +157,14 @@ private void handleEnroll(APDU apdu) throws ISOException { short dataOffset = apdu.getOffsetCdata(); boolean extendedLength = (dataOffset != ISO7816.OFFSET_CDATA); short outOffset; + if (len != 64) { ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); } + // Deny if user presence cannot be validated - if ((flags & INSTALL_FLAG_DISABLE_USER_PRESENCE) == 0) { - if (scratchPersistent[0] != 0) { - ISOException.throwIt(FIDO_SW_TEST_OF_PRESENCE_REQUIRED); - } - } - // Check if the counter overflowed - if (counterOverflowed) { - ISOException.throwIt(ISO7816.SW_FILE_FULL); - } - // Set user presence - scratchPersistent[0] = (byte)1; + presence.verify_user_presence(); + // Generate the key pair if (localPrivateTransient) { Secp256r1.setCommonCurveParameters(localPrivateKey); @@ -222,7 +208,6 @@ private void handleSign(APDU apdu) throws ISOException { short dataOffset = apdu.getOffsetCdata(); byte p1 = buffer[ISO7816.OFFSET_P1]; boolean sign = false; - boolean counterOverflow = true; short keyHandleLength; boolean extendedLength = (dataOffset != ISO7816.OFFSET_CDATA); short outOffset = SCRATCH_PAD; @@ -238,10 +223,6 @@ private void handleSign(APDU apdu) throws ISOException { default: ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2); } - // Check if the counter overflowed - if (counterOverflowed) { - ISOException.throwIt(ISO7816.SW_FILE_FULL); - } // Verify key handle if (localPrivateTransient) { Secp256r1.setCommonCurveParameters(localPrivateKey); @@ -250,38 +231,23 @@ private void handleSign(APDU apdu) throws ISOException { if (!fidoImpl.unwrap(buffer, (short)(dataOffset + 65), keyHandleLength, buffer, (short)(dataOffset + APDU_APPLICATION_PARAMETER_OFFSET), (sign ? localPrivateKey : null))) { ISOException.throwIt(FIDO_SW_INVALID_KEY_HANDLE); } + // If not signing, return with the "correct" exception if (!sign) { ISOException.throwIt(FIDO_SW_TEST_OF_PRESENCE_REQUIRED); } + // If signing, only proceed if user presence can be validated - if ((flags & INSTALL_FLAG_DISABLE_USER_PRESENCE) == 0) { - if (scratchPersistent[0] != 0) { - ISOException.throwIt(FIDO_SW_TEST_OF_PRESENCE_REQUIRED); - } - } - scratchPersistent[0] = (byte)1; + presence.verify_user_presence(); + // Increase the counter - boolean carry = false; - JCSystem.beginTransaction(); - for (byte i=0; i<4; i++) { - short addValue = (i == 0 ? (short)1 : (short)0); - short val = (short)((short)(counter[(short)(4 - 1 - i)] & 0xff) + addValue); - if (carry) { - val++; - } - carry = (val > 255); - counter[(short)(4 - 1 - i)] = (byte)val; - } - JCSystem.commitTransaction(); - if (carry) { - // Game over - counterOverflowed = true; - ISOException.throwIt(ISO7816.SW_FILE_FULL); - } + counter.inc(); + // Prepare reply scratch[outOffset++] = FLAG_USER_PRESENCE_VERIFIED; - outOffset = Util.arrayCopyNonAtomic(counter, (short)0, scratch, outOffset, (short)4); + + outOffset = counter.writeValue(scratch, outOffset); + localSignature.init(localPrivateKey, Signature.MODE_SIGN); localSignature.update(buffer, (short)(dataOffset + APDU_APPLICATION_PARAMETER_OFFSET), (short)32); localSignature.update(scratch, SCRATCH_PAD, (short)5);