Merge pull request #10201 from SgtCoDFish/signerrcheck

Add more error checks during the signing process

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.

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.