From 3522dab1cb845a6359c6bec32bceff82666b61b8 Mon Sep 17 00:00:00 2001 From: Nathan Smith Date: Fri, 28 Jan 2022 10:35:55 -0600 Subject: [PATCH 1/3] Refacter CT log client to use interface Signed-off-by: Nathan Smith --- cmd/app/serve.go | 2 +- pkg/api/ca.go | 4 ++-- pkg/ctl/{ctl.go => client.go} | 21 ++++------------- pkg/ctl/{ctl_test.go => client_test.go} | 2 +- pkg/ctl/errorresponse.go | 31 +++++++++++++++++++++++++ pkg/ctl/interface.go | 22 ++++++++++++++++++ 6 files changed, 61 insertions(+), 21 deletions(-) rename pkg/ctl/{ctl.go => client.go} (81%) rename pkg/ctl/{ctl_test.go => client_test.go} (98%) 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..0c4cef265 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -171,7 +171,7 @@ 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) } diff --git a/pkg/api/ca.go b/pkg/api/ca.go index c677a1770..905206e1e 100644 --- a/pkg/api/ca.go +++ b/pkg/api/ca.go @@ -54,14 +54,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) 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/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) +} From 0cf0977f8cc468bde22af2282f9296692310ec41 Mon Sep 17 00:00:00 2001 From: Nathan Smith Date: Thu, 27 Jan 2022 08:44:18 -0600 Subject: [PATCH 2/3] Add logging middleware for CTL client Signed-off-by: Nathan Smith --- pkg/ctl/logging.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 pkg/ctl/logging.go diff --git a/pkg/ctl/logging.go b/pkg/ctl/logging.go new file mode 100644 index 000000000..344adb607 --- /dev/null +++ b/pkg/ctl/logging.go @@ -0,0 +1,43 @@ +// 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 loggingClient struct { + next Client + logger *zap.SugaredLogger +} + +func (lc *loggingClient) 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 to a certificate transparenct log client +func WithLogging(next Client, logger *zap.SugaredLogger) Client { + return &loggingClient{next, logger} +} From 255e07a4a32d99b4f99a8aa3c59dce8e57474d57 Mon Sep 17 00:00:00 2001 From: Nathan Smith Date: Thu, 27 Jan 2022 09:08:00 -0600 Subject: [PATCH 3/3] 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 | 1 + pkg/api/ca.go | 3 - pkg/ctl/{logging.go => ctl_logging.go} | 9 +-- pkg/ctl/ctl_logging_test.go | 78 ++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 7 deletions(-) rename pkg/ctl/{logging.go => ctl_logging.go} (79%) create mode 100644 pkg/ctl/ctl_logging_test.go diff --git a/cmd/app/serve.go b/cmd/app/serve.go index 0c4cef265..a4867ea68 100644 --- a/cmd/app/serve.go +++ b/cmd/app/serve.go @@ -174,6 +174,7 @@ func runServeCmd(cmd *cobra.Command, args []string) { 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 905206e1e..51f178a36 100644 --- a/pkg/api/ca.go +++ b/pkg/api/ca.go @@ -174,7 +174,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 { @@ -186,8 +185,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/logging.go b/pkg/ctl/ctl_logging.go similarity index 79% rename from pkg/ctl/logging.go rename to pkg/ctl/ctl_logging.go index 344adb607..76f8e928d 100644 --- a/pkg/ctl/logging.go +++ b/pkg/ctl/ctl_logging.go @@ -20,12 +20,12 @@ import ( "go.uber.org/zap" ) -type loggingClient struct { +type ctlLoggingClient struct { next Client logger *zap.SugaredLogger } -func (lc *loggingClient) AddChain(csc *ca.CodeSigningCertificate) (*CertChainResponse, error) { +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 { @@ -37,7 +37,8 @@ func (lc *loggingClient) AddChain(csc *ca.CodeSigningCertificate) (*CertChainRes return resp, nil } -// WithLogging adds logging to a certificate transparenct log client +// 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 &loggingClient{next, logger} + 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") + }) + } +}