From f7f465c40043d253c3ff6d9bdb42a32d7da2d02e Mon Sep 17 00:00:00 2001
From: Joe Richey <joerichey@google.com>
Date: Wed, 9 Dec 2020 14:25:47 -0800
Subject: [PATCH] Move Alg checks to getSigningHashAlg

This makes it easier to add other signing operations going forward.

This also fixes a bug where we would segfault when using a signing key
with a Null SigScheme. There is now a test to check for this.

Signed-off-by: Joe Richey <joerichey@google.com>
---
 tpm2tools/signer.go      | 37 ++++++++++++++++++++++++-------------
 tpm2tools/signer_test.go | 18 ++++++++++++++++++
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/tpm2tools/signer.go b/tpm2tools/signer.go
index 23590c85..bcd10b72 100644
--- a/tpm2tools/signer.go
+++ b/tpm2tools/signer.go
@@ -88,29 +88,40 @@ func (k *Key) GetSigner() (crypto.Signer, error) {
 	if k.hasAttribute(tpm2.FlagRestricted) {
 		return nil, fmt.Errorf("restricted keys are not supported")
 	}
+	hashAlg, err := getSigningHashAlg(k)
+	if err != nil {
+		return nil, err
+	}
+	// For crypto.Signer, Go does the hashing. Make sure the hash is supported.
+	hash, err := hashAlg.Hash()
+	if err != nil {
+		return nil, err
+	}
+	return &tpmSigner{k, hash}, nil
+}
+
+func getSigningHashAlg(k *Key) (tpm2.Algorithm, error) {
 	if !k.hasAttribute(tpm2.FlagSign) {
-		return nil, fmt.Errorf("non-signing key used with GetSigner()")
+		return tpm2.AlgNull, fmt.Errorf("non-signing key used with signing operation")
 	}
 
 	var sigScheme *tpm2.SigScheme
-
 	switch k.pubArea.Type {
 	case tpm2.AlgRSA:
 		sigScheme = k.pubArea.RSAParameters.Sign
-		if sigScheme.Alg != tpm2.AlgRSAPSS && sigScheme.Alg != tpm2.AlgRSASSA {
-			return nil, fmt.Errorf("unsupported signing algorithm: %v", sigScheme.Alg)
-		}
 	case tpm2.AlgECC:
 		sigScheme = k.pubArea.ECCParameters.Sign
-		if sigScheme.Alg != tpm2.AlgECDSA {
-			return nil, fmt.Errorf("unsupported signing algorithm: %v", sigScheme.Alg)
-		}
 	default:
-		return nil, fmt.Errorf("unsupported key type: %v", k.pubArea.Type)
+		return tpm2.AlgNull, fmt.Errorf("unsupported key type: %v", k.pubArea.Type)
 	}
-	hash, err := sigScheme.Hash.Hash()
-	if err != nil {
-		return nil, err
+
+	if sigScheme == nil {
+		return tpm2.AlgNull, fmt.Errorf("unsupported null signing scheme")
+	}
+	switch sigScheme.Alg {
+	case tpm2.AlgRSAPSS, tpm2.AlgRSASSA, tpm2.AlgECDSA:
+		return sigScheme.Hash, nil
+	default:
+		return tpm2.AlgNull, fmt.Errorf("unsupported signing algorithm: %v", sigScheme.Alg)
 	}
-	return &tpmSigner{k, hash}, nil
 }
diff --git a/tpm2tools/signer_test.go b/tpm2tools/signer_test.go
index 278febfc..e6977f49 100644
--- a/tpm2tools/signer_test.go
+++ b/tpm2tools/signer_test.go
@@ -242,3 +242,21 @@ func TestFailSignPSS(t *testing.T) {
 		})
 	}
 }
+
+// Signing keys without a signature scheme are incompatible with GetSigner
+func TestFailGetSignerNullScheme(t *testing.T) {
+	template := templateSSA(tpm2.AlgSHA256)
+	template.RSAParameters.Sign = nil
+
+	rwc := internal.GetTPM(t)
+	defer CheckedClose(t, rwc)
+	key, err := NewKey(rwc, tpm2.HandleEndorsement, template)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer key.Close()
+
+	if _, err = key.GetSigner(); err == nil {
+		t.Error("expected failure when calling GetSigner")
+	}
+}