Skip to content

fix(metadata): reject ed25519 public keys with invalid length#739

Open
kanywst wants to merge 1 commit into
theupdateframework:masterfrom
kanywst:fix/ed25519-key-length-validation
Open

fix(metadata): reject ed25519 public keys with invalid length#739
kanywst wants to merge 1 commit into
theupdateframework:masterfrom
kanywst:fix/ed25519-key-length-validation

Conversation

@kanywst

@kanywst kanywst commented Jun 11, 2026

Copy link
Copy Markdown

ToPublicKey accepts an ed25519 public key of any length. crypto/ed25519.Verify panics when the length is not ed25519.PublicKeySize, so a malformed ed25519 key in root or delegated metadata panics VerifyDelegate instead of returning an error.

Repro against abd8cd2:

key := &metadata.Key{
    Type:   metadata.KeyTypeEd25519,
    Scheme: metadata.KeySchemeEd25519,
    Value:  metadata.KeyVal{PublicKey: "abcd"}, // decodes to 2 bytes
}
root := metadata.Root(time.Now().AddDate(1, 0, 0).UTC())
_ = root.Signed.AddKey(key, metadata.ROOT)
data, _ := root.ToBytes(false)
trustedmetadata.New(data)
panic: ed25519: bad public key length: 2
crypto/ed25519.Verify ...
.../signature.(*ED25519Verifier).VerifySignature ...
.../metadata.(*Metadata[...]).VerifyDelegate metadata/metadata.go:408
.../trustedmetadata.New metadata/trustedmetadata/trustedmetadata.go:51

Also reachable via updater.New (trusted root) and GetTargetInfo (a delegated role's delegations.keys).

Same panic-to-error case as #713 (Key.ID()), which did not cover ToPublicKey. Low severity: needs a malformed key that reaches verification, and affects availability only, not signature validity.

Includes a regression test.

Copilot AI review requested due to automatic review settings June 11, 2026 15:00
@kanywst
kanywst requested a review from a team as a code owner June 11, 2026 15:00

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds validation to reject malformed Ed25519 public keys early (preventing downstream panics) and introduces regression tests covering invalid-length keys.

Changes:

  • Add Ed25519 public key length check in Key.ToPublicKey().
  • Add a unit test that verifies invalid-length Ed25519 keys are rejected and do not trigger panics through VerifyDelegate.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
metadata/metadata_test.go Adds a test ensuring invalid-length Ed25519 public keys error and don’t panic in delegate verification.
metadata/keys.go Rejects non-32-byte Ed25519 public keys in ToPublicKey() to avoid later panics in verification.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread metadata/metadata_test.go
Comment thread metadata/keys.go Outdated
crypto/ed25519.Verify panics when the public key length is not
ed25519.PublicKeySize, so a malformed ed25519 key in root or delegated
metadata panicked VerifyDelegate instead of returning an error. Validate
the length on decode, matching the panic-to-error change for Key.ID() in theupdateframework#713.

Adds a regression test.

Signed-off-by: kanywst <niwatakuma@icloud.com>
@kanywst
kanywst force-pushed the fix/ed25519-key-length-validation branch from 11c11d9 to b69fc50 Compare June 11, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants