Grandiras/ATProto.NET

Fork 0

DID document key extraction accepts only `Multikey`, not the legacy verification-method types #98

New issue

Closed

opened 2026-08-20 21:35:51 +00:00 by claude-bot · 2 comments

claude-bot commented

2026-08-20 21:35:51 +00:00

Collaborator

Both places the SDK pulls a signing key out of a DID document require type == "Multikey":

src/ATProtoNet/Spaces/SpaceAuthority.cs:121 (FindMultikey, behind GetSigningKey)
src/ATProtoNet/Streaming/FirehoseVerifier.cs:345

The reference implementation accepts three types, not one — packages/identity/src/did/atproto-data.ts:

if (key.type === 'EcdsaSecp256r1VerificationKey2019') {
  didKey = crypto.formatDidKey(crypto.P256_JWT_ALG, keyBytes)
} else if (key.type === 'EcdsaSecp256k1VerificationKey2019') {
  didKey = crypto.formatDidKey(crypto.SECP256K1_JWT_ALG, keyBytes)
} else if (key.type === 'Multikey') {
  const parsed = crypto.parseMultikey(key.publicKeyMultibase)
  didKey = crypto.formatDidKey(parsed.jwtAlg, parsed.keyBytes)
}

The two encodings differ, so this is not just a string comparison: a Multikey's publicKeyMultibase is base58btc over multicodec-tagged compressed key bytes (0xe7 0x01 || 33 bytes for secp256k1), while the legacy EcdsaSecp256k1VerificationKey2019 form is base58btc over a bare uncompressed point (0x04 || X || Y, 65 bytes) with no multicodec prefix. Converting means compressing by the parity of Y before AtProtoCrypto.ImportCompressedPublicKey.

Effect. Against a document that publishes the legacy form, SpaceAuthority.GetSigningKey returns null, so SpaceSyncer.ResolveSigningKeyAsync throws SpaceRepoVerificationException("'{did}' publishes no AT Protocol signing key") and no permissioned commit can be verified at all; the firehose verifier likewise treats every commit from that account as unverifiable.

How it surfaced. While adding the space integration tests (#93). plc.directory serves Multikey today, so production did:plc is unaffected — but the reference dev-env network pins an older @did-plc/lib that serves the legacy form, and hand-written did:web documents may too. SpaceNetworkFixture.ResolveSigningKeyAsync in tests/ATProtoNet.IntegrationTests/ works around it with its own resolver so the test network's key format is not what the commit-verification tests end up asserting; that workaround should go away once the SDK handles both.

Worth deciding as part of a fix: where the conversion belongs (a shared helper on DidDocument or in AtProtoCrypto, rather than duplicated in SpaceAuthority and FirehoseVerifier), and whether EcdsaSecp256r1VerificationKey2019 is worth accepting alongside the k256 one.

Filed by Claude while working on #93 (run).

Both places the SDK pulls a signing key out of a DID document require `type == "Multikey"`: - `src/ATProtoNet/Spaces/SpaceAuthority.cs:121` (`FindMultikey`, behind `GetSigningKey`) - `src/ATProtoNet/Streaming/FirehoseVerifier.cs:345` The reference implementation accepts three types, not one — `packages/identity/src/did/atproto-data.ts`: ```ts if (key.type === 'EcdsaSecp256r1VerificationKey2019') { didKey = crypto.formatDidKey(crypto.P256_JWT_ALG, keyBytes) } else if (key.type === 'EcdsaSecp256k1VerificationKey2019') { didKey = crypto.formatDidKey(crypto.SECP256K1_JWT_ALG, keyBytes) } else if (key.type === 'Multikey') { const parsed = crypto.parseMultikey(key.publicKeyMultibase) didKey = crypto.formatDidKey(parsed.jwtAlg, parsed.keyBytes) } ``` The two encodings differ, so this is not just a string comparison: a `Multikey`'s `publicKeyMultibase` is base58btc over multicodec-tagged compressed key bytes (`0xe7 0x01 || 33 bytes` for secp256k1), while the legacy `EcdsaSecp256k1VerificationKey2019` form is base58btc over a bare **uncompressed** point (`0x04 || X || Y`, 65 bytes) with no multicodec prefix. Converting means compressing by the parity of Y before `AtProtoCrypto.ImportCompressedPublicKey`. **Effect.** Against a document that publishes the legacy form, `SpaceAuthority.GetSigningKey` returns `null`, so `SpaceSyncer.ResolveSigningKeyAsync` throws `SpaceRepoVerificationException("'{did}' publishes no AT Protocol signing key")` and no permissioned commit can be verified at all; the firehose verifier likewise treats every commit from that account as unverifiable. **How it surfaced.** While adding the space integration tests (#93). plc.directory serves `Multikey` today, so production `did:plc` is unaffected — but the reference `dev-env` network pins an older `@did-plc/lib` that serves the legacy form, and hand-written `did:web` documents may too. `SpaceNetworkFixture.ResolveSigningKeyAsync` in `tests/ATProtoNet.IntegrationTests/` works around it with its own resolver so the test network's key format is not what the commit-verification tests end up asserting; that workaround should go away once the SDK handles both. Worth deciding as part of a fix: where the conversion belongs (a shared helper on `DidDocument` or in `AtProtoCrypto`, rather than duplicated in `SpaceAuthority` and `FirehoseVerifier`), and whether `EcdsaSecp256r1VerificationKey2019` is worth accepting alongside the k256 one. --- _Filed by Claude while working on #93 ([run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/319))._

claude-bot referenced this issue

2026-08-20 21:35:51 +00:00

Spaces: integration tests against a permissioned-data PDS #93

claude-bot was assigned by Grandiras

2026-08-20 21:45:12 +00:00

claude-bot commented

2026-08-20 21:45:18 +00:00

Author

Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/330); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-98/run-330.md) — updating every ~5 minutes while the task runs.

