Fix Go potential bugs and maintainability (#1496)

* Fix returning statement copies `sync.Mutex` by value

* Add comment for exported `IssuanceChainCache` struct

* Remove unnecessary else usage

* Prefer using `any` instead of `interface{}`

* Add exported method comment for `BuildLogLeafWithChainHash`

* Add method comment for export `IssuanceChainStorage` struct

* Return `nil` instead of `&trillian.LogLeaf{}` when there is an err

trillian/ctfe/cache/noop/noop.go

@@ -17,6 +17,7 @@ package noop
 
 import "context"
 
+// IssuanceChainCache is a no-op implementation of the IssuanceChainCache interface.
 type IssuanceChainCache struct{}
 
 func (c *IssuanceChainCache) Get(_ context.Context, key []byte) ([]byte, error) {

trillian/ctfe/services.go

@@ -115,24 +115,26 @@ func (s *issuanceChainService) BuildLogLeaf(ctx context.Context, chain []*x509.C
 	if s.isCTFEStorageEnabled() {
 		issuanceChain, err := asn1.Marshal(raw[1:])
 		if err != nil {
-			return &trillian.LogLeaf{}, fmt.Errorf("failed to marshal issuance chain: %s", err)
+			return nil, fmt.Errorf("failed to marshal issuance chain: %s", err)
 		}
 		hash, err := s.add(ctx, issuanceChain)
 		if err != nil {
-			return &trillian.LogLeaf{}, fmt.Errorf("failed to add issuance chain into CTFE storage: %s", err)
+			return nil, fmt.Errorf("failed to add issuance chain into CTFE storage: %s", err)
 		}
 		leaf, err := util.BuildLogLeafWithChainHash(logPrefix, *merkleLeaf, 0, raw[0], hash, isPrecert)
 		if err != nil {
-			return &trillian.LogLeaf{}, fmt.Errorf("failed to build LogLeaf: %s", err)
+			return nil, fmt.Errorf("failed to build LogLeaf: %s", err)
 		}
-		return &leaf, nil
-	} else {
-		leaf, err := util.BuildLogLeaf(logPrefix, *merkleLeaf, 0, raw[0], raw[1:], isPrecert)
-		if err != nil {
-			return &trillian.LogLeaf{}, fmt.Errorf("failed to build LogLeaf: %s", err)
-		}
-		return &leaf, nil
+		return leaf, nil
+	}
+
+	// Trillian gRPC
+	leaf, err := util.BuildLogLeaf(logPrefix, *merkleLeaf, 0, raw[0], raw[1:], isPrecert)
+	if err != nil {
+		return nil, fmt.Errorf("failed to build LogLeaf: %s", err)
 	}
+	return leaf, nil
+
 }
 
 // FixLogLeaf recreates and populates the LogLeaf.ExtraData if CTFE storage

trillian/ctfe/storage/mysql/mysql.go

@@ -32,6 +32,7 @@ const (
 	insertIssuanceChainSQL      = "INSERT INTO IssuanceChain(IdentityHash, ChainValue) VALUES (?, ?)"
 )
 
+// IssuanceChainStorage is a MySQL implementation of the IssuanceChainStorage interface.
 type IssuanceChainStorage struct {
 	db *sql.DB
 }

trillian/ctfe/storage/storage.go

@@ -41,9 +41,9 @@ func NewIssuanceChainStorage(ctx context.Context, backend configpb.LogConfig_Iss
 	case configpb.LogConfig_ISSUANCE_CHAIN_STORAGE_BACKEND_CTFE:
 		if strings.HasPrefix(dbConn, "mysql") {
 			return mysql.NewIssuanceChainStorage(ctx, dbConn), nil
-		} else {
-			return nil, errors.New("failed to initialise IssuanceChainService due to unsupported driver in CTFE storage connection string")
 		}
+
+		return nil, errors.New("failed to initialise IssuanceChainService due to unsupported driver in CTFE storage connection string")
 	}
 
 	return nil, errors.New("unsupported issuance chain storage backend")

trillian/util/log_leaf.go

@@ -28,7 +28,7 @@ import (
 func BuildLogLeaf(logPrefix string,
 	merkleLeaf ct.MerkleTreeLeaf, leafIndex int64,
 	cert ct.ASN1Cert, chain []ct.ASN1Cert, isPrecert bool,
-) (trillian.LogLeaf, error) {
+) (*trillian.LogLeaf, error) {
 	return buildLogLeaf(logPrefix, merkleLeaf, leafIndex, cert, chain, nil, isPrecert)
 }
 
@@ -51,14 +51,16 @@ func ExtraDataForChain(cert ct.ASN1Cert, chain []ct.ASN1Cert, isPrecert bool) ([
 	return tls.Marshal(extra)
 }
 
-func BuildLogLeafWithChainHash(logPrefix string, merkleLeaf ct.MerkleTreeLeaf, leafIndex int64, cert ct.ASN1Cert, chainHash []byte, isPrecert bool) (trillian.LogLeaf, error) {
+// BuildLogLeafWithChainHash returns a Trillian LogLeaf structure for a
+// (pre-)cert and the chain of certificates leading it up to a known root.
+func BuildLogLeafWithChainHash(logPrefix string, merkleLeaf ct.MerkleTreeLeaf, leafIndex int64, cert ct.ASN1Cert, chainHash []byte, isPrecert bool) (*trillian.LogLeaf, error) {
 	return buildLogLeaf(logPrefix, merkleLeaf, leafIndex, cert, nil, chainHash, isPrecert)
 }
 
 // ExtraDataForChainHash creates the extra data associated with a log entry as
 // described in RFC6962 section 4.6 except the chain being replaced with its hash.
 func ExtraDataForChainHash(cert ct.ASN1Cert, chainHash []byte, isPrecert bool) ([]byte, error) {
-	var extra interface{}
+	var extra any
 
 	if isPrecert {
 		// For a pre-cert, the extra data is a TLS-encoded PrecertChainEntry.
@@ -80,11 +82,11 @@ func ExtraDataForChainHash(cert ct.ASN1Cert, chainHash []byte, isPrecert bool) (
 // buildLogLeaf builds the trillian.LogLeaf. The chainHash argument controls
 // whether ExtraDataForChain or ExtraDataForChainHash method will be called.
 // If chainHash is not nil, but neither is chain, then chain will be ignored.
-func buildLogLeaf(logPrefix string, merkleLeaf ct.MerkleTreeLeaf, leafIndex int64, cert ct.ASN1Cert, chain []ct.ASN1Cert, chainHash []byte, isPrecert bool) (trillian.LogLeaf, error) {
+func buildLogLeaf(logPrefix string, merkleLeaf ct.MerkleTreeLeaf, leafIndex int64, cert ct.ASN1Cert, chain []ct.ASN1Cert, chainHash []byte, isPrecert bool) (*trillian.LogLeaf, error) {
 	leafData, err := tls.Marshal(merkleLeaf)
 	if err != nil {
 		klog.Warningf("%s: Failed to serialize Merkle leaf: %v", logPrefix, err)
-		return trillian.LogLeaf{}, err
+		return nil, err
 	}
 
 	var extraData []byte
@@ -95,12 +97,12 @@ func buildLogLeaf(logPrefix string, merkleLeaf ct.MerkleTreeLeaf, leafIndex int6
 	}
 	if err != nil {
 		klog.Warningf("%s: Failed to serialize chain for ExtraData: %v", logPrefix, err)
-		return trillian.LogLeaf{}, err
+		return nil, err
 	}
 	// leafIDHash allows Trillian to detect duplicate entries, so this should be
 	// a hash over the cert data.
 	leafIDHash := sha256.Sum256(cert.Data)
-	return trillian.LogLeaf{
+	return &trillian.LogLeaf{
 		LeafValue:        leafData,
 		ExtraData:        extraData,
 		LeafIndex:        leafIndex,