From 5bef43b527bdbea0b91795fb03a83b713d63a9d7 Mon Sep 17 00:00:00 2001 From: lusoris Date: Fri, 23 Oct 2026 11:59:21 +0200 Subject: [PATCH] fix(adapter): return SSH string length errors instead of panicking When converting SSH string lengths to uint32, sshString panicked on overflow. Return ([]byte, error) instead and propagate wrapped errors naming each field across OpenSSH key blob generation and review receipt signing. Handle errors when generating the allowed_signers document during bundle writing. Add test coverage verifying that SSH string length overflow returns an error naming the affected field without panicking. Signed-off-by: lusoris --- tools/knowledge-adapter-v2/adapter.go | 6 +- tools/knowledge-adapter-v2/security_test.go | 56 +++++++++ tools/knowledge-adapter-v2/sshsig.go | 128 ++++++++++++++------ 3 files changed, 155 insertions(+), 35 deletions(-) diff --git a/tools/knowledge-adapter-v2/adapter.go b/tools/knowledge-adapter-v2/adapter.go index 483f59e..72a3642 100644 --- a/tools/knowledge-adapter-v2/adapter.go +++ b/tools/knowledge-adapter-v2/adapter.go @@ -280,6 +280,10 @@ func bundlePaths(root string, target acquisitionTarget, spec SourceSpec, proposa } func writeBundleFiles(bundle authorityBundle, specBytes, proposalBytes, receiptBytes, signature []byte) error { + allowedSigners, err := allowedSignersDocument() + if err != nil { + return fmt.Errorf("prepare allowed_signers: %w", err) + } writes := []struct { path string data []byte @@ -289,7 +293,7 @@ func writeBundleFiles(bundle authorityBundle, specBytes, proposalBytes, receiptB {bundle.proposalPath, proposalBytes, 0o600}, {bundle.receiptPath, receiptBytes, 0o600}, {bundle.signaturePath, signature, 0o600}, - {bundle.allowedSignersPath, allowedSignersDocument(), 0o600}, + {bundle.allowedSignersPath, allowedSigners, 0o600}, } for _, write := range writes { if err := os.WriteFile(write.path, write.data, write.mode); err != nil { diff --git a/tools/knowledge-adapter-v2/security_test.go b/tools/knowledge-adapter-v2/security_test.go index 333c08d..61fbeed 100644 --- a/tools/knowledge-adapter-v2/security_test.go +++ b/tools/knowledge-adapter-v2/security_test.go @@ -1,6 +1,8 @@ package main import ( + "crypto/ed25519" + "errors" "os" "path/filepath" "strings" @@ -62,3 +64,57 @@ func TestSSHStringLengthBounds(t *testing.T) { } } } + +func TestSSHStringOverflowReturnsError(t *testing.T) { + prev := sshStringLength + defer func() { sshStringLength = prev }() + + overflowErr := errors.New("SSH string exceeds uint32 length") + sshStringLength = func(length int) (uint32, error) { + return 0, overflowErr + } + + if _, err := sshString([]byte("test")); !errors.Is(err, overflowErr) { + t.Fatalf("sshString: expected overflow error, got %v", err) + } + + pub := devPrivateKey().Public().(ed25519.PublicKey) + if _, err := ed25519PublicBlob(pub); err == nil || !strings.Contains(err.Error(), "key type") { + t.Fatalf("ed25519PublicBlob: expected field in error, got %v", err) + } + + if _, err := allowedSignersDocument(); err == nil || !strings.Contains(err.Error(), "key type") { + t.Fatalf("allowedSignersDocument: expected field in error, got %v", err) + } +} + +func TestMarshalPrivateKeyFieldOverflow(t *testing.T) { + prev := sshStringLength + defer func() { sshStringLength = prev }() + + var failIdx int + var currentCall int + sshStringLength = func(length int) (uint32, error) { + currentCall++ + if currentCall == failIdx { + return 0, errors.New("SSH string exceeds uint32 length") + } + return defaultSSHStringLength(length) + } + + priv := devPrivateKey() + fields := []string{ + "key type", "public key", + "key type", "public key", "private key", "comment", + "cipher name", "kdf name", "kdf options", + "public key blob", "private key blob", + } + for i, field := range fields { + currentCall = 0 + failIdx = i + 1 + _, err := marshalOpenSSHPrivateKey(priv, "dev-comment") + if err == nil || !strings.Contains(err.Error(), field) { + t.Fatalf("field %q: expected field name in error, got %v", field, err) + } + } +} diff --git a/tools/knowledge-adapter-v2/sshsig.go b/tools/knowledge-adapter-v2/sshsig.go index 2f9fc64..5ec7827 100644 --- a/tools/knowledge-adapter-v2/sshsig.go +++ b/tools/knowledge-adapter-v2/sshsig.go @@ -38,68 +38,124 @@ func devPrivateKey() ed25519.PrivateKey { return ed25519.NewKeyFromSeed(devSeed()) } -func sshString(value []byte) []byte { +func sshString(value []byte) ([]byte, error) { length, err := sshStringLength(len(value)) if err != nil { - panic(err) + return nil, err } out := make([]byte, 4+len(value)) binary.BigEndian.PutUint32(out, length) copy(out[4:], value) - return out + return out, nil } -func sshStringLength(length int) (uint32, error) { +var sshStringLength = defaultSSHStringLength + +func defaultSSHStringLength(length int) (uint32, error) { if length < 0 || length > math.MaxUint32 { return 0, errors.New("SSH string exceeds uint32 length") } return uint32(length), nil } -func ed25519PublicBlob(pub ed25519.PublicKey) []byte { - var blob []byte - blob = append(blob, sshString([]byte("ssh-ed25519"))...) - blob = append(blob, sshString(pub)...) - return blob +type sshField struct { + name string + value []byte +} + +func appendSSHFields(dst []byte, fields []sshField) ([]byte, error) { + for _, f := range fields { + encoded, err := sshString(f.value) + if err != nil { + return nil, fmt.Errorf("%s: %w", f.name, err) + } + dst = append(dst, encoded...) + } + return dst, nil +} + +func ed25519PublicBlob(pub ed25519.PublicKey) ([]byte, error) { + return appendSSHFields(nil, []sshField{ + {"key type", []byte("ssh-ed25519")}, + {"public key", pub}, + }) } // allowedSignersDocument returns the operator-trusted allowed_signers policy // authorizing exactly the deterministic dev reviewer with its Ed25519 key. -func allowedSignersDocument() []byte { +func allowedSignersDocument() ([]byte, error) { pub := devPrivateKey().Public().(ed25519.PublicKey) - line := devReviewerPrincipal + " ssh-ed25519 " + base64.StdEncoding.EncodeToString(ed25519PublicBlob(pub)) + "\n" - return []byte(line) + pubBlob, err := ed25519PublicBlob(pub) + if err != nil { + return nil, fmt.Errorf("dev reviewer public blob: %w", err) + } + line := devReviewerPrincipal + " ssh-ed25519 " + base64.StdEncoding.EncodeToString(pubBlob) + "\n" + return []byte(line), nil } -// marshalOpenSSHPrivateKey encodes the deterministic dev key in the unencrypted -// OpenSSH private-key format so /usr/bin/ssh-keygen can sign with it. The two -// four-byte check words are fixed (they never affect the detached signature), -// keeping the transient key file itself deterministic. -func marshalOpenSSHPrivateKey(priv ed25519.PrivateKey, comment string) []byte { - pub := priv.Public().(ed25519.PublicKey) - pubBlob := ed25519PublicBlob(pub) +func marshalOpenSSHPrivateSection(pub ed25519.PublicKey, priv ed25519.PrivateKey, comment string) ([]byte, error) { check := []byte{0x63, 0x61, 0x75, 0x64} // "caud" - var private []byte + private := append([]byte(nil), check...) private = append(private, check...) - private = append(private, check...) - private = append(private, sshString([]byte("ssh-ed25519"))...) - private = append(private, sshString(pub)...) - private = append(private, sshString(priv)...) - private = append(private, sshString([]byte(comment))...) + var err error + private, err = appendSSHFields(private, []sshField{ + {"key type", []byte("ssh-ed25519")}, + {"public key", pub}, + {"private key", priv}, + {"comment", []byte(comment)}, + }) + if err != nil { + return nil, err + } for pad := byte(1); len(private)%8 != 0; pad++ { private = append(private, pad) } - var blob []byte - blob = append(blob, []byte("openssh-key-v1\x00")...) - blob = append(blob, sshString([]byte("none"))...) - blob = append(blob, sshString([]byte("none"))...) - blob = append(blob, sshString([]byte{})...) + return private, nil +} + +func marshalOpenSSHKeyBlob(pubBlob, private []byte) ([]byte, error) { + blob := append([]byte(nil), []byte("openssh-key-v1\x00")...) + var err error + blob, err = appendSSHFields(blob, []sshField{ + {"cipher name", []byte("none")}, + {"kdf name", []byte("none")}, + {"kdf options", []byte{}}, + }) + if err != nil { + return nil, err + } count := make([]byte, 4) binary.BigEndian.PutUint32(count, 1) blob = append(blob, count...) - blob = append(blob, sshString(pubBlob)...) - blob = append(blob, sshString(private)...) - return pem.EncodeToMemory(&pem.Block{Type: "OPENSSH PRIVATE KEY", Bytes: blob}) + blob, err = appendSSHFields(blob, []sshField{ + {"public key blob", pubBlob}, + {"private key blob", private}, + }) + if err != nil { + return nil, err + } + return blob, nil +} + +// marshalOpenSSHPrivateKey encodes the deterministic dev key in the unencrypted +// OpenSSH private-key format so /usr/bin/ssh-keygen can sign with it. The two +// four-byte check words are fixed (they never affect the detached signature), +// keeping the transient key file itself deterministic. +func marshalOpenSSHPrivateKey(priv ed25519.PrivateKey, comment string) ([]byte, error) { + pub := priv.Public().(ed25519.PublicKey) + pubBlob, err := ed25519PublicBlob(pub) + if err != nil { + return nil, fmt.Errorf("public key blob: %w", err) + } + private, err := marshalOpenSSHPrivateSection(pub, priv, comment) + if err != nil { + return nil, fmt.Errorf("private section: %w", err) + } + blob, err := marshalOpenSSHKeyBlob(pubBlob, private) + if err != nil { + return nil, fmt.Errorf("key container: %w", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "OPENSSH PRIVATE KEY", Bytes: blob}), nil } // signReviewReceipt writes the receipt and the transient dev key, produces a @@ -110,7 +166,11 @@ func signReviewReceipt(receiptBytes []byte, workDir string) ([]byte, error) { return nil, fmt.Errorf("prepare signing work dir: %w", err) } keyPath := filepath.Join(workDir, "dev-review-key") - if err := os.WriteFile(keyPath, marshalOpenSSHPrivateKey(devPrivateKey(), devReviewerPrincipal), 0o400); err != nil { + keyData, err := marshalOpenSSHPrivateKey(devPrivateKey(), devReviewerPrincipal) + if err != nil { + return nil, fmt.Errorf("marshal dev review key: %w", err) + } + if err := os.WriteFile(keyPath, keyData, 0o400); err != nil { return nil, fmt.Errorf("write transient dev review key: %w", err) } defer os.Remove(keyPath)