bug(fxconfig/transaction): parseCertificateOrPublicKey silently swallows parse errors, giving misleading diagnostics on invalid PEM key files

[Prachi194agrawal]

Summary

parseCertificateOrPublicKey() in tools/fxconfig/internal/transaction/policy.go has two silent failure paths that discard the real error and always return the same generic "no ECDSA public key in block" message — regardless of what actually went wrong. This makes diagnosing --threshold-policy file issues unnecessarily hard in practice.

---

Affected File

tools/fxconfig/internal/transaction/policy.go
Function: parseCertificateOrPublicKey, called via getPubKeyFromPemData → CreateThresholdPolicy

---

Root Cause

The function has two code paths where the real error is silently dropped:

func parseCertificateOrPublicKey(blockBytes []byte) ([]byte, error) {
    cert, err := x509.ParseCertificate(blockBytes)
    var publicKey any
    if err == nil {
        if cert.PublicKey != nil && cert.PublicKeyAlgorithm == x509.ECDSA {
            publicKey = cert.PublicKey
        }
        // ❌ Silent path 1: cert parsed fine but isn't ECDSA (e.g. RSA).
        // publicKey stays nil — no context returned about the algorithm mismatch.
    } else {
        anyPublicKey, err2 := x509.ParsePKIXPublicKey(blockBytes)
        if err2 == nil && anyPublicKey != nil {
            publicKey, ok = anyPublicKey.(*ecdsa.PublicKey)
            if !ok {
                return nil, errors.New("public key is not a ecdsa public key")
            }
        }
        // ❌ Silent path 2: err2 != nil (corrupt/malformed bytes).
        // err2 is never propagated — falls through silently.
    }

    if publicKey == nil {
        return nil, errors.New("no ECDSA public key in block") // same message for all failure paths
    }

The caller getPubKeyFromPemData further swallows this via continue, so the end user sees:
no ECDSA public key in pem file

…with no indication of the actual root cause.

---

Two Confirmed Failure Scenarios

Scenario A — Valid non-ECDSA certificate (e.g. RSA cert)

x509.ParseCertificate succeeds, but cert.PublicKeyAlgorithm != x509.ECDSA, so publicKey stays nil. The function returns the generic error "no ECDSA public key in block" with no mention that the file held a perfectly valid RSA certificate with the wrong algorithm.

Scenario B — Corrupt or malformed PEM block bytes

x509.ParseCertificate fails (expected). The fallback x509.ParsePKIXPublicKey also fails with a real asn1: structure error, but err2 is never returned or wrapped — it falls through silently. The user still sees the same generic message, with the underlying ASN.1 error completely lost.

---

Reproducer

Add the following tests to tools/fxconfig/internal/transaction/policy_parse_test.go:

package transaction

import (
    "crypto/ecdsa"
    "crypto/elliptic"
    "crypto/rand"
    "crypto/rsa"
    "crypto/x509"
    "crypto/x509/pkix"
    "math/big"
    "strings"
    "testing"
    "time"
)

// Scenario A: Valid RSA certificate, wrong algorithm.
func TestParseCertificateOrPublicKey_RSACertGivesGenericError(t *testing.T) {
    rsaKey, err := rsa.GenerateKey(rand.Reader, 2048)
    if err != nil {
        t.Fatalf("rsa keygen: %v", err)
    }
    template := &x509.Certificate{
        SerialNumber: big.NewInt(1),
        Subject:      pkix.Name{CommonName: "test-rsa"},
        NotBefore:    time.Now(),
        NotAfter:     time.Now().Add(time.Hour),
    }
    certDER, err := x509.CreateCertificate(rand.Reader, template, template, &rsaKey.PublicKey, rsaKey)
    if err != nil {
        t.Fatalf("create cert: %v", err)
    }
    _, gotErr := parseCertificateOrPublicKey(certDER)
    if gotErr == nil {
        t.Fatal("expected an error for RSA cert, got nil")
    }
    if strings.Contains(gotErr.Error(), "RSA") || strings.Contains(gotErr.Error(), "algorithm") {
        t.Logf("GOOD: error mentions algorithm: %v", gotErr)
    } else {
        t.Logf("BUG CONFIRMED (Scenario A): error is generic, root cause lost: %q", gotErr.Error())
    }
}

// Scenario B: Corrupt bytes drop real parse error.
func TestParseCertificateOrPublicKey_CorruptBytesDropsRealErr(t *testing.T) {
    corrupt := []byte("this is definitely not valid ASN.1 DER data")
    _, gotErr := parseCertificateOrPublicKey(corrupt)
    if gotErr == nil {
        t.Fatal("expected error for corrupt bytes, got nil")
    }
    if strings.Contains(gotErr.Error(), "asn1") || strings.Contains(gotErr.Error(), "structure") {
        t.Logf("GOOD: real parse error is propagated: %v", gotErr)
    } else {
        t.Logf("BUG CONFIRMED (Scenario B): real parse error swallowed, got generic: %q", gotErr.Error())
    }
}

// Scenario C: Valid ECDSA public key should succeed (regression guard).
func TestParseCertificateOrPublicKey_ValidECDSAKeySucceeds(t *testing.T) {
    ecKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
    pubDER, _ := x509.MarshalPKIXPublicKey(&ecKey.PublicKey)
    result, err := parseCertificateOrPublicKey(pubDER)
    if err != nil {
        t.Fatalf("unexpected error for valid ECDSA key: %v", err)
    }
    if len(result) == 0 {
        t.Fatal("expected non-empty DER bytes")
    }
}

// Scenario D: Valid ECDSA certificate should succeed (regression guard).
func TestParseCertificateOrPublicKey_ValidECDSACertSucceeds(t *testing.T) {
    ecKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
    template := &x509.Certificate{
        SerialNumber: big.NewInt(2),
        Subject:      pkix.Name{CommonName: "test-ecdsa"},
        NotBefore:    time.Now(),
        NotAfter:     time.Now().Add(time.Hour),
    }
    certDER, _ := x509.CreateCertificate(rand.Reader, template, template, &ecKey.PublicKey, ecKey)
    result, err := parseCertificateOrPublicKey(certDER)
    if err != nil {
        t.Fatalf("unexpected error for valid ECDSA cert: %v", err)
    }
    if len(result) == 0 {
        t.Fatal("expected non-empty DER bytes")
    }
}

Run with:

go test ./tools/fxconfig/internal/transaction -run TestParseCertificateOrPublicKey -v

---

Observed Output (confirmed)

Both bugs confirmed. Scenarios C and D pass cleanly, confirming the regression guards work and valid ECDSA inputs are unaffected.

---

Expected vs. Actual Errors

Input	Current error	Expected error
Valid RSA certificate	"no ECDSA public key in block"	"certificate uses RSA algorithm, expected ECDSA"
Corrupt DER bytes	"no ECDSA public key in block"	"failed to parse public key: asn1: structure error: ..."

---

Suggested Fix

func parseCertificateOrPublicKey(blockBytes []byte) ([]byte, error) {
    cert, err := x509.ParseCertificate(blockBytes)
    var publicKey any
    if err == nil {
        if cert.PublicKeyAlgorithm != x509.ECDSA {
            return nil, fmt.Errorf(
                "certificate uses %s algorithm, expected ECDSA",
                cert.PublicKeyAlgorithm,
            )
        }
        publicKey = cert.PublicKey
    } else {
        anyPublicKey, err2 := x509.ParsePKIXPublicKey(blockBytes)
        if err2 != nil {
            return nil, fmt.Errorf("failed to parse public key: %w", err2)
        }
        var ok bool
        publicKey, ok = anyPublicKey.(*ecdsa.PublicKey)
        if !ok {
            return nil, errors.New("public key is not an ECDSA public key")
        }
    }
    key, err := x509.MarshalPKIXPublicKey(publicKey)
    if err != nil {
        return nil, fmt.Errorf("marshalling public key failed: %w", err)
    }
    return key, nil
}

---

Impact

Users running:

fxconfig namespace create --threshold-policy <path>
fxconfig namespace update --threshold-policy <path>

…who supply a wrong-algorithm certificate or a corrupted PEM file currently receive only:
no ECDSA public key in pem file

There is no indication of whether the failure was an algorithm mismatch or a structural parse error, which significantly slows down debugging.

---

Environment

• Repo: hyperledger/fabric-x
• File: tools/fxconfig/internal/transaction/policy.go
• Functions: parseCertificateOrPublicKey, getPubKeyFromPemData, CreateThresholdPolicy
• Verified with: Go 1.23, branch bug/policy-parse-errors

[Prachi194agrawal]

I want to work on this issue. Please assign it to me, @mbrandenburger.