Skip to content

fix(sync): accept only signatures whose key and R have order L - #1846

Open
A-EDev wants to merge 1 commit into
mainfrom
fix/sync2-ed25519-prime-order
Open

A-EDev wants to merge 1 commit into
mainfrom
fix/sync2-ed25519-prime-order

Conversation

@A-EDev

@A-EDev A-EDev commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follows flow-sync #101, spec/crypto.md, Ed25519 checks: the key A and the signature's R must each be a point of order exactly L. BouncyCastle's Ed25519.validatePublicKeyFull checks exactly that, so Curves.kt now uses it on both and drops its own small-order list and canonical check, which the method covers. isValidSigningKey, which guards keys entering the roster, uses the same check. An honest signer never produces what this refuses, so nothing changes for existing groups.

verify in Curves.kt is the only place that verifies an Ed25519 signature, so all its callers are covered: roster entries (Roster.kt), log objects (Session.verified, ObjectCheck.admits), live messages (LiveEnvelopes) and the LAN hello (LanLink, both sides). Snapshot blobs and the walk down from a cut are checked by hash, not by signature, as the spec has it.

Vectors: ed25519.json and roster.json come byte for byte from flow-sync #101 (01050b2), with their two README rows. VectorsTest now also refuses mixedOrderKeys. The README's other rows are left as they were.

Related issue

None. Follows flow-sync #101, which should land first.

Change type

  • Bug fix
  • Feature
  • Refactor or maintenance
  • Build, packaging, or CI
  • Documentation

Validation

All Gradle commands ran with --offline -Dorg.gradle.jvmargs="-Xmx4096m -XX:MaxMetaspaceSize=1024m -Dfile.encoding=UTF-8 -Djava.net.preferIPv6Addresses=true".

  • :app:testGithubDebugUnitTest --tests 'io.github.aedev.flow.sync2.*': 301 tests, 0 failed.

  • ktlintCheck: exit 0.

  • javap on bcprov-jdk18on-1.86.jar, the version in gradle/libs.versions.toml, shows validatePublicKeyFull(byte[], int). No new dependency.

  • Before the change, VectorsTest.ed25519_strict_verification (identity-r) and RosterTest.invalid_entries_are_rejected_for_the_named_reason (mixed-order-key) failed on the new vectors.

  • Each of these made tests fail when tried: R not checked (2 tests), A only partly checked (2), A not checked at verify (1).

  • ./gradlew :app:assembleGithubDebug: not run locally, CI builds it.

  • ./gradlew :app:testGithubDebugUnitTest: full suite not run locally, CI runs it.

  • I ran any additional flavor-specific build or test tasks affected by this change.

  • I manually tested the affected behavior on an Android device or emulator: not done.

Test device and Android version: none.

Risk and compatibility

Cost, measured on a Mac on the JVM, 1 KiB message, 3 runs of 3,000 each. Not measured on a phone, so these say nothing about a phone's absolute numbers:

Per call
Ed25519.verify alone 44 to 52 µs
validatePublicKeyFull on A 37 to 40 µs
validatePublicKeyFull on R 35 to 37 µs
Curves.verify now 108 to 124 µs

So one verify costs a little over twice what it did. An object is verified twice, once when it's kept and once when it's read. Downloading and reading a log of 2,000 objects of 866 bytes took 0.9 to 1.2 s before and 1.1 to 1.3 s after on the same Mac, in 3 runs each, which is noisy. Larger objects spend a smaller share on this. No number here is a phone's.

No nightly note: nothing an honest group sees changes. roster.json and the vector README also change in the other open PRs that follow flow-sync #103 and #1831.

  • The change does not introduce secrets, private data, or unexpected telemetry.
  • New user-facing text uses Android string resources. (None added.)
  • Dependency and lockfile changes are intentional and limited to this PR.
  • Room schema changes include the required version bump and migration, or this PR does not change the Room schema.
  • Breaking changes and upgrade steps are clearly documented. (None.)

Copilot AI balanced review requested due to automatic review settings November 7, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Nov 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants