diff --git a/pkg/provenance/sign.go b/pkg/provenance/sign.go index 4fefb4ba0..c41f90c61 100644 --- a/pkg/provenance/sign.go +++ b/pkg/provenance/sign.go @@ -216,7 +216,7 @@ func (s *Signatory) ClearSign(chartpath string) (string, error) { b, err := messageBlock(chartpath) if err != nil { - return "", nil + return "", err } // Sign the buffer @@ -224,9 +224,24 @@ func (s *Signatory) ClearSign(chartpath string) (string, error) { if err != nil { return "", err } + _, err = io.Copy(w, b) - w.Close() - return out.String(), err + + if err != nil { + // NB: We intentionally don't call `w.Close()` here! `w.Close()` is the method which + // actually does the PGP signing, and therefore is the part which uses the private key. + // In other words, if we call Close here, there's a risk that there's an attempt to use the + // private key to sign garbage data (since we know that io.Copy failed, `w` won't contain + // anything useful). + return "", errors.Wrap(err, "failed to write to clearsign encoder") + } + + err = w.Close() + if err != nil { + return "", errors.Wrap(err, "failed to either sign or armor message block") + } + + return out.String(), nil } // Verify checks a signature and verifies that it is legit for a chart. diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index c63daebf4..93c169263 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -16,6 +16,9 @@ limitations under the License. package provenance import ( + "crypto" + "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -230,6 +233,36 @@ func TestClearSign(t *testing.T) { } } +// failSigner always fails to sign and returns an error +type failSigner struct{} + +func (s failSigner) Public() crypto.PublicKey { + return nil +} + +func (s failSigner) Sign(_ io.Reader, _ []byte, _ crypto.SignerOpts) ([]byte, error) { + return nil, fmt.Errorf("always fails") +} + +func TestClearSignError(t *testing.T) { + signer, err := NewFromFiles(testKeyfile, testPubfile) + if err != nil { + t.Fatal(err) + } + + // ensure that signing always fails + signer.Entity.PrivateKey.PrivateKey = failSigner{} + + sig, err := signer.ClearSign(testChartfile) + if err == nil { + t.Fatal("didn't get an error from ClearSign but expected one") + } + + if sig != "" { + t.Fatalf("expected an empty signature after failed ClearSign but got %q", sig) + } +} + func TestDecodeSignature(t *testing.T) { // Unlike other tests, this does a round-trip test, ensuring that a signature // generated by the library can also be verified by the library.