Merge branch 'constant-time-mac-verification' into 'master'

Add constant-time method for verifying MACs

See merge request akwizgran/briar!773
This commit is contained in:
Torsten Grote
2018-04-25 12:08:49 +00:00
20 changed files with 120 additions and 72 deletions

View File

@@ -105,8 +105,8 @@ public interface ClientHelper {
byte[] sign(String label, BdfList toSign, byte[] privateKey)
throws FormatException, GeneralSecurityException;
void verifySignature(String label, byte[] sig, byte[] publicKey,
BdfList signed) throws FormatException, GeneralSecurityException;
void verifySignature(byte[] signature, String label, BdfList signed,
byte[] publicKey) throws FormatException, GeneralSecurityException;
Author parseAndValidateAuthor(BdfList author) throws FormatException;

View File

@@ -67,8 +67,8 @@ public interface CryptoComponent {
* signature created for another purpose
* @return true if the signature was valid, false otherwise.
*/
boolean verify(String label, byte[] signedData, byte[] publicKey,
byte[] signature) throws GeneralSecurityException;
boolean verifySignature(byte[] signature, String label, byte[] signed,
byte[] publicKey) throws GeneralSecurityException;
/**
* Returns the hash of the given inputs. The inputs are unambiguously
@@ -91,6 +91,18 @@ public interface CryptoComponent {
*/
byte[] mac(String label, SecretKey macKey, byte[]... inputs);
/**
* Verifies that the given message authentication code is valid for the
* given secret key and inputs.
*
* @param label a namespaced label indicating the purpose of this MAC, to
* prevent it from being repurposed or colliding with a MAC created for
* another purpose
* @return true if the MAC was valid, false otherwise.
*/
boolean verifyMac(byte[] mac, String label, SecretKey macKey,
byte[]... inputs);
/**
* Encrypts and authenticates the given plaintext so it can be written to
* storage. The encryption and authentication keys are derived from the

View File

@@ -381,9 +381,10 @@ class ClientHelperImpl implements ClientHelper {
}
@Override
public void verifySignature(String label, byte[] sig, byte[] publicKey,
BdfList signed) throws FormatException, GeneralSecurityException {
if (!crypto.verify(label, toByteArray(signed), publicKey, sig)) {
public void verifySignature(byte[] signature, String label, BdfList signed,
byte[] publicKey) throws FormatException, GeneralSecurityException {
if (!crypto.verifySignature(signature, label, toByteArray(signed),
publicKey)) {
throw new GeneralSecurityException("Invalid signature");
}
}

View File

@@ -251,7 +251,8 @@ class ContactExchangeTaskImpl extends Thread implements ContactExchangeTask {
r.readListEnd();
LOG.info("Received pseudonym");
// Verify the signature
if (!crypto.verify(SIGNING_LABEL_EXCHANGE, nonce, publicKey, sig)) {
if (!crypto.verifySignature(sig, SIGNING_LABEL_EXCHANGE, nonce,
publicKey)) {
if (LOG.isLoggable(INFO))
LOG.info("Invalid signature");
throw new GeneralSecurityException();

View File

@@ -205,12 +205,12 @@ class CryptoComponentImpl implements CryptoComponent {
}
@Override
public boolean verify(String label, byte[] signedData, byte[] publicKey,
byte[] signature) throws GeneralSecurityException {
public boolean verifySignature(byte[] signature, String label,
byte[] signed, byte[] publicKey) throws GeneralSecurityException {
PublicKey key = signatureKeyParser.parsePublicKey(publicKey);
Signature sig = new EdSignature();
sig.initVerify(key);
updateSignature(sig, label, signedData);
updateSignature(sig, label, signed);
return sig.verify(signature);
}
@@ -262,6 +262,17 @@ class CryptoComponentImpl implements CryptoComponent {
return output;
}
@Override
public boolean verifyMac(byte[] mac, String label, SecretKey macKey,
byte[]... inputs) {
byte[] expected = mac(label, macKey, inputs);
if (mac.length != expected.length) return false;
// Constant-time comparison
int cmp = 0;
for (int i = 0; i < mac.length; i++) cmp |= mac[i] ^ expected[i];
return cmp == 0;
}
@Override
public byte[] encryptWithPassword(byte[] input, String password) {
AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher();

View File

@@ -37,6 +37,7 @@ import java.util.Random;
import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_SIGNATURE_LENGTH;
import static org.briarproject.bramble.test.TestUtils.getAuthor;
import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
import static org.briarproject.bramble.test.TestUtils.getRandomId;
@@ -300,30 +301,34 @@ public class ClientHelperImplTest extends BrambleTestCase {
@Test
public void testVerifySignature() throws Exception {
byte[] signature = getRandomBytes(MAX_SIGNATURE_LENGTH);
byte[] publicKey = getRandomBytes(42);
byte[] bytes = expectToByteArray(list);
byte[] signed = expectToByteArray(list);
context.checking(new Expectations() {{
oneOf(cryptoComponent).verify(label, bytes, publicKey, rawMessage);
oneOf(cryptoComponent).verifySignature(signature, label, signed,
publicKey);
will(returnValue(true));
}});
clientHelper.verifySignature(label, rawMessage, publicKey, list);
clientHelper.verifySignature(signature, label, list, publicKey);
context.assertIsSatisfied();
}
@Test
public void testVerifyWrongSignature() throws Exception {
byte[] signature = getRandomBytes(MAX_SIGNATURE_LENGTH);
byte[] publicKey = getRandomBytes(42);
byte[] bytes = expectToByteArray(list);
byte[] signed = expectToByteArray(list);
context.checking(new Expectations() {{
oneOf(cryptoComponent).verify(label, bytes, publicKey, rawMessage);
oneOf(cryptoComponent).verifySignature(signature, label, signed,
publicKey);
will(returnValue(false));
}});
try {
clientHelper.verifySignature(label, rawMessage, publicKey, list);
clientHelper.verifySignature(signature, label, list, publicKey);
fail();
} catch (GeneralSecurityException e) {
// expected

View File

@@ -143,9 +143,9 @@ public class EdSignatureTest extends SignatureTest {
}
@Override
protected boolean verify(String label, byte[] signedData, byte[] publicKey,
byte[] signature) throws GeneralSecurityException {
return crypto.verify(label, signedData, publicKey, signature);
protected boolean verify(byte[] signature, String label, byte[] signed,
byte[] publicKey) throws GeneralSecurityException {
return crypto.verifySignature(signature, label, signed, publicKey);
}
@Test

View File

@@ -126,13 +126,13 @@ public class KeyEncodingAndParsingTest extends BrambleTestCase {
byte[] signature = crypto.sign("test", message,
privateKey.getEncoded());
// Verify the signature
assertTrue(crypto.verify("test", message, publicKey.getEncoded(),
signature));
assertTrue(crypto.verifySignature(signature, "test", message,
publicKey.getEncoded()));
// Encode and parse the public key - no exceptions should be thrown
publicKey = parser.parsePublicKey(publicKey.getEncoded());
// Verify the signature again
assertTrue(crypto.verify("test", message, publicKey.getEncoded(),
signature));
assertTrue(crypto.verifySignature(signature, "test", message,
publicKey.getEncoded()));
}
@Test
@@ -146,15 +146,15 @@ public class KeyEncodingAndParsingTest extends BrambleTestCase {
byte[] signature = crypto.sign("test", message,
privateKey.getEncoded());
// Verify the signature
assertTrue(crypto.verify("test", message, publicKey.getEncoded(),
signature));
assertTrue(crypto.verifySignature(signature, "test", message,
publicKey.getEncoded()));
// Encode and parse the private key - no exceptions should be thrown
privateKey = parser.parsePrivateKey(privateKey.getEncoded());
// Sign the data again - the signatures should be the same
byte[] signature1 = crypto.sign("test", message,
privateKey.getEncoded());
assertTrue(crypto.verify("test", message, publicKey.getEncoded(),
signature1));
assertTrue(crypto.verifySignature(signature1, "test", message,
publicKey.getEncoded()));
assertArrayEquals(signature, signature1);
}

View File

@@ -13,6 +13,7 @@ import static org.briarproject.bramble.test.TestUtils.getSecretKey;
import static org.briarproject.bramble.util.StringUtils.getRandomString;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
public class MacTest extends BrambleTestCase {
@@ -32,6 +33,7 @@ public class MacTest extends BrambleTestCase {
byte[] mac = crypto.mac(label1, key1, input1, input2, input3);
byte[] mac1 = crypto.mac(label1, key1, input1, input2, input3);
assertArrayEquals(mac, mac1);
assertTrue(crypto.verifyMac(mac, label1, key1, input1, input2, input3));
}
@Test
@@ -40,6 +42,11 @@ public class MacTest extends BrambleTestCase {
byte[] mac = crypto.mac(label1, key1, input1, input2, input3);
byte[] mac1 = crypto.mac(label2, key1, input1, input2, input3);
assertFalse(Arrays.equals(mac, mac1));
// Each MAC should fail to verify with the other MAC's label
assertFalse(crypto.verifyMac(mac, label2, key1, input1, input2,
input3));
assertFalse(crypto.verifyMac(mac1, label1, key2, input1, input2,
input3));
}
@Test
@@ -48,6 +55,11 @@ public class MacTest extends BrambleTestCase {
byte[] mac = crypto.mac(label1, key1, input1, input2, input3);
byte[] mac1 = crypto.mac(label1, key2, input1, input2, input3);
assertFalse(Arrays.equals(mac, mac1));
// Each MAC should fail to verify with the other MAC's key
assertFalse(crypto.verifyMac(mac, label1, key2, input1, input2,
input3));
assertFalse(crypto.verifyMac(mac1, label2, key1, input1, input2,
input3));
}
@Test
@@ -57,6 +69,11 @@ public class MacTest extends BrambleTestCase {
byte[] mac = crypto.mac(label1, key1, input1, input2, input3);
byte[] mac1 = crypto.mac(label1, key1, input3, input2, input1);
assertFalse(Arrays.equals(mac, mac1));
// Each MAC should fail to verify with the other MAC's inputs
assertFalse(crypto.verifyMac(mac, label1, key2, input3, input2,
input1));
assertFalse(crypto.verifyMac(mac1, label1, key1, input1, input2,
input3));
}
}

View File

@@ -28,8 +28,8 @@ public abstract class SignatureTest extends BrambleTestCase {
protected abstract byte[] sign(String label, byte[] toSign,
byte[] privateKey) throws GeneralSecurityException;
protected abstract boolean verify(String label, byte[] signedData,
byte[] publicKey, byte[] signature) throws GeneralSecurityException;
protected abstract boolean verify(byte[] signature, String label,
byte[] signed, byte[] publicKey) throws GeneralSecurityException;
SignatureTest() {
crypto = new CryptoComponentImpl(new TestSecureRandomProvider(), null);
@@ -85,7 +85,7 @@ public abstract class SignatureTest extends BrambleTestCase {
@Test
public void testSignatureVerification() throws Exception {
byte[] sig = sign(label, inputBytes, privateKey);
assertTrue(verify(label, inputBytes, publicKey, sig));
assertTrue(verify(sig, label, inputBytes, publicKey));
}
@Test
@@ -95,7 +95,7 @@ public abstract class SignatureTest extends BrambleTestCase {
byte[] privateKey2 = k2.getPrivate().getEncoded();
// calculate the signature with different key, should fail to verify
byte[] sig = sign(label, inputBytes, privateKey2);
assertFalse(verify(label, inputBytes, publicKey, sig));
assertFalse(verify(sig, label, inputBytes, publicKey));
}
@Test
@@ -104,7 +104,7 @@ public abstract class SignatureTest extends BrambleTestCase {
byte[] inputBytes2 = TestUtils.getRandomBytes(123);
// calculate the signature with different input, should fail to verify
byte[] sig = sign(label, inputBytes, privateKey);
assertFalse(verify(label, inputBytes2, publicKey, sig));
assertFalse(verify(sig, label, inputBytes2, publicKey));
}
@Test
@@ -113,7 +113,7 @@ public abstract class SignatureTest extends BrambleTestCase {
String label2 = StringUtils.getRandomString(42);
// calculate the signature with different label, should fail to verify
byte[] sig = sign(label, inputBytes, privateKey);
assertFalse(verify(label2, inputBytes, publicKey, sig));
assertFalse(verify(sig, label2, inputBytes, publicKey));
}
}

View File

@@ -111,8 +111,8 @@ class BlogPostValidator extends BdfMessageValidator {
Blog b = blogFactory.parseBlog(g);
Author a = b.getAuthor();
try {
clientHelper.verifySignature(SIGNING_LABEL_POST, sig,
a.getPublicKey(), signed);
clientHelper.verifySignature(sig, SIGNING_LABEL_POST, signed,
a.getPublicKey());
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}
@@ -157,8 +157,8 @@ class BlogPostValidator extends BdfMessageValidator {
Blog b = blogFactory.parseBlog(g);
Author a = b.getAuthor();
try {
clientHelper.verifySignature(SIGNING_LABEL_COMMENT, sig,
a.getPublicKey(), signed);
clientHelper.verifySignature(sig, SIGNING_LABEL_COMMENT,
signed, a.getPublicKey());
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}

View File

@@ -66,8 +66,8 @@ class ForumPostValidator extends BdfMessageValidator {
BdfList signed = BdfList.of(g.getId(), m.getTimestamp(), parent,
authorList, content);
try {
clientHelper.verifySignature(SIGNING_LABEL_POST, sig,
author.getPublicKey(), signed);
clientHelper.verifySignature(sig, SIGNING_LABEL_POST,
signed, author.getPublicKey());
} catch (GeneralSecurityException e) {
throw new InvalidMessageException(e);
}

View File

@@ -500,7 +500,7 @@ class IntroduceeManager {
byte[] key = localState.getRaw(PUBLIC_KEY);
// Verify the signature
if (!cryptoComponent.verify(SIGNING_LABEL, nonce, key, sig)) {
if (!cryptoComponent.verifySignature(sig, SIGNING_LABEL, nonce, key)) {
LOG.warning("Invalid nonce signature in ACK");
throw new GeneralSecurityException();
}

View File

@@ -112,8 +112,9 @@ class GroupMessageValidator extends BdfMessageValidator {
creator.getId(), member.getId(), g.getId(),
inviteTimestamp);
try {
clientHelper.verifySignature(SIGNING_LABEL_INVITE,
creatorSignature, creator.getPublicKey(), token);
clientHelper.verifySignature(creatorSignature,
SIGNING_LABEL_INVITE,
token, creator.getPublicKey());
} catch (GeneralSecurityException e) {
throw new FormatException();
}
@@ -128,8 +129,8 @@ class GroupMessageValidator extends BdfMessageValidator {
inviteList
);
try {
clientHelper.verifySignature(SIGNING_LABEL_JOIN, memberSignature,
member.getPublicKey(), signed);
clientHelper.verifySignature(memberSignature, SIGNING_LABEL_JOIN,
signed, member.getPublicKey());
} catch (GeneralSecurityException e) {
throw new FormatException();
}
@@ -165,8 +166,8 @@ class GroupMessageValidator extends BdfMessageValidator {
content
);
try {
clientHelper.verifySignature(SIGNING_LABEL_POST, signature,
member.getPublicKey(), signed);
clientHelper.verifySignature(signature, SIGNING_LABEL_POST,
signed, member.getPublicKey());
} catch (GeneralSecurityException e) {
throw new FormatException();
}

View File

@@ -94,8 +94,8 @@ class GroupInvitationValidator extends BdfMessageValidator {
privateGroup.getId()
);
try {
clientHelper.verifySignature(SIGNING_LABEL_INVITE, signature,
creator.getPublicKey(), signed);
clientHelper.verifySignature(signature, SIGNING_LABEL_INVITE,
signed, creator.getPublicKey());
} catch (GeneralSecurityException e) {
throw new FormatException();
}

View File

@@ -280,7 +280,7 @@ public class BlogPostValidatorTest extends BriarTestCase {
oneOf(clientHelper).toList(b.getAuthor());
will(returnValue(authorList));
oneOf(clientHelper)
.verifySignature(label, sig, author.getPublicKey(), signed);
.verifySignature(sig, label, signed, author.getPublicKey());
}});
}

View File

@@ -64,8 +64,8 @@ public class ForumPostValidatorTest extends ValidatorTestCase {
public void testAcceptsNullParentId() throws Exception {
expectCreateAuthor();
context.checking(new Expectations() {{
oneOf(clientHelper).verifySignature(SIGNING_LABEL_POST, signature,
authorPublicKey, signedWithoutParent);
oneOf(clientHelper).verifySignature(signature, SIGNING_LABEL_POST,
signedWithoutParent, authorPublicKey);
}});
BdfMessageContext messageContext = v.validateMessage(message, group,
@@ -139,8 +139,8 @@ public class ForumPostValidatorTest extends ValidatorTestCase {
expectCreateAuthor();
context.checking(new Expectations() {{
oneOf(clientHelper).verifySignature(SIGNING_LABEL_POST, signature,
authorPublicKey, signedWithShortContent);
oneOf(clientHelper).verifySignature(signature, SIGNING_LABEL_POST,
signedWithShortContent, authorPublicKey);
}});
BdfMessageContext messageContext = v.validateMessage(message, group,
@@ -189,8 +189,8 @@ public class ForumPostValidatorTest extends ValidatorTestCase {
throws Exception {
expectCreateAuthor();
context.checking(new Expectations() {{
oneOf(clientHelper).verifySignature(SIGNING_LABEL_POST, signature,
authorPublicKey, signedWithParent);
oneOf(clientHelper).verifySignature(signature, SIGNING_LABEL_POST,
signedWithParent, authorPublicKey);
will(throwException(new FormatException()));
}});
@@ -203,8 +203,8 @@ public class ForumPostValidatorTest extends ValidatorTestCase {
throws Exception {
expectCreateAuthor();
context.checking(new Expectations() {{
oneOf(clientHelper).verifySignature(SIGNING_LABEL_POST, signature,
authorPublicKey, signedWithParent);
oneOf(clientHelper).verifySignature(signature, SIGNING_LABEL_POST,
signedWithParent, authorPublicKey);
will(throwException(new GeneralSecurityException()));
}});

View File

@@ -262,8 +262,8 @@ public class IntroduceeManagerTest extends BriarTestCase {
);
context.checking(new Expectations() {{
oneOf(cryptoComponent).verify(SIGNING_LABEL, nonce,
introducee2.getAuthor().getPublicKey(), sig);
oneOf(cryptoComponent).verifySignature(sig, SIGNING_LABEL, nonce,
introducee2.getAuthor().getPublicKey());
will(returnValue(false));
}});
@@ -292,8 +292,8 @@ public class IntroduceeManagerTest extends BriarTestCase {
state.put(SIGNATURE, sig);
context.checking(new Expectations() {{
oneOf(cryptoComponent).verify(SIGNING_LABEL, nonce,
publicKeyBytes, sig);
oneOf(cryptoComponent).verifySignature(sig, SIGNING_LABEL, nonce,
publicKeyBytes);
will(returnValue(true));
}});
introduceeManager.verifySignature(state);

View File

@@ -401,8 +401,8 @@ public class GroupMessageValidatorTest extends ValidatorTestCase {
expectParseAuthor(creatorList, creator);
expectParsePrivateGroup();
context.checking(new Expectations() {{
oneOf(clientHelper).verifySignature(SIGNING_LABEL_JOIN,
memberSignature, creator.getPublicKey(), signed);
oneOf(clientHelper).verifySignature(memberSignature,
SIGNING_LABEL_JOIN, signed, creator.getPublicKey());
if (!memberSigValid)
will(throwException(new GeneralSecurityException()));
}});
@@ -422,13 +422,13 @@ public class GroupMessageValidatorTest extends ValidatorTestCase {
oneOf(groupInvitationFactory).createInviteToken(creator.getId(),
member.getId(), privateGroup.getId(), inviteTimestamp);
will(returnValue(token));
oneOf(clientHelper).verifySignature(SIGNING_LABEL_INVITE,
creatorSignature, creator.getPublicKey(), token);
oneOf(clientHelper).verifySignature(creatorSignature,
SIGNING_LABEL_INVITE, token, creator.getPublicKey());
if (!creatorSigValid) {
will(throwException(new GeneralSecurityException()));
} else {
oneOf(clientHelper).verifySignature(SIGNING_LABEL_JOIN,
memberSignature, member.getPublicKey(), signed);
oneOf(clientHelper).verifySignature(memberSignature,
SIGNING_LABEL_JOIN, signed, member.getPublicKey());
if (!memberSigValid)
will(throwException(new GeneralSecurityException()));
}
@@ -648,8 +648,8 @@ public class GroupMessageValidatorTest extends ValidatorTestCase {
);
expectParseAuthor(memberList, member);
context.checking(new Expectations() {{
oneOf(clientHelper).verifySignature(SIGNING_LABEL_POST,
memberSignature, member.getPublicKey(), signed);
oneOf(clientHelper).verifySignature(memberSignature,
SIGNING_LABEL_POST, signed, member.getPublicKey());
if (!sigValid)
will(throwException(new GeneralSecurityException()));
}});

View File

@@ -256,8 +256,8 @@ public class GroupInvitationValidatorTest extends ValidatorTestCase {
oneOf(privateGroupFactory).createPrivateGroup(groupName, creator,
salt);
will(returnValue(privateGroup));
oneOf(clientHelper).verifySignature(SIGNING_LABEL_INVITE, signature,
creator.getPublicKey(), signed);
oneOf(clientHelper).verifySignature(signature, SIGNING_LABEL_INVITE,
signed, creator.getPublicKey());
if (exception) {
will(throwException(new GeneralSecurityException()));
} else {