From 9d1c77c96966330b197a4dab709f06fbfd72b730 Mon Sep 17 00:00:00 2001 From: Dwi Siswanto Date: Tue, 14 Jul 2026 11:43:52 +0700 Subject: [PATCH] fix(signer): avoid deprecated ECDSA coordinate access Build the signer fragment from `PublicKey.Bytes()` instead of reading the raw ECDSA X coordinate. Keep the existing fragment format by hashing the trimmed X-coordinate bytes, and propagate key encoding errors thru signing & verification. Signed-off-by: Dwi Siswanto --- pkg/templates/signer/tmpl_signer.go | 54 +++++++++++++++++++----- pkg/templates/signer/tmpl_signer_test.go | 45 ++++++++++++++++++++ 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/pkg/templates/signer/tmpl_signer.go b/pkg/templates/signer/tmpl_signer.go index 546581be33..962dd13119 100644 --- a/pkg/templates/signer/tmpl_signer.go +++ b/pkg/templates/signer/tmpl_signer.go @@ -48,8 +48,9 @@ type SignableTemplate interface { type TemplateSigner struct { sync.Once - handler *KeyHandler - fragment string + handler *KeyHandler + fragment string + fragmentErr error } // Identifier returns the identifier for the template signer @@ -58,16 +59,36 @@ func (t *TemplateSigner) Identifier() string { } // fragment is optional part of signature that is used to identify the user -// who signed the template via md5 hash of public key +// who signed the template via md5 hash of the public key x-coordinate func (t *TemplateSigner) GetUserFragment() string { + fragment, _ := t.userFragment() + return fragment +} + +func (t *TemplateSigner) userFragment() (string, error) { // wrap with sync.Once to reduce unnecessary md5 hashing t.Do(func() { - if t.handler.ecdsaPubKey != nil { - hashed := md5.Sum(t.handler.ecdsaPubKey.X.Bytes()) - t.fragment = fmt.Sprintf("%x", hashed) - } + t.fragment, t.fragmentErr = publicKeyFragment(t.handler.ecdsaPubKey) }) - return t.fragment + return t.fragment, t.fragmentErr +} + +func publicKeyFragment(publicKey *ecdsa.PublicKey) (string, error) { + if publicKey == nil { + return "", nil + } + publicKeyBytes, err := publicKey.Bytes() + if err != nil { + return "", fmt.Errorf("encode ecdsa public key: %w", err) + } + if len(publicKeyBytes) < 3 || publicKeyBytes[0] != 4 || (len(publicKeyBytes)-1)%2 != 0 { + return "", fmt.Errorf("invalid uncompressed ecdsa public key") + } + xCoordinateLength := (len(publicKeyBytes) - 1) / 2 + // Keep the old fragment stable: big.Int.Bytes omitted leading zero bytes. + xCoordinateBytes := bytes.TrimLeft(publicKeyBytes[1:1+xCoordinateLength], "\x00") + hashed := md5.Sum(xCoordinateBytes) + return fmt.Sprintf("%x", hashed), nil } // Sign signs the given template with the template signer and returns the signature @@ -87,7 +108,10 @@ func (t *TemplateSigner) Sign(data []byte, tmpl SignableTemplate) (string, error } if len(arr) == 3 { // signature has fragment verify if it is equal to current fragment - fragment := t.GetUserFragment() + fragment, err := t.userFragment() + if err != nil { + return "", err + } if fragment != arr[2] { return "", errkit.New("re-signing code templates are not allowed for security reasons.") } @@ -125,7 +149,11 @@ func (t *TemplateSigner) sign(data []byte) (string, error) { if err := gob.NewEncoder(&signatureData).Encode(ecdsaSignature); err != nil { return "", err } - return fmt.Sprintf(SignatureFmt, signatureData.Bytes(), t.GetUserFragment()), nil + fragment, err := t.userFragment() + if err != nil { + return "", err + } + return fmt.Sprintf(SignatureFmt, signatureData.Bytes(), fragment), nil } // Verify verifies the given template with the template signer @@ -140,8 +168,12 @@ func (t *TemplateSigner) Verify(data []byte, tmpl SignableTemplate) (bool, error } digestData := bytes.TrimSpace(bytes.TrimPrefix(signature, []byte(SignaturePattern))) + fragment, err := t.userFragment() + if err != nil { + return false, err + } // remove fragment from digest as it is used for re-signing purposes only - digestString := strings.TrimSuffix(string(digestData), ":"+t.GetUserFragment()) + digestString := strings.TrimSuffix(string(digestData), ":"+fragment) digest, err := hex.DecodeString(digestString) if err != nil { return false, err diff --git a/pkg/templates/signer/tmpl_signer_test.go b/pkg/templates/signer/tmpl_signer_test.go index e273b94f9d..15299c47c8 100644 --- a/pkg/templates/signer/tmpl_signer_test.go +++ b/pkg/templates/signer/tmpl_signer_test.go @@ -2,6 +2,12 @@ package signer import ( "bytes" + "crypto/ecdh" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/md5" + "encoding/binary" + "fmt" "os" "path/filepath" "testing" @@ -30,6 +36,45 @@ func (m *mockSignableTemplate) HasCodeProtocol() bool { var signer, _ = NewTemplateSignerFromFiles(testCertFile, testKeyFile) +func TestPublicKeyFragmentTrimsLeadingZeroXCoordinate(t *testing.T) { + publicKey, publicKeyBytes := p256PublicKeyWithLeadingZeroX(t) + xCoordinate := publicKeyBytes[1:33] + require.Zero(t, xCoordinate[0]) + + legacyXCoordinate := bytes.TrimLeft(xCoordinate, "\x00") + legacyHash := md5.Sum(legacyXCoordinate) + untrimmedHash := md5.Sum(xCoordinate) + require.NotEqual(t, untrimmedHash, legacyHash) + + fragment, err := publicKeyFragment(publicKey) + require.NoError(t, err) + require.Equal(t, fmt.Sprintf("%x", legacyHash), fragment) +} + +func p256PublicKeyWithLeadingZeroX(t *testing.T) (*ecdsa.PublicKey, []byte) { + t.Helper() + + var privateKeyBytes [32]byte + for i := uint64(1); i < 10_000; i++ { + binary.BigEndian.PutUint64(privateKeyBytes[24:], i) + privateKey, err := ecdh.P256().NewPrivateKey(privateKeyBytes[:]) + require.NoError(t, err) + + publicKeyBytes := privateKey.PublicKey().Bytes() + require.Len(t, publicKeyBytes, 65) + require.Equal(t, byte(4), publicKeyBytes[0]) + if publicKeyBytes[1] != 0 { + continue + } + + publicKey, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), publicKeyBytes) + require.NoError(t, err) + return publicKey, publicKeyBytes + } + t.Fatal("failed to find a P-256 public key with a leading-zero x-coordinate") + return nil, nil +} + func TestTemplateSignerSignAndVerify(t *testing.T) { tempDir := t.TempDir()