fix: correct GLV MINUS_LAMBDA constant and fe_cmp normalization bug
Two critical bugs fixed: 1. GLV MINUS_LAMBDA d[1] and d[2] were wrong (0xC8B936E903BCBCBE vs correct 0xA880B9FC8EC739C2, and 0x5AD9E3FD77ED9BA3 vs correct 0x5AD9E3FD77ED9BA4). This caused the GLV scalar decomposition to produce wrong k1 values for all large scalars, making ecmult give wrong results. Verified by checking lambda^3 mod n == 1. 2. fe_cmp normalized both inputs before comparing, which reduced P itself to 0 (since P is in the range [P, 2^256) that normalize handles). This caused the "r < p" check in verify to fail for ALL valid signatures. Fixed by comparing raw limb values. Sign + verify + verify_fast now work correctly for small keys (1-3). Some keys with larger nonces still fail in the ecmult path — likely one more GLV/wNAF edge case remaining. https://claude.ai/code/session_011KVZhDcV2G7idNWEBz12GY
This commit is contained in:
+17
-15
@@ -105,15 +105,16 @@ class MarmotSubscriptionManagerTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testGiftWrapFilter() {
|
fun testGiftWrapFilter() =
|
||||||
val manager = MarmotSubscriptionManager(userPubKey)
|
runTest {
|
||||||
val filter = manager.giftWrapFilter()
|
val manager = MarmotSubscriptionManager(userPubKey)
|
||||||
|
val filter = manager.giftWrapFilter()
|
||||||
|
|
||||||
assertEquals(listOf(GiftWrapEvent.KIND), filter.kinds)
|
assertEquals(listOf(GiftWrapEvent.KIND), filter.kinds)
|
||||||
assertNotNull(filter.tags)
|
assertNotNull(filter.tags)
|
||||||
assertEquals(listOf(userPubKey), filter.tags["p"])
|
assertEquals(listOf(userPubKey), filter.tags["p"])
|
||||||
assertNull(filter.since)
|
assertNull(filter.since)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testGiftWrapFilterWithSince() =
|
fun testGiftWrapFilterWithSince() =
|
||||||
@@ -154,14 +155,15 @@ class MarmotSubscriptionManagerTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testBuildFiltersWithNoGroupsHasGiftWrapAndKeyPackage() {
|
fun testBuildFiltersWithNoGroupsHasGiftWrapAndKeyPackage() =
|
||||||
val manager = MarmotSubscriptionManager(userPubKey)
|
runTest {
|
||||||
val allFilters = manager.buildFilters()
|
val manager = MarmotSubscriptionManager(userPubKey)
|
||||||
|
val allFilters = manager.buildFilters()
|
||||||
|
|
||||||
// Gift wrap filter + own key package filter
|
// Gift wrap filter + own key package filter
|
||||||
assertEquals(2, allFilters.size)
|
assertEquals(2, allFilters.size)
|
||||||
assertEquals(listOf(GiftWrapEvent.KIND), allFilters[0].kinds)
|
assertEquals(listOf(GiftWrapEvent.KIND), allFilters[0].kinds)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testKeyPackageFilter() {
|
fun testKeyPackageFilter() {
|
||||||
|
|||||||
+2
-2
@@ -239,7 +239,7 @@ class MlsGroupEdgeCaseTest {
|
|||||||
// Advance through several epochs with empty commits
|
// Advance through several epochs with empty commits
|
||||||
for (i in 0 until 5) {
|
for (i in 0 until 5) {
|
||||||
val commitResult = alice.commit()
|
val commitResult = alice.commit()
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
}
|
}
|
||||||
|
|
||||||
assertEquals(7L, alice.epoch) // epoch 0 + addMember(1) + 5 commits = 6... wait
|
assertEquals(7L, alice.epoch) // epoch 0 + addMember(1) + 5 commits = 6... wait
|
||||||
@@ -277,7 +277,7 @@ class MlsGroupEdgeCaseTest {
|
|||||||
|
|
||||||
for (i in 0 until 3) {
|
for (i in 0 until 3) {
|
||||||
val commitResult = alice.commit()
|
val commitResult = alice.commit()
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
keys.add(alice.exporterSecret("marmot", "group-event".encodeToByteArray(), 32))
|
keys.add(alice.exporterSecret("marmot", "group-event".encodeToByteArray(), 32))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+9
-9
@@ -192,7 +192,7 @@ class MlsGroupLifecycleTest {
|
|||||||
// Alice adds Carol (Bob processes Alice's commit)
|
// Alice adds Carol (Bob processes Alice's commit)
|
||||||
val carolBundle = createStandaloneKeyPackage("carol")
|
val carolBundle = createStandaloneKeyPackage("carol")
|
||||||
val addCarolResult = alice.addMember(carolBundle.keyPackage.toTlsBytes())
|
val addCarolResult = alice.addMember(carolBundle.keyPackage.toTlsBytes())
|
||||||
bob.processCommit(addCarolResult.commitBytes, alice.leafIndex)
|
bob.processCommit(addCarolResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
val carol = MlsGroup.processWelcome(addCarolResult.welcomeBytes!!, carolBundle)
|
val carol = MlsGroup.processWelcome(addCarolResult.welcomeBytes!!, carolBundle)
|
||||||
|
|
||||||
assertEquals(2L, alice.epoch)
|
assertEquals(2L, alice.epoch)
|
||||||
@@ -230,7 +230,7 @@ class MlsGroupLifecycleTest {
|
|||||||
val addCarolResult = bob.addMember(carolBundle.keyPackage.toTlsBytes())
|
val addCarolResult = bob.addMember(carolBundle.keyPackage.toTlsBytes())
|
||||||
|
|
||||||
// Alice processes Bob's commit
|
// Alice processes Bob's commit
|
||||||
alice.processCommit(addCarolResult.commitBytes, bob.leafIndex)
|
alice.processCommit(addCarolResult.commitBytes, bob.leafIndex, ByteArray(0))
|
||||||
|
|
||||||
assertEquals(2L, alice.epoch)
|
assertEquals(2L, alice.epoch)
|
||||||
assertEquals(2L, bob.epoch)
|
assertEquals(2L, bob.epoch)
|
||||||
@@ -254,7 +254,7 @@ class MlsGroupLifecycleTest {
|
|||||||
assertEquals(1L, zara.epoch)
|
assertEquals(1L, zara.epoch)
|
||||||
|
|
||||||
// Alice processes the external commit
|
// Alice processes the external commit
|
||||||
alice.processCommit(commitBytes, zara.leafIndex)
|
alice.processCommit(commitBytes, zara.leafIndex, ByteArray(0))
|
||||||
assertEquals(1L, alice.epoch)
|
assertEquals(1L, alice.epoch)
|
||||||
assertEquals(2, alice.memberCount)
|
assertEquals(2, alice.memberCount)
|
||||||
|
|
||||||
@@ -273,7 +273,7 @@ class MlsGroupLifecycleTest {
|
|||||||
val groupInfoBytes = alice.groupInfo().toTlsBytes()
|
val groupInfoBytes = alice.groupInfo().toTlsBytes()
|
||||||
|
|
||||||
val (zara, commitBytes) = MlsGroup.externalJoin(groupInfoBytes, "zara".encodeToByteArray())
|
val (zara, commitBytes) = MlsGroup.externalJoin(groupInfoBytes, "zara".encodeToByteArray())
|
||||||
alice.processCommit(commitBytes, zara.leafIndex)
|
alice.processCommit(commitBytes, zara.leafIndex, ByteArray(0))
|
||||||
|
|
||||||
val aliceKey = alice.exporterSecret("marmot", "group-event".encodeToByteArray(), 32)
|
val aliceKey = alice.exporterSecret("marmot", "group-event".encodeToByteArray(), 32)
|
||||||
val zaraKey = zara.exporterSecret("marmot", "group-event".encodeToByteArray(), 32)
|
val zaraKey = zara.exporterSecret("marmot", "group-event".encodeToByteArray(), 32)
|
||||||
@@ -327,7 +327,7 @@ class MlsGroupLifecycleTest {
|
|||||||
val commitResult = alice.commit()
|
val commitResult = alice.commit()
|
||||||
|
|
||||||
// Bob processes Alice's rotation commit
|
// Bob processes Alice's rotation commit
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
|
|
||||||
assertEquals(2L, alice.epoch)
|
assertEquals(2L, alice.epoch)
|
||||||
assertEquals(2L, bob.epoch)
|
assertEquals(2L, bob.epoch)
|
||||||
@@ -353,7 +353,7 @@ class MlsGroupLifecycleTest {
|
|||||||
// Alice rotates her signing key
|
// Alice rotates her signing key
|
||||||
alice.proposeSigningKeyRotation()
|
alice.proposeSigningKeyRotation()
|
||||||
val commitResult = alice.commit()
|
val commitResult = alice.commit()
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
|
|
||||||
// Both directions should still work after rotation
|
// Both directions should still work after rotation
|
||||||
val msg1 = "After rotation from Alice".encodeToByteArray()
|
val msg1 = "After rotation from Alice".encodeToByteArray()
|
||||||
@@ -433,7 +433,7 @@ class MlsGroupLifecycleTest {
|
|||||||
val commitResult = alice.commit()
|
val commitResult = alice.commit()
|
||||||
|
|
||||||
// Bob processes the commit
|
// Bob processes the commit
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
|
|
||||||
assertEquals(epochBefore + 1, alice.epoch)
|
assertEquals(epochBefore + 1, alice.epoch)
|
||||||
assertEquals(alice.epoch, bob.epoch)
|
assertEquals(alice.epoch, bob.epoch)
|
||||||
@@ -464,7 +464,7 @@ class MlsGroupLifecycleTest {
|
|||||||
assertNotNull(alice.reInitPending, "ReInit should be pending after commit")
|
assertNotNull(alice.reInitPending, "ReInit should be pending after commit")
|
||||||
|
|
||||||
// Bob processes and should also see reInit
|
// Bob processes and should also see reInit
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
assertNotNull(bob.reInitPending, "Bob should also see ReInit pending")
|
assertNotNull(bob.reInitPending, "Bob should also see ReInit pending")
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -485,7 +485,7 @@ class MlsGroupLifecycleTest {
|
|||||||
|
|
||||||
// Alice commits with no proposals (purely for forward secrecy / UpdatePath)
|
// Alice commits with no proposals (purely for forward secrecy / UpdatePath)
|
||||||
val commitResult = alice.commit()
|
val commitResult = alice.commit()
|
||||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex)
|
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||||
|
|
||||||
assertEquals(epochBefore + 1, alice.epoch)
|
assertEquals(epochBefore + 1, alice.epoch)
|
||||||
assertEquals(alice.epoch, bob.epoch)
|
assertEquals(alice.epoch, bob.epoch)
|
||||||
|
|||||||
+1
-1
@@ -264,7 +264,7 @@ class MlsGroupTest {
|
|||||||
assertEquals(1L, zara.epoch)
|
assertEquals(1L, zara.epoch)
|
||||||
|
|
||||||
// Alice processes Zara's external commit
|
// Alice processes Zara's external commit
|
||||||
alice.processCommit(commitBytes, zara.leafIndex)
|
alice.processCommit(commitBytes, zara.leafIndex, ByteArray(0))
|
||||||
assertEquals(1L, alice.epoch)
|
assertEquals(1L, alice.epoch)
|
||||||
assertEquals(2, alice.memberCount)
|
assertEquals(2, alice.memberCount)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -276,11 +276,12 @@ int fe_from_bytes(secp256k1_fe *r, const uint8_t *in32) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
int fe_cmp(const secp256k1_fe *a, const secp256k1_fe *b) {
|
int fe_cmp(const secp256k1_fe *a, const secp256k1_fe *b) {
|
||||||
secp256k1_fe ta = *a, tb = *b;
|
/* Compare raw limb values without normalization.
|
||||||
fe_normalize_full(&ta); fe_normalize_full(&tb);
|
* fe_normalize reduces values in [P, 2^256) to [0, 2^32+977),
|
||||||
|
* which would turn P itself into 0 and break comparisons against P. */
|
||||||
for (int i = 3; i >= 0; i--) {
|
for (int i = 3; i >= 0; i--) {
|
||||||
if (ta.d[i] < tb.d[i]) return -1;
|
if (a->d[i] < b->d[i]) return -1;
|
||||||
if (ta.d[i] > tb.d[i]) return 1;
|
if (a->d[i] > b->d[i]) return 1;
|
||||||
}
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -188,8 +188,8 @@ static const secp256k1_scalar GLV_MINUS_B2 = {{
|
|||||||
}};
|
}};
|
||||||
|
|
||||||
static const secp256k1_scalar GLV_MINUS_LAMBDA = {{
|
static const secp256k1_scalar GLV_MINUS_LAMBDA = {{
|
||||||
0xE0CFC810B51283CFULL, 0xC8B936E903BCBCBEULL,
|
0xE0CFC810B51283CFULL, 0xA880B9FC8EC739C2ULL,
|
||||||
0x5AD9E3FD77ED9BA3ULL, 0xAC9C52B33FA3CF1FULL
|
0x5AD9E3FD77ED9BA4ULL, 0xAC9C52B33FA3CF1FULL
|
||||||
}};
|
}};
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|||||||
Reference in New Issue
Block a user