From 3f1e17e060647e4b2af2e659e7c168411298b73f Mon Sep 17 00:00:00 2001 From: Nathan Smith <12156185+nsmith5@users.noreply.github.com> Date: Thu, 3 Feb 2022 23:43:24 -0800 Subject: [PATCH] Move CTL logging logic over to CTL package (#353) * Refacter CT log client to use interface Signed-off-by: Nathan Smith * Add logging middleware for CTL client Signed-off-by: Nathan Smith * Log in CTL client not API handler Removes CTL related logging from the API handler and pushes it over to the CTL client. Signed-off-by: Nathan Smith --- cmd/app/serve.go | 3 +- pkg/api/ca.go | 7 +-- pkg/ctl/{ctl.go => client.go} | 21 ++----- pkg/ctl/{ctl_test.go => client_test.go} | 2 +- pkg/ctl/ctl_logging.go | 44 ++++++++++++++ pkg/ctl/ctl_logging_test.go | 78 +++++++++++++++++++++++++ pkg/ctl/errorresponse.go | 31 ++++++++++ pkg/ctl/interface.go | 22 +++++++ 8 files changed, 184 insertions(+), 24 deletions(-) rename pkg/ctl/{ctl.go => client.go} (81%) rename pkg/ctl/{ctl_test.go => client_test.go} (98%) create mode 100644 pkg/ctl/ctl_logging.go create mode 100644 pkg/ctl/ctl_logging_test.go create mode 100644 pkg/ctl/errorresponse.go create mode 100644 pkg/ctl/interface.go diff --git a/cmd/app/serve.go b/cmd/app/serve.go index 22b86c0ce..a4867ea68 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -171,9 +171,10 @@ func runServeCmd(cmd *cobra.Command, args []string) { host, port := viper.GetString("host"), viper.GetString("port") log.Logger.Infof("%s:%s", host, port) - var ctClient *ctl.Client + var ctClient ctl.Client if logURL := viper.GetString("ct-log-url"); logURL != "" { ctClient = ctl.New(logURL) + ctClient = ctl.WithLogging(ctClient, log.Logger) } var handler http.Handler diff --git a/pkg/api/ca.go b/pkg/api/ca.go index 605106592..e86871d21 100644 --- a/pkg/api/ca.go +++ b/pkg/api/ca.go @@ -55,14 +55,14 @@ const ( ) type api struct { - ct *ctl.Client + ct ctl.Client ca certauth.CertificateAuthority *http.ServeMux } // New creates a new http.Handler for serving the Fulcio API. -func New(ct *ctl.Client, ca certauth.CertificateAuthority) http.Handler { +func New(ct ctl.Client, ca certauth.CertificateAuthority) http.Handler { var a api a.ServeMux = http.NewServeMux() a.HandleFunc(signingCertPath, a.signingCert) @@ -175,7 +175,6 @@ func (a *api) signingCert(w http.ResponseWriter, req *http.Request) { } // Submit to CTL - logger.Info("Submitting CTL inclusion for OIDC grant: ", subject.Value) if a.ct != nil { sct, err := a.ct.AddChain(csc) if err != nil { @@ -187,8 +186,6 @@ func (a *api) signingCert(w http.ResponseWriter, req *http.Request) { handleFulcioAPIError(w, req, http.StatusInternalServerError, err, failedToMarshalSCT) return } - logger.Info("CTL Submission Signature Received: ", sct.Signature) - logger.Info("CTL Submission ID Received: ", sct.ID) } else { logger.Info("Skipping CT log upload.") } diff --git a/pkg/ctl/ctl.go b/pkg/ctl/client.go similarity index 81% rename from pkg/ctl/ctl.go rename to pkg/ctl/client.go index 044259c92..52b23fa42 100644 --- a/pkg/ctl/ctl.go +++ b/pkg/ctl/client.go @@ -28,14 +28,14 @@ import ( const addChainPath = "ct/v1/add-chain" -type Client struct { +type client struct { c *http.Client url string } -func New(url string) *Client { +func New(url string) Client { c := &http.Client{Timeout: 30 * time.Second} - return &Client{ + return &client{ c: c, url: url, } @@ -53,20 +53,7 @@ type CertChainResponse struct { Signature string `json:"signature"` } -type ErrorResponse struct { - StatusCode int `json:"statusCode"` - ErrorCode string `json:"errorCode"` - Message string `json:"message"` -} - -func (err *ErrorResponse) Error() string { - if err.ErrorCode == "" { - return fmt.Sprintf("%d CT API error: %s", err.StatusCode, err.Message) - } - return fmt.Sprintf("%d (%s) CT API error: %s", err.StatusCode, err.ErrorCode, err.Message) -} - -func (c *Client) AddChain(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) { +func (c *client) AddChain(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) { chainjson := &certChain{Chain: []string{ base64.StdEncoding.EncodeToString(csc.FinalCertificate.Raw), }} diff --git a/pkg/ctl/ctl_test.go b/pkg/ctl/client_test.go similarity index 98% rename from pkg/ctl/ctl_test.go rename to pkg/ctl/client_test.go index 41a019779..262ac7549 100644 --- a/pkg/ctl/ctl_test.go +++ b/pkg/ctl/client_test.go @@ -47,7 +47,7 @@ func Test_AddChain(t *testing.T) { })) defer server.Close() - api := Client{server.Client(), server.URL} + api := New(server.URL) csc, _ := ca.CreateCSCFromPEM(nil, rootCert, clientCert) body, err := api.AddChain(csc) assert.NoError(t, err) diff --git a/pkg/ctl/ctl_logging.go b/pkg/ctl/ctl_logging.go new file mode 100644 index 000000000..76f8e928d --- /dev/null +++ b/pkg/ctl/ctl_logging.go @@ -0,0 +1,44 @@ +// Copyright 2021 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package ctl + +import ( + "github.com/sigstore/fulcio/pkg/ca" + "go.uber.org/zap" +) + +type ctlLoggingClient struct { + next Client + logger *zap.SugaredLogger +} + +func (lc *ctlLoggingClient) AddChain(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) { + lc.logger.Info("Submitting CTL inclusion for subject", csc.Subject.Value) + resp, err := lc.next.AddChain(csc) + if err != nil { + lc.logger.Error("Failed to submit certificate to CTL with error", err) + return nil, err + } + lc.logger.Info("CTL Submission Signature Received: ", resp.Signature) + lc.logger.Info("CTL Submission ID Received: ", resp.ID) + return resp, nil +} + +// WithLogging adds logging (in the writing helpful information to console +// sense) to a certificate transparenct log client +func WithLogging(next Client, logger *zap.SugaredLogger) Client { + return &ctlLoggingClient{next, logger} +} diff --git a/pkg/ctl/ctl_logging_test.go b/pkg/ctl/ctl_logging_test.go new file mode 100644 index 000000000..6f731c06f --- /dev/null +++ b/pkg/ctl/ctl_logging_test.go @@ -0,0 +1,78 @@ +// Copyright 2021 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ctl + +import ( + "errors" + "regexp" + "testing" + + "github.com/sigstore/fulcio/pkg/ca" + "github.com/sigstore/fulcio/pkg/challenges" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +type clientFunc func(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) + +func (f clientFunc) AddChain(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) { + return f(csc) +} + +func TestWithLogging(t *testing.T) { + tests := map[string]struct { + Client Client + ExpectedOutput *regexp.Regexp + }{ + "Error in client should be logged": { + clientFunc(func(*ca.CodeSigningCertificate) (*CertChainResponse, error) { + return nil, errors.New(`ctl: testing error`) + }), + regexp.MustCompile(`ctl: testing error`), + }, + "Success in client should log information": { + clientFunc(func(*ca.CodeSigningCertificate) (*CertChainResponse, error) { + return &CertChainResponse{}, nil + }), + regexp.MustCompile(`CTL Submission ID Received:`), + }, + } + + for test, data := range tests { + t.Run(test, func(t *testing.T) { + // Given + observedZapCore, observedLogs := observer.New(zap.InfoLevel) + observedLogger := zap.New(observedZapCore) + client := WithLogging(data.Client, observedLogger.Sugar()) + + csc := ca.CodeSigningCertificate{ + Subject: &challenges.ChallengeResult{}, + } + + // When + _, _ = client.AddChain(&csc) + + // Then + for _, entry := range observedLogs.All() { + if data.ExpectedOutput.MatchString(entry.Message) { + // We received expected output so the test passes + return + } + } + // If we got here we didn't match the expected output so test fails + t.Error("Didn't find expected output in logs") + }) + } +} diff --git a/pkg/ctl/errorresponse.go b/pkg/ctl/errorresponse.go new file mode 100644 index 000000000..a7c0f6463 --- /dev/null +++ b/pkg/ctl/errorresponse.go @@ -0,0 +1,31 @@ +// Copyright 2021 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package ctl + +import "fmt" + +type ErrorResponse struct { + StatusCode int `json:"statusCode"` + ErrorCode string `json:"errorCode"` + Message string `json:"message"` +} + +func (err *ErrorResponse) Error() string { + if err.ErrorCode == "" { + return fmt.Sprintf("%d CT API error: %s", err.StatusCode, err.Message) + } + return fmt.Sprintf("%d (%s) CT API error: %s", err.StatusCode, err.ErrorCode, err.Message) +} diff --git a/pkg/ctl/interface.go b/pkg/ctl/interface.go new file mode 100644 index 000000000..278d9fd04 --- /dev/null +++ b/pkg/ctl/interface.go @@ -0,0 +1,22 @@ +// Copyright 2021 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package ctl + +import "github.com/sigstore/fulcio/pkg/ca" + +type Client interface { + AddChain(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) +}