From 11e1457506ed17f94c04dc9cca55ad0ac4889aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 12 Aug 2026 13:54:47 +0200 Subject: [PATCH] fix(identity): stop formatting signers as raw private keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multikey signers are byte slices holding the private key, so passing one to a formatting verb writes the key material into the output. Two error messages in EncodeSignerToPEM did exactly that. Add identity.Signer, a wrapper whose String() returns the key DID, and use it wherever the identity package holds a signer. Fixing the type rather than the two call sites keeps future fmt and log calls safe. The same wrapper belongs on the multikey signer implementations in ucantone, which would cover every dependency at once; this change makes libforge safe in the meantime. Signed-off-by: Miroslav Bajtoš Assisted-by: Claude:claude-opus-5[1m] --- identity/identity.go | 7 ++++--- identity/pem.go | 21 +++++++++++++++------ identity/pem_test.go | 30 ++++++++++++++++++++++++++++++ identity/signer.go | 27 +++++++++++++++++++++++++++ identity/signer_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 identity/signer.go create mode 100644 identity/signer_test.go diff --git a/identity/identity.go b/identity/identity.go index 717dbe4..0bf77c4 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -20,23 +20,24 @@ type Identity struct { // New creates a new identity. If privateKeyBase64 is empty, generates a new // key. If serviceDID is empty, uses the key DID derived from the key. func New(privateKeyBase64 string, serviceDID string) (Identity, error) { - var signer multikey.Signer + var keySigner multikey.Signer var issuer multikey.Issuer var err error if privateKeyBase64 == "" { // Generate ephemeral identity - signer, err = ed25519.Generate() + keySigner, err = ed25519.Generate() if err != nil { return Identity{}, fmt.Errorf("failed to generate signer: %w", err) } } else { // Decode provided key - signer, err = ed25519.Parse(privateKeyBase64) + keySigner, err = ed25519.Parse(privateKeyBase64) if err != nil { return Identity{}, fmt.Errorf("failed to create signer from key: %w", err) } } + signer := NewSigner(keySigner) if serviceDID == "" { issuer = multikey.KeyIssuer(signer) diff --git a/identity/pem.go b/identity/pem.go index bee6bee..d73a7e9 100644 --- a/identity/pem.go +++ b/identity/pem.go @@ -13,7 +13,11 @@ import ( // EncodeSignerToPEM encodes a signer to a PKCS#8 PEM format. The signer's key // should be of a type supported by ["crypto/x509".MarshalPKCS8PrivateKey]. -func EncodeSignerToPEM(signer multikey.Signer) ([]byte, error) { +func EncodeSignerToPEM(keySigner multikey.Signer) ([]byte, error) { + // Wrap the signer so the error messages below name it by its DID instead of + // printing its private key bytes. + signer := NewSigner(keySigner) + privateKeyBytes, err := x509.MarshalPKCS8PrivateKey(signer.PrivateKey()) if err != nil { return nil, fmt.Errorf("marshaling private key of signer %s: %w", signer, err) @@ -34,7 +38,7 @@ func EncodeSignerToPEM(signer multikey.Signer) ([]byte, error) { // DecodeSignerFromPEM loads a private key from a PKCS#8 PEM as a signer. // Currently, only Ed25519 keys are supported. -func DecodeSignerFromPEM(pemData []byte) (multikey.Signer, error) { +func DecodeSignerFromPEM(pemData []byte) (Signer, error) { var privateKey *crypto_ed25519.PrivateKey rest := pemData for { @@ -47,12 +51,12 @@ func DecodeSignerFromPEM(pemData []byte) (multikey.Signer, error) { if block.Type == "PRIVATE KEY" { parsedKey, err := x509.ParsePKCS8PrivateKey(block.Bytes) if err != nil { - return nil, fmt.Errorf("parsing PKCS#8 private key: %w", err) + return Signer{}, fmt.Errorf("parsing PKCS#8 private key: %w", err) } key, ok := parsedKey.(crypto_ed25519.PrivateKey) if !ok { - return nil, fmt.Errorf("key is not an Ed25519 private key") + return Signer{}, fmt.Errorf("key is not an Ed25519 private key") } privateKey = &key break @@ -60,8 +64,13 @@ func DecodeSignerFromPEM(pemData []byte) (multikey.Signer, error) { } if privateKey == nil { - return nil, fmt.Errorf("no PRIVATE KEY block found in PEM file") + return Signer{}, fmt.Errorf("no PRIVATE KEY block found in PEM file") + } + + signer, err := ed25519.FromRaw(privateKey.Seed()) + if err != nil { + return Signer{}, fmt.Errorf("creating signer from private key: %w", err) } - return ed25519.FromRaw(privateKey.Seed()) + return NewSigner(signer), nil } diff --git a/identity/pem_test.go b/identity/pem_test.go index fe21bbc..8fcc867 100644 --- a/identity/pem_test.go +++ b/identity/pem_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/fil-forge/libforge/identity" + "github.com/fil-forge/ucantone/multikey" "github.com/fil-forge/ucantone/multikey/ed25519" "github.com/stretchr/testify/require" ) @@ -24,6 +25,35 @@ func TestEd25519SignerPEMRoundTrip(t *testing.T) { require.Equal(t, original.KeyDID(), decoded.KeyDID()) } +// unmarshalableSigner reports a private key that +// ["crypto/x509".MarshalPKCS8PrivateKey] cannot encode, to exercise the error +// path of EncodeSignerToPEM. +type unmarshalableSigner struct { + multikey.Signer +} + +func (s unmarshalableSigner) PrivateKey() any { + return struct{}{} +} + +func TestEncodeSignerToPEM_MarshalErrorNamesSignerByDID(t *testing.T) { + keySigner, err := ed25519.Generate() + require.NoError(t, err) + + _, err = identity.EncodeSignerToPEM(unmarshalableSigner{keySigner}) + + require.ErrorContains(t, err, "marshaling private key of signer "+keySigner.KeyDID().String()) +} + +func TestEncodeSignerToPEM_MarshalErrorHidesPrivateKey(t *testing.T) { + keySigner, err := ed25519.Generate() + require.NoError(t, err) + + _, err = identity.EncodeSignerToPEM(unmarshalableSigner{keySigner}) + + require.NotContains(t, err.Error(), string(keySigner.Raw())) +} + func TestDecodeEd25519SignerFromPEM_NoPrivateKeyBlock(t *testing.T) { pemData := []byte("-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----\n") _, err := identity.DecodeSignerFromPEM(pemData) diff --git a/identity/signer.go b/identity/signer.go new file mode 100644 index 0000000..5103bea --- /dev/null +++ b/identity/signer.go @@ -0,0 +1,27 @@ +package identity + +import ( + "github.com/fil-forge/ucantone/multikey" +) + +// Signer is a [multikey.Signer] that is safe to pass to formatting and logging +// functions. Multikey signers are byte slices holding the private key, so +// formatting one directly writes the key material into the output. +// +// Prefer this type over [multikey.Signer] wherever libforge holds a signer. +type Signer struct { + multikey.Signer +} + +var _ multikey.Signer = Signer{} + +// NewSigner wraps a multikey signer so that formatting it is safe. +func NewSigner(signer multikey.Signer) Signer { + return Signer{Signer: signer} +} + +// String returns the DID of the signer's key. It never returns private key +// material. +func (s Signer) String() string { + return s.KeyDID().String() +} diff --git a/identity/signer_test.go b/identity/signer_test.go new file mode 100644 index 0000000..33edc08 --- /dev/null +++ b/identity/signer_test.go @@ -0,0 +1,41 @@ +package identity_test + +import ( + "fmt" + "testing" + + "github.com/fil-forge/libforge/identity" + "github.com/fil-forge/ucantone/multikey/ed25519" + "github.com/stretchr/testify/require" +) + +func TestSignerStringReturnsKeyDID(t *testing.T) { + keySigner, err := ed25519.Generate() + require.NoError(t, err) + + signer := identity.NewSigner(keySigner) + + require.Equal(t, keySigner.KeyDID().String(), signer.String()) +} + +// Formatting verbs a signer may plausibly reach in an error message or a log +// line. None of them may print the private key. +var signerFormatVerbs = map[string]string{ + "%s": "%s", + "%v": "%s", + "%q": "%q", +} + +func TestSignerFormattingPrintsKeyDID(t *testing.T) { + for verb, expectedVerb := range signerFormatVerbs { + t.Run(verb, func(t *testing.T) { + keySigner, err := ed25519.Generate() + require.NoError(t, err) + + formatted := fmt.Sprintf(verb, identity.NewSigner(keySigner)) + + expected := fmt.Sprintf(expectedVerb, keySigner.KeyDID().String()) + require.Equal(t, expected, formatted) + }) + } +}