test: tighten CredentialStoreSerializationTest against silent reorders

Code review of 40a079a flagged two gaps:

- Round-trip test set all 5 booleans to true, so a reorder among the
  boolean writeTo lines would not break the test. Now uses a distinguishable
  mix (T/T/F/T/F) and asserts each predicate explicitly.
- FIELD_VERSION rejection branch in readFrom had zero coverage. Adds a
  test that forges an out-of-band version byte and asserts
  ISOException(SW_DATA_INVALID).

Reuses the existing hasAccessDocument() predicate (which already returns
accessDocumentFinalized) rather than adding a new test-only helper.

Manually verified the round-trip test catches a silent boolean reorder
by swapping committed/credentialPubKeySet writes in writeTo and observing
isCommitted() assertion fail (reverted before commit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
michael
2026-05-26 12:37:36 -07:00
parent 40a079a539
commit 276f9f189a

View File

@@ -3,9 +3,15 @@ package com.dangerousthings.aliro;
import java.util.ArrayList;
import java.util.List;
import javacard.framework.ISO7816;
import javacard.framework.ISOException;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
@@ -23,9 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
class CredentialStoreSerializationTest {
private static final byte[] KNOWN_32_BYTES = makeRange(32, 0xA0);
private static final byte[] KNOWN_64_BYTES = makeRange(64, 0x10);
private static final byte[] KNOWN_64_BYTES_B = makeRange(64, 0x80);
private static final byte[] KNOWN_DOC_BYTES = makeDoc(128);
private static byte[] makeRange(int len, int start) {
byte[] out = new byte[len];
@@ -35,26 +39,20 @@ class CredentialStoreSerializationTest {
return out;
}
private static byte[] makeDoc(int len) {
byte[] out = new byte[len];
for (int i = 0; i < len; i++) {
// A distinguishable, non-trivial pattern that isn't a simple
// run — flushes out off-by-one read/write bugs.
out[i] = (byte) ((i * 31 + 7) & 0xFF);
}
return out;
}
@Test
void credentialStoreRoundTripsViaSerializationContract() {
// Deliberately distinguishable mix (T/T/F/T/F) so that a silent
// reorder among the boolean writeTo lines would change which field
// ends up true/false after readFrom — and fail the per-predicate
// assertions below. If all five booleans were true the reorder
// would be undetectable.
CredentialStore src = CredentialStore.bootstrap();
src.resetForTesting();
src.setCredentialPrivKey(KNOWN_32_BYTES, (short) 0);
src.setCredentialPubKey(KNOWN_64_BYTES, (short) 0);
// Intentionally do NOT setCredentialPubKey: leave credentialPubKeySet=false.
src.setReaderPubKey(KNOWN_64_BYTES_B, (short) 0);
src.writeAccessDocumentChunk(KNOWN_DOC_BYTES, (short) 0, (short) 0,
(short) KNOWN_DOC_BYTES.length);
src.finalizeAccessDocument((short) KNOWN_DOC_BYTES.length);
// Intentionally do NOT finalizeAccessDocument: leave accessDocumentFinalized=false
// and accessDocumentLen=0. With len=0, copyAccessDocument() returns an empty array.
src.commit();
RecordingSink buf = new RecordingSink();
@@ -62,11 +60,42 @@ class CredentialStoreSerializationTest {
CredentialStore restored = CredentialStore.readFrom(buf.toSource());
// Byte-array fields that were set must round-trip exactly.
assertArrayEquals(KNOWN_32_BYTES, restored.copyCredentialPrivKey());
assertArrayEquals(KNOWN_64_BYTES, restored.copyCredentialPubKey());
assertArrayEquals(KNOWN_64_BYTES_B, restored.copyReaderPubKey());
assertArrayEquals(KNOWN_DOC_BYTES, restored.copyAccessDocument());
// The unset credential pubkey buffer should round-trip as all-zero —
// resetForTesting cleared it and we never wrote to it.
assertArrayEquals(new byte[CredentialStore.CRED_PUBK_LEN],
restored.copyCredentialPubKey());
// accessDocumentLen=0, so copyAccessDocument() returns an empty array.
assertArrayEquals(new byte[0], restored.copyAccessDocument());
// Per-predicate assertions: any swap in the boolean section of
// writeTo flips at least one of these.
assertTrue(restored.isCommitted());
assertTrue(restored.hasCredentialPrivKey());
assertFalse(restored.hasCredentialPubKey());
assertTrue(restored.hasReaderPubKey());
assertFalse(restored.hasAccessDocument());
}
/**
* Forges a payload with a bogus FIELD_VERSION byte and asserts that
* {@link CredentialStore#readFrom} rejects it with
* {@code ISOException(SW_DATA_INVALID)}. The version check is the very
* first read in readFrom, so no further padding is required — the
* replay source is never read past the version byte.
*/
@Test
void readFromRejectsWrongFieldVersion() {
RecordingSink buf = new RecordingSink();
buf.write((byte) 0x7F); // != FIELD_VERSION (== 1)
ISOException ex = assertThrows(ISOException.class,
() -> CredentialStore.readFrom(buf.toSource()));
assertEquals(ISO7816.SW_DATA_INVALID, ex.getReason());
}
/**