From a2d98c5789314e464fd63684565e02f2dfd078d4 Mon Sep 17 00:00:00 2001 From: Martin Hutchinson Date: Tue, 25 Jun 2024 09:46:37 +0100 Subject: [PATCH] Refactor issuance chain service (#1512) Primary goal is to have 2 separate implementations of the different services, instead of having both implementations inside the same class with conditional switching. This is realized by introducing a _direct_ chain service that performs the legacy implementation of storing chains directly inside the extra data in Trillian. The logic for the new feature is now in an _indirect_ chain service that requires the storage and cache that chains are stored in. --- trillian/ctfe/handlers.go | 23 +++-- trillian/ctfe/handlers_test.go | 2 +- trillian/ctfe/instance.go | 8 +- trillian/ctfe/services.go | 181 +++++++++++++++++---------------- trillian/ctfe/services_test.go | 4 +- 5 files changed, 112 insertions(+), 106 deletions(-) diff --git a/trillian/ctfe/handlers.go b/trillian/ctfe/handlers.go index 2ade820d3d..4a70fc257d 100644 --- a/trillian/ctfe/handlers.go +++ b/trillian/ctfe/handlers.go @@ -256,6 +256,11 @@ func NewCertValidationOpts(trustedRoots *x509util.PEMCertPool, currentTime time. return vOpts } +type leafChainBuilder interface { + BuildLogLeaf(ctx context.Context, chain []*x509.Certificate, logPrefix string, merkleLeaf *ct.MerkleTreeLeaf, isPrecert bool) (*trillian.LogLeaf, error) + FixLogLeaf(ctx context.Context, leaf *trillian.LogLeaf) error +} + // logInfo holds information for a specific log instance. type logInfo struct { // LogPrefix is a pre-formatted string identifying the log for diagnostics @@ -279,7 +284,7 @@ type logInfo struct { // sthGetter provides STHs for the log sthGetter STHGetter // issuanceChainService provides the issuance chain add and get operations - issuanceChainService *issuanceChainService + issuanceChainService leafChainBuilder } // newLogInfo creates a new instance of logInfo. @@ -288,7 +293,7 @@ func newLogInfo( validationOpts CertValidationOpts, signer crypto.Signer, timeSource util.TimeSource, - issuanceChainService *issuanceChainService, + issuanceChainService leafChainBuilder, ) *logInfo { vCfg := instanceOpts.Validated cfg := vCfg.Config @@ -388,6 +393,10 @@ func (li *logInfo) getSTH(ctx context.Context) (*ct.SignedTreeHead, error) { return sth, nil } +func (li *logInfo) buildLeaf(ctx context.Context, chain []*x509.Certificate, merkleLeaf *ct.MerkleTreeLeaf, isPrecert bool) (*trillian.LogLeaf, error) { + return li.issuanceChainService.BuildLogLeaf(ctx, chain, li.LogPrefix, merkleLeaf, isPrecert) +} + // ParseBodyAsJSONChain tries to extract cert-chain out of request. func ParseBodyAsJSONChain(r *http.Request) (ct.AddChainRequest, error) { body, err := io.ReadAll(r.Body) @@ -470,7 +479,7 @@ func addChainInternal(ctx context.Context, li *logInfo, w http.ResponseWriter, r if err != nil { return http.StatusBadRequest, fmt.Errorf("failed to build MerkleTreeLeaf: %s", err) } - leaf, err := li.issuanceChainService.BuildLogLeaf(ctx, chain, li.LogPrefix, merkleLeaf, isPrecert) + leaf, err := li.buildLeaf(ctx, chain, merkleLeaf, isPrecert) if err != nil { return http.StatusInternalServerError, err } @@ -949,14 +958,6 @@ func verifyAddChain(li *logInfo, req ct.AddChainRequest, expectingPrecert bool) return validPath, nil } -func extractRawCerts(chain []*x509.Certificate) []ct.ASN1Cert { - raw := make([]ct.ASN1Cert, len(chain)) - for i, cert := range chain { - raw[i] = ct.ASN1Cert{Data: cert.Raw} - } - return raw -} - // marshalAndWriteAddChainResponse is used by add-chain and add-pre-chain to create and write // the JSON response to the client func marshalAndWriteAddChainResponse(sct *ct.SignedCertificateTimestamp, signer crypto.Signer, w http.ResponseWriter) error { diff --git a/trillian/ctfe/handlers_test.go b/trillian/ctfe/handlers_test.go index ada5220cbd..f7db4796ab 100644 --- a/trillian/ctfe/handlers_test.go +++ b/trillian/ctfe/handlers_test.go @@ -165,7 +165,7 @@ func setupTest(t *testing.T, pemRoots []string, signer crypto.Signer) handlerTes cfg := &configpb.LogConfig{LogId: 0x42, Prefix: "test", IsMirror: false} vCfg := &ValidatedLogConfig{Config: cfg} iOpts := InstanceOptions{Validated: vCfg, Client: info.client, Deadline: time.Millisecond * 500, MetricFactory: monitoring.InertMetricFactory{}, RequestLog: new(DefaultRequestLog)} - info.li = newLogInfo(iOpts, vOpts, signer, fakeTimeSource, newIssuanceChainService(nil, nil)) + info.li = newLogInfo(iOpts, vOpts, signer, fakeTimeSource, &directIssuanceChainService{}) for _, pemRoot := range pemRoots { if !info.roots.AppendCertsFromPEM([]byte(pemRoot)) { diff --git a/trillian/ctfe/instance.go b/trillian/ctfe/instance.go index 6fe480ce03..e723aa36be 100644 --- a/trillian/ctfe/instance.go +++ b/trillian/ctfe/instance.go @@ -185,13 +185,17 @@ func setUpLogInfo(ctx context.Context, opts InstanceOptions) (*logInfo, error) { if err != nil { return nil, err } - // issuanceChainCache is nil if the cache related flags are not defined. + if issuanceChainStorage == nil { + return newLogInfo(opts, validationOpts, signer, new(util.SystemTimeSource), &directIssuanceChainService{}), nil + } + + // We are storing chains outside of Trillian, so set up cache and service. issuanceChainCache, err := cache.NewIssuanceChainCache(ctx, opts.CacheType, opts.CacheOption) if err != nil { return nil, err } - issuanceChainService := newIssuanceChainService(issuanceChainStorage, issuanceChainCache) + issuanceChainService := newIndirectIssuanceChainService(issuanceChainStorage, issuanceChainCache) logInfo := newLogInfo(opts, validationOpts, signer, new(util.SystemTimeSource), issuanceChainService) return logInfo, nil diff --git a/trillian/ctfe/services.go b/trillian/ctfe/services.go index 4643b14fb8..31393efce2 100644 --- a/trillian/ctfe/services.go +++ b/trillian/ctfe/services.go @@ -17,7 +17,6 @@ package ctfe import ( "context" "crypto/sha256" - "errors" "fmt" "github.com/google/certificate-transparency-go/asn1" @@ -32,127 +31,76 @@ import ( ct "github.com/google/certificate-transparency-go" ) -type issuanceChainService struct { - storage storage.IssuanceChainStorage - cache cache.IssuanceChainCache +// directIssuanceChainService does no special work, relying on the chains +// being serialized in the extraData directly. +type directIssuanceChainService struct { } -func newIssuanceChainService(s storage.IssuanceChainStorage, c cache.IssuanceChainCache) *issuanceChainService { - service := &issuanceChainService{ - storage: s, - cache: c, +func (s *directIssuanceChainService) BuildLogLeaf(ctx context.Context, chain []*x509.Certificate, logPrefix string, merkleLeaf *ct.MerkleTreeLeaf, isPrecert bool) (*trillian.LogLeaf, error) { + raw := extractRawCerts(chain) + // 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 service + return leaf, nil } -func (s *issuanceChainService) isCTFEStorageEnabled() bool { - return s.storage != nil +func (s *directIssuanceChainService) FixLogLeaf(ctx context.Context, leaf *trillian.LogLeaf) error { + return nil } -// GetByHash returns the issuance chain with hash as the input. -func (s *issuanceChainService) GetByHash(ctx context.Context, hash []byte) ([]byte, error) { - // Return err if CTFE storage backend is not enabled. - if !s.isCTFEStorageEnabled() { - return nil, errors.New("failed to GetByHash when storage is nil") - } - - // Return if found in cache. - chain, err := s.cache.Get(ctx, hash) - if chain != nil || err != nil { - return chain, err +func newIndirectIssuanceChainService(s storage.IssuanceChainStorage, c cache.IssuanceChainCache) *indirectIssuanceChainService { + if s == nil || c == nil { + panic("storage and cache are required") } - - // Find in storage if cache miss. - chain, err = s.storage.FindByKey(ctx, hash) - if err != nil { - return nil, err + service := &indirectIssuanceChainService{ + storage: s, + cache: c, } - // If there is any error from cache set, do not return the error because - // the chain is still available for read. - go func(ctx context.Context, hash, chain []byte) { - if err := s.cache.Set(ctx, hash, chain); err != nil { - klog.Errorf("failed to set hash and chain into cache: %v", err) - } - }(ctx, hash, chain) - - return chain, nil + return service } -// add adds the issuance chain into the storage and cache and returns the hash -// of the chain. -func (s *issuanceChainService) add(ctx context.Context, chain []byte) ([]byte, error) { - // Return err if CTFE storage backend is not enabled. - if !s.isCTFEStorageEnabled() { - return nil, errors.New("failed to Add when storage is nil") - } - - hash := issuanceChainHash(chain) - - if err := s.storage.Add(ctx, hash, chain); err != nil { - return nil, err - } - - // If there is any error from cache set, do not return the error because - // the chain is already stored. - go func(ctx context.Context, hash, chain []byte) { - if err := s.cache.Set(ctx, hash, chain); err != nil { - klog.Errorf("failed to set hash and chain into cache: %v", err) - } - }(ctx, hash, chain) - - return hash, nil +// indirectIssuanceChainService uses an external storage mechanism to store the chains. +// This feature allows for deduplication of chains, but requires more work to inject the +// chain data back inside the leaves. +type indirectIssuanceChainService struct { + storage storage.IssuanceChainStorage + cache cache.IssuanceChainCache } // BuildLogLeaf builds the MerkleTreeLeaf that gets sent to the backend, and make a trillian.LogLeaf for it. -func (s *issuanceChainService) BuildLogLeaf(ctx context.Context, chain []*x509.Certificate, logPrefix string, merkleLeaf *ct.MerkleTreeLeaf, isPrecert bool) (*trillian.LogLeaf, error) { +func (s *indirectIssuanceChainService) BuildLogLeaf(ctx context.Context, chain []*x509.Certificate, logPrefix string, merkleLeaf *ct.MerkleTreeLeaf, isPrecert bool) (*trillian.LogLeaf, error) { raw := extractRawCerts(chain) - // If CTFE storage is enabled for issuance chain, add the chain to storage - // and cache, and then build log leaf. If Trillian gRPC is enabled for - // issuance chain, build the log leaf. - if s.isCTFEStorageEnabled() { - issuanceChain, err := asn1.Marshal(raw[1:]) - if err != nil { - return nil, fmt.Errorf("failed to marshal issuance chain: %s", err) - } - hash, err := s.add(ctx, issuanceChain) - if err != nil { - 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 nil, fmt.Errorf("failed to build LogLeaf: %s", err) - } - return leaf, nil + // Add the chain to storage and cache, and then build log leaf + issuanceChain, err := asn1.Marshal(raw[1:]) + if err != nil { + return nil, fmt.Errorf("failed to marshal issuance chain: %s", err) } - - // Trillian gRPC - leaf, err := util.BuildLogLeaf(logPrefix, *merkleLeaf, 0, raw[0], raw[1:], isPrecert) + hash, err := s.add(ctx, issuanceChain) + if err != nil { + 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 nil, fmt.Errorf("failed to build LogLeaf: %s", err) } return leaf, nil - } // FixLogLeaf recreates and populates the LogLeaf.ExtraData if CTFE storage // backend is enabled and the type of LogLeaf.ExtraData contains any hash // (e.g. PrecertChainEntryHash, CertificateChainHash). -func (s *issuanceChainService) FixLogLeaf(ctx context.Context, leaf *trillian.LogLeaf) error { - // Skip if CTFE storage backend is not enabled. - if !s.isCTFEStorageEnabled() { - return nil - } - +func (s *indirectIssuanceChainService) FixLogLeaf(ctx context.Context, leaf *trillian.LogLeaf) error { // As the struct stored in leaf.ExtraData is unknown, the only way is to try to unmarshal with each possible struct. // Try to unmarshal with ct.PrecertChainEntryHash struct. var precertChainHash ct.PrecertChainEntryHash if rest, err := tls.Unmarshal(leaf.ExtraData, &precertChainHash); err == nil && len(rest) == 0 { var chain []ct.ASN1Cert if len(precertChainHash.IssuanceChainHash) > 0 { - chainBytes, err := s.GetByHash(ctx, precertChainHash.IssuanceChainHash) + chainBytes, err := s.getByHash(ctx, precertChainHash.IssuanceChainHash) if err != nil { return err } @@ -182,7 +130,7 @@ func (s *issuanceChainService) FixLogLeaf(ctx context.Context, leaf *trillian.Lo if rest, err := tls.Unmarshal(leaf.ExtraData, &certChainHash); err == nil && len(rest) == 0 { var entries []ct.ASN1Cert if len(certChainHash.IssuanceChainHash) > 0 { - chainBytes, err := s.GetByHash(ctx, certChainHash.IssuanceChainHash) + chainBytes, err := s.getByHash(ctx, certChainHash.IssuanceChainHash) if err != nil { return err } @@ -219,8 +167,61 @@ func (s *issuanceChainService) FixLogLeaf(ctx context.Context, leaf *trillian.Lo return fmt.Errorf("unknown extra data type in log leaf: %s", string(leaf.MerkleLeafHash)) } +// getByHash returns the issuance chain with hash as the input. +func (s *indirectIssuanceChainService) getByHash(ctx context.Context, hash []byte) ([]byte, error) { + // Return if found in cache. + chain, err := s.cache.Get(ctx, hash) + if chain != nil || err != nil { + return chain, err + } + + // Find in storage if cache miss. + chain, err = s.storage.FindByKey(ctx, hash) + if err != nil { + return nil, err + } + + // If there is any error from cache set, do not return the error because + // the chain is still available for read. + go func(ctx context.Context, hash, chain []byte) { + if err := s.cache.Set(ctx, hash, chain); err != nil { + klog.Errorf("failed to set hash and chain into cache: %v", err) + } + }(ctx, hash, chain) + + return chain, nil +} + +// add adds the issuance chain into the storage and cache and returns the hash +// of the chain. +func (s *indirectIssuanceChainService) add(ctx context.Context, chain []byte) ([]byte, error) { + hash := issuanceChainHash(chain) + + if err := s.storage.Add(ctx, hash, chain); err != nil { + return nil, err + } + + // If there is any error from cache set, do not return the error because + // the chain is already stored. + go func(ctx context.Context, hash, chain []byte) { + if err := s.cache.Set(ctx, hash, chain); err != nil { + klog.Errorf("failed to set hash and chain into cache: %v", err) + } + }(ctx, hash, chain) + + return hash, nil +} + // issuanceChainHash returns the SHA-256 hash of the chain. func issuanceChainHash(chain []byte) []byte { checksum := sha256.Sum256(chain) return checksum[:] } + +func extractRawCerts(chain []*x509.Certificate) []ct.ASN1Cert { + raw := make([]ct.ASN1Cert, len(chain)) + for i, cert := range chain { + raw[i] = ct.ASN1Cert{Data: cert.Raw} + } + return raw +} diff --git a/trillian/ctfe/services_test.go b/trillian/ctfe/services_test.go index 4082fc4bff..accbce54b5 100644 --- a/trillian/ctfe/services_test.go +++ b/trillian/ctfe/services_test.go @@ -36,7 +36,7 @@ func TestIssuanceChainServiceAddAndGet(t *testing.T) { ctx := context.Background() storage := &fakeIssuanceChainStorage{} cache := &fakeIssuanceChainCache{} - issuanceChainService := newIssuanceChainService(storage, cache) + issuanceChainService := newIndirectIssuanceChainService(storage, cache) for _, test := range tests { hash, err := issuanceChainService.add(ctx, test.chain) @@ -44,7 +44,7 @@ func TestIssuanceChainServiceAddAndGet(t *testing.T) { t.Errorf("IssuanceChainService.Add(): %v", err) } - got, err := issuanceChainService.GetByHash(ctx, hash) + got, err := issuanceChainService.getByHash(ctx, hash) if err != nil { t.Errorf("IssuanceChainService.GetByHash(): %v", err) }