manifest: extend signing payload to cover the whole manifest

pkg/manifest/manifest.go

@@ -105,7 +105,11 @@ type Condition struct {
 type Store struct {
 	// Publisher pubkey, base64 ed25519 ("ed25519:<base64>").
 	Publisher string `json:"publisher"`
-	// Signature: store's signature over (id || manifest_version || binary.sha256 || grants-hash).
+	// Signature: store's signature over the manifest signing payload. New
+	// signatures use the v2 payload, which commits to the publisher key plus
+	// a hash of the whole manifest; the older v1 payload covered only
+	// (id || manifest_version || binary.sha256 || grants-hash) and is still
+	// accepted on verify. See SigningPayload.
 	Signature string `json:"signature"`
 }
 
@@ -177,15 +181,30 @@ func canonicalJSON(v any) ([]byte, error) {
 	return json.Marshal(v)
 }
 
-// signingPayload builds the canonical byte-string the Store.Signature
-// must sign. The publisher key is included so that a signature cannot
-// be reused with a different publisher identity — swapping the
-// publisher key invalidates the signature. Once a trust-anchor check
-// (hardcoded publisher pubkey match) is added, this guarantees the
-// manifest was signed by the known publisher.
+// SigningPayloadV2Prefix is the domain-separation tag that opens every v2
+// signing payload. It keeps v2 payload bytes disjoint from v1 payload bytes,
+// so a signature produced for one scheme can never verify under the other.
+const SigningPayloadV2Prefix = "pilot.app-manifest.v2"
+
+// AllowLegacySignaturePayload controls whether VerifySignature falls back to
+// the v1 signing payload when the v2 payload does not verify. Manifests signed
+// before v2 existed carry v1 signatures, so this defaults to true. Set it to
+// false to accept v2 signatures only.
+var AllowLegacySignaturePayload = true
+
+// signingPayloadV1 builds the original canonical byte-string for
+// Store.Signature. The publisher key is included so that a signature cannot
+// be reused with a different publisher identity — swapping the publisher key
+// invalidates the signature.
 //
 // Format: publisher || ":" || id || ":" || manifest_version || ":" || binary.sha256 || ":" || grants-sha256-hex
-func (m *Manifest) signingPayload() ([]byte, error) {
+//
+// It covers Store.Publisher, ID, ManifestVersion, Binary.SHA256 and Grants.
+// Every other manifest field — Binary.Runtime, Binary.Path, Exposes,
+// Protection, Affiliates, Depends, Extends, DynamicExtends, AppVersion — sits
+// outside these bytes and can therefore be edited without disturbing a v1
+// signature. signingPayloadV2 closes that gap.
+func (m *Manifest) signingPayloadV1() ([]byte, error) {
 	grantsJSON, err := canonicalJSON(m.Grants)
 	if err != nil {
 		return nil, fmt.Errorf("grants marshal: %w", err)
@@ -196,6 +215,65 @@ func (m *Manifest) signingPayload() ([]byte, error) {
 	return []byte(payload), nil
 }
 
+// signingPayloadV2 builds the current canonical byte-string for
+// Store.Signature. It commits to the whole manifest rather than a hand-picked
+// subset: the payload is the domain-separation prefix, the publisher key, and
+// the sha256 of the canonical JSON encoding of the manifest with
+// Store.Signature blanked out.
+//
+// Blanking Store.Signature is what makes the hash computable on both sides —
+// the signer does not yet have a signature, and the verifier must reproduce
+// the same pre-signature bytes. Everything else is included verbatim, so any
+// edit to Binary.Runtime, Binary.Path, Exposes, Protection, Affiliates,
+// Depends, Extends, DynamicExtends or AppVersion changes the hash and the
+// signature no longer verifies.
+//
+// Format: "pilot.app-manifest.v2" || ":" || publisher || ":" || manifest-sha256-hex
+func (m *Manifest) signingPayloadV2() ([]byte, error) {
+	// Shallow copy: only the Store.Signature scalar is rewritten, and the
+	// copy is discarded once hashed, so the caller's manifest is untouched.
+	unsigned := *m
+	unsigned.Store.Signature = ""
+
+	body, err := canonicalJSON(&unsigned)
+	if err != nil {
+		return nil, fmt.Errorf("manifest marshal: %w", err)
+	}
+	bodyHash := sha256.Sum256(body)
+	payload := fmt.Sprintf("%s:%s:%x", SigningPayloadV2Prefix, m.Store.Publisher, bodyHash)
+	return []byte(payload), nil
+}
+
+// SigningPayload returns the bytes a publisher must sign to produce
+// Store.Signature. New signatures use the v2 payload.
+func (m *Manifest) SigningPayload() ([]byte, error) {
+	return m.signingPayloadV2()
+}
+
+// Sign fills in Store.Signature with an ed25519 signature over the v2 signing
+// payload and returns the base64 signature it stored. Store.Publisher must
+// already be set to the "ed25519:<base64>" form of priv's public key, since the
+// publisher string is part of the payload.
+func (m *Manifest) Sign(priv ed25519.PrivateKey) (string, error) {
+	if len(priv) != ed25519.PrivateKeySize {
+		return "", fmt.Errorf("private key: wrong length %d, want %d", len(priv), ed25519.PrivateKeySize)
+	}
+	pubkey, err := decodeEd25519Pub(m.Store.Publisher)
+	if err != nil {
+		return "", fmt.Errorf("store.publisher: %w", err)
+	}
+	if !bytes.Equal(pubkey, priv.Public().(ed25519.PublicKey)) {
+		return "", fmt.Errorf("store.publisher does not match the signing key")
+	}
+	payload, err := m.signingPayloadV2()
+	if err != nil {
+		return "", err
+	}
+	sig := base64.StdEncoding.EncodeToString(ed25519.Sign(priv, payload))
+	m.Store.Signature = sig
+	return sig, nil
+}
+
 // decodeEd25519Pub parses an "ed25519:<base64>" (or bare base64) public key
 // into raw bytes, validating the length.
 func decodeEd25519Pub(s string) ([]byte, error) {
@@ -245,11 +323,17 @@ func (m *Manifest) VerifyTrustAnchor(cataloguePublisher string) error {
 }
 
 // VerifySignature checks that Store.Signature is a valid ed25519
-// signature over the signing payload, verified against the Store.Publisher
-// key embedded in the manifest. This provides cryptographic integrity —
-// tampering with any manifest field that feeds the signing payload
-// (Publisher, ID, ManifestVersion, Binary.SHA256, Grants) will cause
-// verification to fail.
+// signature over a signing payload, verified against the Store.Publisher
+// key embedded in the manifest.
+//
+// Two payload versions are accepted. The v2 payload commits to the entire
+// manifest, so editing any field — including Binary.Path, Binary.Runtime,
+// Exposes, Protection, Affiliates, Depends, Extends and DynamicExtends —
+// invalidates the signature. The v1 payload covers only Publisher, ID,
+// ManifestVersion, Binary.SHA256 and Grants; it is tried as a fallback while
+// AllowLegacySignaturePayload is true so that manifests signed before v2 keep
+// verifying. New signatures should be produced with Sign / SigningPayload,
+// which emit v2.
 //
 // IMPORTANT: This does NOT check that Store.Publisher is trusted. For
 // non-sideloaded apps, callers MUST also call VerifyTrustAnchor(cataloguePublisher)
@@ -278,12 +362,23 @@ func (m *Manifest) VerifySignature() error {
 		return fmt.Errorf("store.signature: wrong signature length %d, want %d", len(sig), ed25519.SignatureSize)
 	}
 
-	payload, err := m.signingPayload()
+	payloadV2, err := m.signingPayloadV2()
 	if err != nil {
 		return err
 	}
-	if !ed25519.Verify(pubkey, payload, sig) {
-		return fmt.Errorf("store.signature: verification failed — manifest may have been tampered with")
+	if ed25519.Verify(pubkey, payloadV2, sig) {
+		return nil
 	}
-	return nil
+
+	if AllowLegacySignaturePayload {
+		payloadV1, err := m.signingPayloadV1()
+		if err != nil {
+			return err
+		}
+		if ed25519.Verify(pubkey, payloadV1, sig) {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("store.signature: verification failed — manifest contents do not match the signature")
 }

pkg/manifest/zz3_signing_payload_test.go

@@ -0,0 +1,214 @@
+package manifest
+
+import (
+	"crypto/ed25519"
+	"crypto/rand"
+	"testing"
+)
+
+// signedV2 returns a valid wallet manifest signed with the v2 payload.
+func signedV2(t *testing.T) *Manifest {
+	t.Helper()
+	pub, priv, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m := mustValid(t)
+	m.Store.Publisher = "ed25519:" + base64Enc(pub)
+	if _, err := m.Sign(priv); err != nil {
+		t.Fatalf("sign: %v", err)
+	}
+	if err := m.VerifySignature(); err != nil {
+		t.Fatalf("freshly signed manifest rejected: %v", err)
+	}
+	return m
+}
+
+// TestV2PayloadCoversWholeManifest walks every field that the v1 payload left
+// outside the signed bytes and asserts that editing it now breaks verification.
+func TestV2PayloadCoversWholeManifest(t *testing.T) {
+	cases := []struct {
+		name   string
+		mutate func(*Manifest)
+	}{
+		{"binary.path", func(m *Manifest) { m.Binary.Path = "bin/other" }},
+		{"binary.runtime", func(m *Manifest) { m.Binary.Runtime = "bun" }},
+		{"binary.sha256", func(m *Manifest) {
+			m.Binary.SHA256 = "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
+		}},
+		{"exposes", func(m *Manifest) { m.Exposes = append(m.Exposes, "wallet.drain") }},
+		{"protection", func(m *Manifest) { m.Protection = "shareable" }},
+		{"affiliates.pubkey", func(m *Manifest) { m.Affiliates[0].Pubkey = "ed25519:CCCC" }},
+		{"affiliates.append", func(m *Manifest) {
+			m.Affiliates = append(m.Affiliates, Affiliate{Pubkey: "ed25519:DDDD", Role: "settlement", Purpose: "extra"})
+		}},
+		{"depends", func(m *Manifest) {
+			m.Depends = append(m.Depends, Dependency{App: "io.pilot.keyring", Methods: []string{"keyring.export"}})
+		}},
+		{"extends", func(m *Manifest) {
+			m.Extends = append(m.Extends, Extension{Primitive: "send-message.pre", Method: "wallet.verify"})
+		}},
+		{"dynamic_extends", func(m *Manifest) {
+			m.DynamicExtends = append(m.DynamicExtends, "send-message.pre")
+		}},
+		{"app_version", func(m *Manifest) { m.AppVersion = "9.9.9" }},
+		{"id", func(m *Manifest) { m.ID = "io.pilot.other" }},
+		{"manifest_version", func(m *Manifest) { m.ManifestVersion = 2 }},
+		{"grants.cap", func(m *Manifest) { m.Grants[0].Cap = "proc.exec" }},
+		{"grants.target", func(m *Manifest) { m.Grants[0].Target = "/etc/passwd" }},
+		{"grants.condition", func(m *Manifest) { m.Grants[1].Condition = nil }},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			m := signedV2(t)
+			tc.mutate(m)
+			if err := m.VerifySignature(); err == nil {
+				t.Errorf("expected verification failure after editing %s, got nil", tc.name)
+			}
+		})
+	}
+}
+
+// TestV1SignatureStillVerifies pins the compatibility guarantee: manifests
+// signed with the v1 payload keep verifying while the legacy fallback is on.
+func TestV1SignatureStillVerifies(t *testing.T) {
+	pub, priv, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m := mustValid(t)
+	m.Store.Publisher = "ed25519:" + base64Enc(pub)
+	sig, err := signTestManifest(m, priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.Store.Signature = sig
+
+	if !AllowLegacySignaturePayload {
+		t.Fatal("AllowLegacySignaturePayload must default to true")
+	}
+	if err := m.VerifySignature(); err != nil {
+		t.Errorf("v1 signature rejected with legacy fallback enabled: %v", err)
+	}
+}
+
+// TestLegacyFallbackCanBeDisabled checks the toggle in both directions: with
+// the fallback off, only v2 signatures verify.
+func TestLegacyFallbackCanBeDisabled(t *testing.T) {
+	prev := AllowLegacySignaturePayload
+	t.Cleanup(func() { AllowLegacySignaturePayload = prev })
+
+	pub, priv, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m := mustValid(t)
+	m.Store.Publisher = "ed25519:" + base64Enc(pub)
+	v1Sig, err := signTestManifest(m, priv)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.Store.Signature = v1Sig
+
+	AllowLegacySignaturePayload = false
+	if err := m.VerifySignature(); err == nil {
+		t.Error("expected v1 signature to be rejected with the legacy fallback disabled")
+	}
+
+	if _, err := m.Sign(priv); err != nil {
+		t.Fatalf("sign: %v", err)
+	}
+	if err := m.VerifySignature(); err != nil {
+		t.Errorf("v2 signature rejected with the legacy fallback disabled: %v", err)
+	}
+}
+
+// TestV1PayloadLeavesFieldsUnsigned records the difference between the two
+// payloads: a v1 signature does not move when Binary.Path changes, which is
+// exactly the range v2 extends over.
+func TestV1PayloadLeavesFieldsUnsigned(t *testing.T) {
+	m := mustValid(t)
+	m.Store.Publisher = "ed25519:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="
+
+	before, err := m.signingPayloadV1()
+	if err != nil {
+		t.Fatal(err)
+	}
+	beforeV2, err := m.signingPayloadV2()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	m.Binary.Path = "bin/elsewhere"
+	m.DynamicExtends = append(m.DynamicExtends, "recv.post")
+
+	after, err := m.signingPayloadV1()
+	if err != nil {
+		t.Fatal(err)
+	}
+	afterV2, err := m.signingPayloadV2()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if string(before) != string(after) {
+		t.Errorf("v1 payload unexpectedly changed:\n%s\n%s", before, after)
+	}
+	if string(beforeV2) == string(afterV2) {
+		t.Error("v2 payload did not change after editing binary.path and dynamic_extends")
+	}
+}
+
+// TestSigningPayloadIndependentOfSignatureField confirms the payload is stable
+// whether or not Store.Signature is populated, so signer and verifier agree.
+func TestSigningPayloadIndependentOfSignatureField(t *testing.T) {
+	m := mustValid(t)
+	m.Store.Signature = ""
+	unsigned, err := m.SigningPayload()
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.Store.Signature = "ed25519:AAAA"
+	populated, err := m.SigningPayload()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(unsigned) != string(populated) {
+		t.Errorf("payload depends on store.signature:\n%s\n%s", unsigned, populated)
+	}
+	if m.Store.Signature != "ed25519:AAAA" {
+		t.Errorf("SigningPayload mutated store.signature: %q", m.Store.Signature)
+	}
+}
+
+func TestSignRejectsMismatchedPublisher(t *testing.T) {
+	_, priv, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	otherPub, _, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m := mustValid(t)
+	m.Store.Publisher = "ed25519:" + base64Enc(otherPub)
+	if _, err := m.Sign(priv); err == nil {
+		t.Error("expected Sign to refuse a publisher that is not the signing key")
+	}
+}
+
+// TestV2SignatureIsNotAcceptedForAnotherPublisher checks the publisher binding
+// survives in v2: re-labelling the manifest with a different publisher key
+// breaks the signature under both payload versions.
+func TestV2SignatureIsNotAcceptedForAnotherPublisher(t *testing.T) {
+	m := signedV2(t)
+	otherPub, _, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.Store.Publisher = "ed25519:" + base64Enc(otherPub)
+	if err := m.VerifySignature(); err == nil {
+		t.Error("expected verification failure after swapping store.publisher")
+	}
+}