claude-bot referenced this issue from a commit

2026-08-20 21:52:42 +00:00

fix: read DID document signing keys from the legacy verification-method types

claude-bot referenced this issue from a pull request that will close it,

2026-08-20 21:53:00 +00:00

Claude: changes for issue #98 #103

claude-bot commented

2026-08-20 21:53:01 +00:00

Author

Collaborator

Pushed claude/issue-98.

What changed

Both key-extraction sites required type == "Multikey"; both now read all three verification-method types the reference implementation accepts, and the conversion lives in one place rather than duplicated.

New shared helpers (src/ATProtoNet/Identity/PlcClient.cs):

VerificationMethod.ToDidKey() — Multikey passes through (its publicKeyMultibase is the did:key encoding); EcdsaSecp256k1VerificationKey2019 / EcdsaSecp256r1VerificationKey2019 are base58btc-decoded, compressed by the parity of Y, and re-tagged with the multicodec prefix for the curve the type names. Returns null for an absent/unrecognized type, throws FormatException when a recognized entry's material is malformed.
DidDocument.GetSigningKey() and DidDocument.GetVerificationKey(fragment) — match both the bare #atproto and the DID-qualified id.

New crypto primitives (AtProtoCrypto): FormatDidKey(publicKey, curve) and CompressPublicKey(publicKey) (accepts 33-byte compressed or 65-byte uncompressed; both curves have 32-byte coordinates, so compression is curve-independent), plus an internal MultibaseToBytes.

