From f8abbd8732f20b8aabec75883667182f0c3e8ac2 Mon Sep 17 00:00:00 2001 From: Arnaud Fontaine Date: Thu, 6 Jul 2017 18:05:59 +0200 Subject: [PATCH] Update AES use in SmartPGP (ENC/DEC/SM) --- src/fr/anssi/smartpgp/CmacKey.java | 4 +- src/fr/anssi/smartpgp/Constants.java | 5 -- src/fr/anssi/smartpgp/PGPKey.java | 10 +-- src/fr/anssi/smartpgp/Persistent.java | 23 ++++--- src/fr/anssi/smartpgp/SecureMessaging.java | 80 +++++++++++++++------- src/fr/anssi/smartpgp/SmartPGPApplet.java | 18 +++-- 6 files changed, 86 insertions(+), 54 deletions(-) diff --git a/src/fr/anssi/smartpgp/CmacKey.java b/src/fr/anssi/smartpgp/CmacKey.java index 569f86a..bd060e4 100644 --- a/src/fr/anssi/smartpgp/CmacKey.java +++ b/src/fr/anssi/smartpgp/CmacKey.java @@ -31,9 +31,9 @@ public final class CmacKey { protected final byte[] k1; protected final byte[] k2; - protected CmacKey() { + protected CmacKey(final byte aesKeyLength) { key = (AESKey)KeyBuilder.buildKey(KeyBuilder.TYPE_AES_TRANSIENT_DESELECT, - (short)(Constants.aesKeyLength() * 8), + (short)(aesKeyLength * 8), false); k1 = JCSystem.makeTransientByteArray(Constants.AES_BLOCK_SIZE, JCSystem.CLEAR_ON_DESELECT); diff --git a/src/fr/anssi/smartpgp/Constants.java b/src/fr/anssi/smartpgp/Constants.java index 3532a92..2c13ea5 100644 --- a/src/fr/anssi/smartpgp/Constants.java +++ b/src/fr/anssi/smartpgp/Constants.java @@ -171,10 +171,6 @@ public final class Constants { (byte)0x00 /* MSE not supported */ }; - protected static final short aesKeyLength() { - return (short)(16 * EXTENDED_CAPABILITIES[1]); - } - protected static final short challengeMaxLength() { return Util.getShort(EXTENDED_CAPABILITIES, (short)2); } @@ -239,7 +235,6 @@ protected static final short specialDoMaxLength() { protected static final byte[] RSA_EXPONENT = { (byte)0x01, (byte)0x00, (byte)0x01 }; - protected static final short AES_BLOCK_SIZE = (short)16; } diff --git a/src/fr/anssi/smartpgp/PGPKey.java b/src/fr/anssi/smartpgp/PGPKey.java index 13efb66..b3a53c0 100644 --- a/src/fr/anssi/smartpgp/PGPKey.java +++ b/src/fr/anssi/smartpgp/PGPKey.java @@ -167,13 +167,9 @@ protected final void setAttributes(final ECCurves ec, ISOException.throwIt(ISO7816.SW_WRONG_DATA); return; } - if(is_secure_messaging_key) { - if((buf[0] != 0x12) || - (((short)(Constants.aesKeyLength()* 8) > (short)128) && - (params.nb_bits < 512))) { - ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); - return; - } + if((buf[0] != 0x12) && is_secure_messaging_key) { + ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); + return; } break; diff --git a/src/fr/anssi/smartpgp/Persistent.java b/src/fr/anssi/smartpgp/Persistent.java index 513563e..3e7c2ea 100644 --- a/src/fr/anssi/smartpgp/Persistent.java +++ b/src/fr/anssi/smartpgp/Persistent.java @@ -56,26 +56,26 @@ public final class Persistent { protected byte sex; - protected byte[] digital_signature_counter; + protected final byte[] digital_signature_counter; - protected byte[] do_0101; + protected final byte[] do_0101; protected short do_0101_length; - protected byte[] do_0102; + protected final byte[] do_0102; protected short do_0102_length; - protected byte[] do_0103; + protected final byte[] do_0103; protected short do_0103_length; - protected byte[] do_0104; + protected final byte[] do_0104; protected short do_0104_length; protected AESKey aes_key; - protected byte[] key_derivation_function; + protected final byte[] key_derivation_function; protected short key_derivation_function_length; protected final OwnerPIN user_pin; /* PW1 */ @@ -122,9 +122,7 @@ protected Persistent() { do_0104 = new byte[Constants.specialDoMaxLength()]; do_0104_length = 0; - aes_key = (AESKey)KeyBuilder.buildKey(KeyBuilder.TYPE_AES, - (short)(Constants.aesKeyLength() * 8), - false); + aes_key = null; pgp_keys = new PGPKey[PGP_KEYS_LENGTH]; for(byte i = 0; i < pgp_keys.length; ++i) { @@ -218,7 +216,12 @@ protected void reset() { } JCSystem.commitTransaction(); - aes_key.clearKey(); + JCSystem.beginTransaction(); + if(aes_key != null) { + aes_key.clearKey(); + aes_key = null; + } + JCSystem.commitTransaction(); user_pin_force_verify_signature = Constants.USER_PIN_DEFAULT_FORCE_VERIFY_SIGNATURE; diff --git a/src/fr/anssi/smartpgp/SecureMessaging.java b/src/fr/anssi/smartpgp/SecureMessaging.java index 3229313..3f90ae2 100644 --- a/src/fr/anssi/smartpgp/SecureMessaging.java +++ b/src/fr/anssi/smartpgp/SecureMessaging.java @@ -41,12 +41,11 @@ public final class SecureMessaging { private final byte[] mac_chaining; private final Cipher cipher; - private final AESKey senc; private final CmacSignature macer; - private final CmacKey smac; - private final CmacKey srmac; - private final CmacKey sreceiptmac; + private AESKey senc; + private CmacKey smac; + private CmacKey srmac; protected final PGPKey static_key; @@ -61,12 +60,9 @@ protected SecureMessaging(final Transients transients) { mac_chaining = JCSystem.makeTransientByteArray(Constants.AES_BLOCK_SIZE, JCSystem.CLEAR_ON_DESELECT); - senc = (AESKey)KeyBuilder.buildKey(KeyBuilder.TYPE_AES_TRANSIENT_DESELECT, - (short)(Constants.aesKeyLength() * 8), - false); - smac = new CmacKey(); - srmac = new CmacKey(); - sreceiptmac = new CmacKey(); + senc = null; + smac = null; + srmac = null; static_key = new PGPKey(true); @@ -74,10 +70,18 @@ protected SecureMessaging(final Transients transients) { } protected final void clearSession(final Transients transients) { - senc.clearKey(); - smac.clearKey(); - srmac.clearKey(); - sreceiptmac.clearKey(); + if(senc != null) { + senc.clearKey(); + senc = null; + } + if(smac != null) { + smac.clearKey(); + smac = null; + } + if(srmac != null) { + srmac.clearKey(); + srmac = null; + } macer.clear(); transients.setSecureMessagingEncryptionCounter((short)0); Util.arrayFillNonAtomic(iv, (short)0, (short)iv.length, (byte)0); @@ -95,9 +99,17 @@ protected final boolean isInitialized() { protected final boolean isSessionAvailable() { return isInitialized() - && senc.isInitialized() - && smac.isInitialized() - && srmac.isInitialized(); + && (senc != null) && senc.isInitialized() + && (smac != null) && smac.isInitialized() + && (srmac != null) && srmac.isInitialized(); + } + + private static final byte aesKeyLength(final ECParams params) { + if(params.nb_bits < (short)512) { + return (byte)16; + } else { + return (byte)32; + } } private final short scp11b(final ECParams params, @@ -123,7 +135,7 @@ private final short scp11b(final ECParams params, short off = (short)crt.length; - if(buf[off] != Constants.aesKeyLength()) { + if(buf[off] != aesKeyLength(params)) { ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); return 0; } @@ -207,10 +219,31 @@ private final short scp11b(final ECParams params, buf, (short)(len + msglen + keydata_len)); } + final CmacKey sreceiptmac = new CmacKey(aesKeyLength(params)); sreceiptmac.setKey(buf, (short)(len + msglen)); - senc.setKey(buf, (short)(len + msglen + Constants.aesKeyLength())); - smac.setKey(buf, (short)(len + msglen + 2 * Constants.aesKeyLength())); - srmac.setKey(buf, (short)(len + msglen + 3 * Constants.aesKeyLength())); + + if(senc != null) { + senc.clearKey(); + senc = null; + } + senc = (AESKey)KeyBuilder.buildKey(KeyBuilder.TYPE_AES_TRANSIENT_DESELECT, + (short)(aesKeyLength(params) * 8), + false); + senc.setKey(buf, (short)(len + msglen + aesKeyLength(params))); + + if(smac != null) { + smac.clearKey(); + smac = null; + } + smac = new CmacKey(aesKeyLength(params)); + smac.setKey(buf, (short)(len + msglen + 2 * aesKeyLength(params))); + + if(srmac != null) { + srmac.clearKey(); + srmac = null; + } + srmac = new CmacKey(aesKeyLength(params)); + srmac.setKey(buf, (short)(len + msglen + 3 * aesKeyLength(params))); Util.arrayFillNonAtomic(buf, len, (short)(msglen + keydata_len), (byte)0); @@ -254,10 +287,7 @@ protected final short establish(final Transients transients, final ECParams params = static_key.ecParams(ec); if(params != null) { - if(((short)(Constants.aesKeyLength() * 8) == (short)128) || - (params.nb_bits >= 512)) { - return scp11b(params, buf, len); - } + return scp11b(params, buf, len); } } diff --git a/src/fr/anssi/smartpgp/SmartPGPApplet.java b/src/fr/anssi/smartpgp/SmartPGPApplet.java index bc32b2c..7398609 100644 --- a/src/fr/anssi/smartpgp/SmartPGPApplet.java +++ b/src/fr/anssi/smartpgp/SmartPGPApplet.java @@ -926,11 +926,19 @@ private final void processPutData(final short lc, case Constants.TAG_AES_KEY: assertAdmin(); - if(lc != Constants.aesKeyLength()) { + if((lc != (short)16) && (lc != (short)32)) { ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); return; } + JCSystem.beginTransaction(); + if(data.aes_key != null) { + data.aes_key.clearKey(); + } + data.aes_key = (AESKey)KeyBuilder.buildKey(KeyBuilder.TYPE_AES, + (short)(lc * 8), + false); data.aes_key.setKey(buf, (short)0); + JCSystem.commitTransaction(); break; case Constants.TAG_CARDHOLDER_CERTIFICATE: @@ -1164,12 +1172,12 @@ private final short processPerformSecurityOperation(final short lc, if(transients.buffer[0] == (byte)0x02) { - if(((short)(lc - 1) % Constants.aesKeyLength()) != 0) { + if(((short)(lc - 1) % Constants.AES_BLOCK_SIZE) != 0) { ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); return 0; } - if(!data.aes_key.isInitialized()) { + if((data.aes_key == null) || !data.aes_key.isInitialized()) { ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); return 0; } @@ -1196,12 +1204,12 @@ private final short processPerformSecurityOperation(final short lc, assertUserMode82(); - if((lc <= 0) || ((lc % Constants.aesKeyLength()) != 0)) { + if((lc <= 0) || ((lc % Constants.AES_BLOCK_SIZE) != 0)) { ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); return 0; } - if(!data.aes_key.isInitialized()) { + if((data.aes_key == null) || !data.aes_key.isInitialized()) { ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); return 0; }