[REFACTOR] Move signature validation behind isValidForRemoveOperation()

Move the signature checks into the objects to clean up the calling code
and make it more testable.
This commit is contained in:
Julian Knutsen 2019-11-07 16:56:32 -08:00
parent 9c7dc0c1ad
commit f915a03ff9
No known key found for this signature in database
GPG key ID: D85F536DB3615B2D
4 changed files with 30 additions and 2 deletions

View file

@ -505,7 +505,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
return false;
// Verify the ProtectedStorageEntry is well formed and valid for the remove operation
if (!protectedStorageEntry.isValidForRemoveOperation() || !checkSignature(protectedStorageEntry))
if (!protectedStorageEntry.isValidForRemoveOperation())
return false;
// If we have already seen an Entry with the same hash, verify the new Entry has the same owner
@ -594,7 +594,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
PublicKey receiversPubKey = protectedMailboxStorageEntry.getReceiversPubKey();
if (!protectedMailboxStorageEntry.isValidForRemoveOperation() || !checkSignature(protectedMailboxStorageEntry))
if (!protectedMailboxStorageEntry.isValidForRemoveOperation())
return false;
// Verify the Entry has the correct receiversPubKey for removal.

View file

@ -117,6 +117,9 @@ public class ProtectedMailboxStorageEntry extends ProtectedStorageEntry {
*/
@Override
public boolean isValidForRemoveOperation() {
if (!this.isSignatureValid())
return false;
MailboxStoragePayload mailboxStoragePayload = this.getMailboxStoragePayload();
boolean result = mailboxStoragePayload.getOwnerPubKey() != null &&
mailboxStoragePayload.getOwnerPubKey().equals(this.getOwnerPubKey());

View file

@ -146,4 +146,18 @@ public class ProtectedMailboxStorageEntryTest {
Assert.assertFalse(protectedStorageEntry.isValidForRemoveOperation());
}
// TESTCASE: isValidForRemoveOperation() should fail if the signature is bad
@Test
public void isValidForRemoveOperation_BadSignature() throws NoSuchAlgorithmException, CryptoException {
KeyPair senderKeys = TestUtils.generateKeyPair();
KeyPair receiverKeys = TestUtils.generateKeyPair();
MailboxStoragePayload mailboxStoragePayload = buildMailboxStoragePayload(senderKeys.getPublic(), receiverKeys.getPublic());
ProtectedStorageEntry protectedStorageEntry = buildProtectedMailboxStorageEntry(mailboxStoragePayload, receiverKeys, receiverKeys.getPublic());
protectedStorageEntry.updateSignature(new byte[] { 0 });
Assert.assertFalse(protectedStorageEntry.isValidForRemoveOperation());
}
}

View file

@ -180,4 +180,15 @@ public class ProtectedStorageEntryTest {
Assert.assertFalse(protectedStorageEntry.isValidForRemoveOperation());
}
// TESTCASE: isValidForRemoveOperation() should fail if the signature is bad
@Test
public void isValidForRemoveOperation_BadSignature() throws NoSuchAlgorithmException, CryptoException {
KeyPair ownerKeys = TestUtils.generateKeyPair();
ProtectedStorageEntry protectedStorageEntry = buildProtectedStorageEntry(ownerKeys, ownerKeys);
protectedStorageEntry.updateSignature(new byte[] { 0 });
Assert.assertFalse(protectedStorageEntry.isValidForRemoveOperation());
}
}