Call sites: SpaceAuthority.GetSigningKey (FindMultikey deleted — #atproto_space then falling back to #atproto, both type-agnostic now) and FirehoseVerifier (its private GetSigningKey deleted).

I did accept EcdsaSecp256r1VerificationKey2019 alongside the k256 one — it's the same code path with a different multicodec prefix, and the reference implementation accepts it.

Workaround removed: SpaceNetworkFixture.ResolveSigningKeyAsync is now just SpaceSyncer.ResolveSigningKeyAsync(DidResolver), so the space commit-verification integration tests assert against the production path. Its private base58 decoder is gone. docs/testing-spaces.md and docs/did-resolution.md updated; CHANGELOG entry under Fixed.

Verification

dotnet build on the solution: 0 warnings, 0 errors. dotnet test tests/ATProtoNet.Tests/ --configuration Release: 1549 passed, 0 failed, 5 skipped (the pre-existing Aspire-manifest gates).

14 new unit tests — tests/ATProtoNet.Tests/Identity/DidDocumentKeyTests.cs and a new section in AtProtoCryptoTests. The load-bearing ones build a legacy verification method from a real key pair's uncompressed point and assert it resolves to the same did:key the key reports for itself, and that a signature made by the private half verifies against the resolved key — looped until both parities of Y have been exercised, since a compression that ignored the parity bit would still produce a well-formed did:key for the wrong point half the time. The k256 case is guarded with the repo's existing PlatformNotSupportedException pattern (it ran here on Linux).

The integration tests themselves still need a dev-env space network (ATPROTO_TEST_SPACES=true), which this container has no way to stand up, so the fixture change is compile-verified only — tests/ATProtoNet.IntegrationTests/ builds clean.

⏱ 7m 32s · 41 turns · tokens: 84.8k in / 29.8k out (+2533.7k cache-read)
📜 Full session transcript
→ Opened PR: #103

Pushed `claude/issue-98`. ## What changed Both key-extraction sites required `type == "Multikey"`; both now read all three verification-method types the reference implementation accepts, and the conversion lives in one place rather than duplicated. **New shared helpers** (`src/ATProtoNet/Identity/PlcClient.cs`): - `VerificationMethod.ToDidKey()` — `Multikey` passes through (its `publicKeyMultibase` *is* the did:key encoding); `EcdsaSecp256k1VerificationKey2019` / `EcdsaSecp256r1VerificationKey2019` are base58btc-decoded, compressed by the parity of Y, and re-tagged with the multicodec prefix for the curve the type names. Returns `null` for an absent/unrecognized type, throws `FormatException` when a recognized entry's material is malformed. - `DidDocument.GetSigningKey()` and `DidDocument.GetVerificationKey(fragment)` — match both the bare `#atproto` and the DID-qualified id. **New crypto primitives** (`AtProtoCrypto`): `FormatDidKey(publicKey, curve)` and `CompressPublicKey(publicKey)` (accepts 33-byte compressed or 65-byte uncompressed; both curves have 32-byte coordinates, so compression is curve-independent), plus an internal `MultibaseToBytes`. **Call sites**: `SpaceAuthority.GetSigningKey` (`FindMultikey` deleted — `#atproto_space` then falling back to `#atproto`, both type-agnostic now) and `FirehoseVerifier` (its private `GetSigningKey` deleted). I did accept `EcdsaSecp256r1VerificationKey2019` alongside the k256 one — it's the same code path with a different multicodec prefix, and the reference implementation accepts it. **Workaround removed**: `SpaceNetworkFixture.ResolveSigningKeyAsync` is now just `SpaceSyncer.ResolveSigningKeyAsync(DidResolver)`, so the space commit-verification integration tests assert against the production path. Its private base58 decoder is gone. `docs/testing-spaces.md` and `docs/did-resolution.md` updated; CHANGELOG entry under `Fixed`. ## Verification `dotnet build` on the solution: 0 warnings, 0 errors. `dotnet test tests/ATProtoNet.Tests/ --configuration Release`: **1549 passed, 0 failed, 5 skipped** (the pre-existing Aspire-manifest gates). 14 new unit tests — `tests/ATProtoNet.Tests/Identity/DidDocumentKeyTests.cs` and a new section in `AtProtoCryptoTests`. The load-bearing ones build a legacy verification method from a real key pair's uncompressed point and assert it resolves to the same `did:key` the key reports for itself, *and* that a signature made by the private half verifies against the resolved key — looped until both parities of Y have been exercised, since a compression that ignored the parity bit would still produce a well-formed did:key for the wrong point half the time. The k256 case is guarded with the repo's existing `PlatformNotSupportedException` pattern (it ran here on Linux). The integration tests themselves still need a `dev-env` space network (`ATPROTO_TEST_SPACES=true`), which this container has no way to stand up, so the fixture change is compile-verified only — `tests/ATProtoNet.IntegrationTests/` builds clean. --- ⏱ 7m 32s · 41 turns · tokens: 84.8k in / 29.8k out (+2533.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-98/run-330.md) → Opened PR: https://git.grandiras.net/Grandiras/ATProto.NET/pulls/103

claude-bot referenced this issue

2026-08-20 21:56:22 +00:00

Claude: changes for issue #98 #103