Fix a bug that KeyConfig.QualifyingData is ignored by tpm.NewKey...() (#477)

... when the algorithm and size are not set.

Author: JasonXJ

attest/application_key_test.go

@@ -30,6 +30,8 @@ import (
 	"encoding/asn1"
 	"math/big"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 func TestSimKeyCreateAndLoad(t *testing.T) {
@@ -678,3 +680,28 @@ func testKeyOpts(t *testing.T, tpm *TPM) {
 		})
 	}
 }
+
+func TestOptsSetDefault(t *testing.T) {
+	if optsSetDefault(nil) != defaultConfig {
+		t.Errorf("expecting optsSetDefault(nil) to return the defaultConfig")
+	}
+
+	normalOpts := KeyConfig{
+		Algorithm: RSA,
+		Size:      2048,
+	}
+	if optsSetDefault(&normalOpts) != &normalOpts {
+		t.Errorf("expecting optsSetDefault() to return the same pointer for normal options")
+	}
+
+	optsMissingAlgorithmAndSize := KeyConfig{
+		QualifyingData: []byte("hello"),
+	}
+	optsAfter := optsSetDefault(&optsMissingAlgorithmAndSize)
+	if optsAfter == &optsMissingAlgorithmAndSize {
+		t.Errorf("expecting optsSetDefault() to return a pointer to a copy")
+	}
+	if diff := cmp.Diff(optsAfter, &KeyConfig{Algorithm: ECDSA, Size: 256, QualifyingData: []byte("hello")}); diff != "" {
+		t.Errorf("optsSetDefault() mismatch (-want +got):\n%s", diff)
+	}
+}

attest/tpm.go

@@ -488,15 +488,25 @@ func (t *TPM) NewAK(opts *AKConfig) (*AK, error) {
 	return t.tpm.newAK(opts)
 }
 
-// NewKey creates an application key certified by the attestation key. If opts is nil
-// then DefaultConfig is used.
-func (t *TPM) NewKey(ak *AK, opts *KeyConfig) (*Key, error) {
+// Return a KeyConfig with default values if appropriate. It never modifies the
+// incoming pointer.
+func optsSetDefault(opts *KeyConfig) *KeyConfig {
 	if opts == nil {
-		opts = defaultConfig
+		return defaultConfig
 	}
 	if opts.Algorithm == "" && opts.Size == 0 {
-		opts = defaultConfig
+		optsCopy := *opts
+		optsCopy.Algorithm = defaultConfig.Algorithm
+		optsCopy.Size = defaultConfig.Size
+		return &optsCopy
 	}
+	return opts
+}
+
+// NewKey creates an application key certified by the attestation key. If opts is nil
+// then DefaultConfig is used.
+func (t *TPM) NewKey(ak *AK, opts *KeyConfig) (*Key, error) {
+	opts = optsSetDefault(opts)
 	return t.tpm.newKey(ak, opts)
 }
 
@@ -506,12 +516,7 @@ func (t *TPM) NewKey(ak *AK, opts *KeyConfig) (*Key, error) {
 // Thus it can be used in cases where the attestation key was not created
 // by go-attestation library. If opts is nil then DefaultConfig is used.
 func (t *TPM) NewKeyCertifiedByKey(akHandle tpmutil.Handle, akAlg Algorithm, opts *KeyConfig) (*Key, error) {
-	if opts == nil {
-		opts = defaultConfig
-	}
-	if opts.Algorithm == "" && opts.Size == 0 {
-		opts = defaultConfig
-	}
+	opts = optsSetDefault(opts)
 	ck := certifyingKey{handle: akHandle, alg: akAlg}
 	return t.tpm.newKeyCertifiedByKey(ck, opts)
 }