status-im
/
status-keycard
mirror of https://github.com/status-im/status-keycard.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

check and fix the S field of the ECDSA signature to avoid malleability

This commit is contained in:
Michele Balistreri 2018-01-19 15:40:36 +03:00
parent e15da6a7e0
commit d475900300
3 changed files with 54 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
src/main/java/im/status/wallet/Crypto.java
View File

@ -13,10 +13,13 @@ public class Crypto {
final static private short KEY_DERIVATION_INPUT_SIZE = 37;
final static private short HMAC_OUT_SIZE = 64;
private static final byte HMAC_IPAD = (byte) 0x36;
private static final byte HMAC_OPAD = (byte) 0x5c;
private static final short HMAC_BLOCK_SIZE = (short) 128;
private static final short HMAC_BLOCK_OFFSET = (short) KEY_DERIVATION_INPUT_SIZE + HMAC_OUT_SIZE;
final static private byte[] MAX_S = { (byte) 0x7F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0x5D, (byte) 0x57, (byte) 0x6E, (byte) 0x73, (byte) 0x57, (byte) 0xA4, (byte) 0x50, (byte) 0x1D, (byte) 0xDF, (byte) 0xE9, (byte) 0x2F, (byte) 0x46, (byte) 0x68, (byte) 0x1B, (byte) 0x20, (byte) 0xA0 };
final static private byte[] S_SUB = { (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFE, (byte) 0xBA, (byte) 0xAE, (byte) 0xDC, (byte) 0xE6, (byte) 0xAF, (byte) 0x48, (byte) 0xA0, (byte) 0x3B, (byte) 0xBF, (byte) 0xD2, (byte) 0x5E, (byte) 0x8C, (byte) 0xD0, (byte) 0x36, (byte) 0x41, (byte) 0x41 };
final static private byte HMAC_IPAD = (byte) 0x36;
final static private byte HMAC_OPAD = (byte) 0x5c;
final static private short HMAC_BLOCK_SIZE = (short) 128;
final static private short HMAC_BLOCK_OFFSET = (short) KEY_DERIVATION_INPUT_SIZE + HMAC_OUT_SIZE;
// The below 4 objects can be accessed anywhere from the entire applet
static RandomData random;
@ -99,6 +102,34 @@ public class Crypto {
return true;
}
/**
* Fixes the S value of the signature as described in BIP-62 to avoid malleable signatures. It also fixes the all
* internal TLV length fields. Returns the number of bytes by which the overall signature length changed (0 or -1).
*
* @param sig the signature
* @param off the offset
* @return the number of bytes by which the signature length changed
*/
static short fixS(byte[] sig, short off) {
short sOff = (short) (sig[(short) (off + 3)] + (short) (off + 5));
short ret = 0;
if (sig[sOff] == 33) {
Util.arrayCopyNonAtomic(sig, (short) (sOff + 2), sig, (short) (sOff + 1), (short) 32);
sig[sOff] = 32;
sig[(short)(off + 1)]--;
ret = -1;
}
sOff++;
if (ret == -1 || ucmp256(sig, sOff, MAX_S, (short) 0) > 0) {
sub256(S_SUB, (short) 0, sig, sOff, sig, sOff);
}
return ret;
}
/**
* Calculates the HMAC-SHA512 with the given key and data. Uses a software implementation which only requires SHA-512
* to be supported on cards which do not have native HMAC-SHA512.
@ -155,7 +186,7 @@ public class Crypto {
}
/**
* Compares two 256-bit numbers. Returns 1 if a > b, -1 if a < b and 0 if a = b.
* Compares two 256-bit numbers. Returns a positive number if a > b, a negative one if a < b and 0 if a = b.
*
* @param a the a operand
* @param aOff the offset of the a operand

9
src/main/java/im/status/wallet/WalletApplet.java
View File

@ -803,15 +803,18 @@ public class WalletApplet extends Applet {
short outLen = apduBuffer[(short)(SecureChannel.SC_OUT_OFFSET + 4)] = (byte) publicKey.getW(apduBuffer, (short) (SecureChannel.SC_OUT_OFFSET + 5));
outLen += 5;
short sigOff = (short) (SecureChannel.SC_OUT_OFFSET + outLen);
if ((apduBuffer[ISO7816.OFFSET_P1]) == SIGN_P1_DATA) {
outLen += signature.sign(apduBuffer, ISO7816.OFFSET_CDATA, len, apduBuffer, (short) (SecureChannel.SC_OUT_OFFSET + outLen));
outLen += signature.sign(apduBuffer, ISO7816.OFFSET_CDATA, len, apduBuffer, sigOff);
} else if ((apduBuffer[ISO7816.OFFSET_P1]) == SIGN_P1_PRECOMPUTED_HASH) {
outLen += signature.signPreComputedHash(apduBuffer, ISO7816.OFFSET_CDATA, len, apduBuffer, (short) (SecureChannel.SC_OUT_OFFSET + outLen));
outLen += signature.signPreComputedHash(apduBuffer, ISO7816.OFFSET_CDATA, len, apduBuffer, sigOff);
} else {
ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
}
outLen += Crypto.fixS(apduBuffer, sigOff);
apduBuffer[(short)(SecureChannel.SC_OUT_OFFSET + 1)] = (byte) 0x81;
apduBuffer[(short)(SecureChannel.SC_OUT_OFFSET + 2)] = (byte) (outLen - 3);
@ -854,7 +857,7 @@ public class WalletApplet extends Applet {
* Processes the EXPORT KEY command. Requires an open secure channel and the PIN to be verified. The P1 parameter is
* an index to which key must be exported from the list of exportable ones. At the moment only the Whisper key with
* key path m/1/1 is exportable. The key is exported only if the current key path matches the key path of the key to
* be exported.
* be exported. The public key of the current path can always be exported with P1=0x00 and P2=0x01.
*
* @param apdu the JCRE-owned APDU object.
*/

12
src/test/java/im/status/wallet/WalletAppletTest.java
View File

@ -704,6 +704,7 @@ public class WalletAppletTest {
assertEquals((SecureChannel.SC_KEY_LENGTH * 2 / 8) + 1, keyData.length);
signature.update(data);
assertTrue(signature.verify(sig));
assertFalse(isMalleable(sig));
}
@Test
@ -1145,6 +1146,17 @@ public class WalletAppletTest {
return key;
}
private boolean isMalleable(byte[] sig) {
int rLen = sig[3];
int sOff = 6 + rLen;
int sLen = sig.length - rLen - 6;
BigInteger s = new BigInteger(Arrays.copyOfRange(sig, sOff, sOff + sLen));
BigInteger limit = new BigInteger("7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0", 16);
return s.compareTo(limit) >= 1;
}
/**
* This method takes the response from the first stage of an assisted key derivation command and derives the complete
* public key from the received X and signature. Outside of test code, proper TLV parsing would be a better idea, here