Repository: google/go-tpm
Pull request #120 (2019-07-24 22:33:33)

Use ExponentRaw and ModulusRaw
Similar to #118, this PR updates `RSAParams` to have `ExponentRaw` and `ModulusRaw` parameters, instead of the four `encodeDefaultExponentAsZero`, `Exponent`, `Modulus`, and `ModulusRaw` parameters.

This allows for simpler encoding/decoding as well as a more clear API. It makes a much clearer distinction between parameters are actually encoded, and the values the represent.

EDIT: For the RSA exponent specifically, `ExponentRaw` is essentially what `encodedExponent()` was before, and the `Exponent()` method is what the `Exponent` field was before.

Before, if the binary data had an Exponent of `0`, we set `encodeDefaultExponentAsZero` to `true` and `Exponent` to `defaultRSAExponent` when decoding. Now, `ExponentRaw` just stays as 0, there is no `encodeDefaultExponentAsZero`, and the `Exponent()` method returns `defaultRSAExponent`. Reencoding is then also straighforward and correct, as verified by the tests.

Corresponding tests and examples are also updated. 

josephlr: > Looks good, tpm2 tests pass?

Yup, with and without real hardware.

Patch:
diff --git a/examples/tpm2-ekcert/main.go b/examples/tpm2-ekcert/main.go
index e122f7c..1fca03d 100644
--- a/examples/tpm2-ekcert/main.go
+++ b/examples/tpm2-ekcert/main.go
@@ -47,7 +47,6 @@ var (
 				Mode:    tpm2.AlgCFB,
 			},
 			KeyBits:    2048,
-			Exponent:   0,
 			ModulusRaw: make([]byte, 256),
 		},
 	}
diff --git a/examples/tpm2-seal-unseal/main.go b/examples/tpm2-seal-unseal/main.go
index 4318bf7..0cbdad6 100644
--- a/examples/tpm2-seal-unseal/main.go
+++ b/examples/tpm2-seal-unseal/main.go
@@ -44,7 +44,6 @@ var (
 				Mode:    tpm2.AlgCFB,
 			},
 			KeyBits:    2048,
-			Exponent:   0,
 			ModulusRaw: make([]byte, 256),
 		},
 	}
diff --git a/tpm2/structures.go b/tpm2/structures.go
index c285aec..941cc02 100644
--- a/tpm2/structures.go
+++ b/tpm2/structures.go
@@ -101,7 +101,7 @@ func (p Public) Key() (crypto.PublicKey, error) {
 	case AlgRSA:
 		// Endianness of big.Int.Bytes/SetBytes and modulus in the TPM is the same
 		// (big-endian).
-		pubKey = &rsa.PublicKey{N: p.RSAParameters.Modulus, E: int(p.RSAParameters.Exponent)}
+		pubKey = &rsa.PublicKey{N: p.RSAParameters.Modulus(), E: int(p.RSAParameters.Exponent())}
 	case AlgECC:
 		curve, ok := toGoCurve[p.ECCParameters.CurveID]
 		if !ok {
@@ -192,35 +192,35 @@ func DecodePublic(buf []byte) (Public, error) {
 //
 // Symmetric and Sign may be nil, depending on key Attributes in Public.
 //
-// One of Modulus and ModulusRaw must always be non-nil. Modulus takes
-// precedence. ModulusRaw is used for key templates where the field named
-// "unique" must be a byte array of all zeroes.
+// ExponentRaw and ModulusRaw are the actual data encoded in the template, which
+// is useful for templates that differ in zero-padding, for example.
 type RSAParams struct {
-	Symmetric *SymScheme
-	Sign      *SigScheme
-	KeyBits   uint16
-	// The default Exponent (65537) has two representations; the
-	// 0 value, and the value 65537.
-	// If encodeDefaultExponentAsZero is set, an exponent of 65537
-	// will be encoded as zero. This is necessary to produce an identical
-	// encoded bitstream, so Name digest calculations will be correct.
-	encodeDefaultExponentAsZero bool
-	Exponent                    uint32
-	ModulusRaw                  tpmutil.U16Bytes
-	Modulus                     *big.Int
+	Symmetric   *SymScheme
+	Sign        *SigScheme
+	KeyBits     uint16
+	ExponentRaw uint32
+	ModulusRaw  tpmutil.U16Bytes
 }
 
-func (p *RSAParams) encodedExponent() uint32 {
-	if p.encodeDefaultExponentAsZero && p.Exponent == defaultRSAExponent {
-		return 0
+// Exponent returns the RSA exponent value represented by ExponentRaw, handling
+// the fact that an exponent of 0 represents a value of 65537 (2^16 + 1).
+func (p *RSAParams) Exponent() uint32 {
+	if p.ExponentRaw == 0 {
+		return defaultRSAExponent
 	}
-	return p.Exponent
+	return p.ExponentRaw
+}
+
+// Modulus returns the RSA modulus value represented by ModulusRaw, handling the
+// fact that the same modulus value can have multiple different representations.
+func (p *RSAParams) Modulus() *big.Int {
+	return new(big.Int).SetBytes(p.ModulusRaw)
 }
 
 func (p *RSAParams) matchesTemplate(t *RSAParams) bool {
 	return reflect.DeepEqual(p.Symmetric, t.Symmetric) &&
 		reflect.DeepEqual(p.Sign, t.Sign) &&
-		p.KeyBits == t.KeyBits && p.encodedExponent() == t.encodedExponent()
+		p.KeyBits == t.KeyBits && p.ExponentRaw == t.ExponentRaw
 }
 
 func (p *RSAParams) encode() ([]byte, error) {
@@ -235,27 +235,11 @@ func (p *RSAParams) encode() ([]byte, error) {
 	if err != nil {
 		return nil, fmt.Errorf("encoding Sign: %v", err)
 	}
-	rest, err := tpmutil.Pack(p.KeyBits, p.encodedExponent())
-	if err != nil {
-		return nil, fmt.Errorf("encoding KeyBits, Exponent: %v", err)
-	}
-
-	if p.Modulus == nil && len(p.ModulusRaw) == 0 {
-		return nil, errors.New("RSAParams.Modulus or RSAParams.ModulusRaw must be set")
-	}
-	if p.Modulus != nil && len(p.ModulusRaw) > 0 {
-		return nil, errors.New("both RSAParams.Modulus and RSAParams.ModulusRaw can't be set")
-	}
-	mod := p.ModulusRaw
-	if p.Modulus != nil {
-		mod = tpmutil.U16Bytes(p.Modulus.Bytes())
-	}
-	unique, err := tpmutil.Pack(mod)
+	rest, err := tpmutil.Pack(p.KeyBits, p.ExponentRaw, p.ModulusRaw)
 	if err != nil {
-		return nil, fmt.Errorf("encoding Modulus: %v", err)
+		return nil, fmt.Errorf("encoding KeyBits, Exponent, Modulus: %v", err)
 	}
-
-	return concat(sym, sig, rest, unique)
+	return concat(sym, sig, rest)
 }
 
 func decodeRSAParams(in *bytes.Buffer) (*RSAParams, error) {
@@ -268,15 +252,9 @@ func decodeRSAParams(in *bytes.Buffer) (*RSAParams, error) {
 	if params.Sign, err = decodeSigScheme(in); err != nil {
 		return nil, fmt.Errorf("decoding Sign: %v", err)
 	}
-	var modBytes tpmutil.U16Bytes
-	if err := tpmutil.UnpackBuf(in, &params.KeyBits, &params.Exponent, &modBytes); err != nil {
+	if err := tpmutil.UnpackBuf(in, &params.KeyBits, &params.ExponentRaw, &params.ModulusRaw); err != nil {
 		return nil, fmt.Errorf("decoding KeyBits, Exponent, Modulus: %v", err)
 	}
-	if params.Exponent == 0 {
-		params.encodeDefaultExponentAsZero = true
-		params.Exponent = defaultRSAExponent
-	}
-	params.Modulus = new(big.Int).SetBytes(modBytes)
 	return &params, nil
 }
 
@@ -296,10 +274,12 @@ type ECPoint struct {
 	XRaw, YRaw tpmutil.U16Bytes
 }
 
+// X returns the X Point value reprsented by XRaw.
 func (p ECPoint) X() *big.Int {
 	return new(big.Int).SetBytes(p.XRaw)
 }
 
+// Y returns the Y Point value reprsented by YRaw.
 func (p ECPoint) Y() *big.Int {
 	return new(big.Int).SetBytes(p.YRaw)
 }


Test patch:
diff --git a/tpm2/benchmark_test.go b/tpm2/benchmark_test.go
index 4109490..a85a518 100644
--- a/tpm2/benchmark_test.go
+++ b/tpm2/benchmark_test.go
@@ -16,7 +16,6 @@ package tpm2
 
 import (
 	"crypto/sha256"
-	"math/big"
 	"testing"
 )
 
@@ -34,8 +33,7 @@ func BenchmarkRSA2048Signing(b *testing.B) {
 				Alg:  AlgRSASSA,
 				Hash: AlgSHA256,
 			},
-			KeyBits: uint16(2048),
-			Modulus: big.NewInt(0),
+			KeyBits: 2048,
 		},
 	}
 
diff --git a/tpm2/encoding_test.go b/tpm2/encoding_test.go
index 52c5410..9b72183 100644
--- a/tpm2/encoding_test.go
+++ b/tpm2/encoding_test.go
@@ -20,7 +20,6 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"encoding/hex"
-	"math/big"
 	"reflect"
 	"testing"
 
@@ -135,16 +134,15 @@ func TestEncodeCreate(t *testing.T) {
 	params := Public{
 		Type:       AlgRSA,
 		NameAlg:    AlgSHA1,
-		Attributes: 0x00030072,
+		Attributes: FlagStorageDefault,
 		RSAParameters: &RSAParams{
 			Symmetric: &SymScheme{
 				Alg:     AlgAES,
 				KeyBits: 128,
 				Mode:    AlgCFB,
 			},
-			KeyBits:  1024,
-			Exponent: uint32(0x00010001),
-			Modulus:  big.NewInt(0),
+			KeyBits:     1024,
+			ExponentRaw: defaultRSAExponent,
 		},
 	}
 	auth := AuthCommand{Session: HandlePasswordSession, Attributes: AttrContinueSession}
diff --git a/tpm2/tpm2_test.go b/tpm2/tpm2_test.go
index 757d2a5..6c30bd4 100644
--- a/tpm2/tpm2_test.go
+++ b/tpm2/tpm2_test.go
@@ -24,9 +24,7 @@ import (
 	"crypto/sha1"
 	"crypto/sha256"
 	"flag"
-	"fmt"
 	"io"
-	"math/big"
 	"reflect"
 	"strings"
 	"testing"
@@ -85,16 +83,15 @@ var (
 	defaultKeyParams = Public{
 		Type:       AlgRSA,
 		NameAlg:    AlgSHA1,
-		Attributes: 0x00030072,
+		Attributes: FlagStorageDefault,
 		RSAParameters: &RSAParams{
 			Symmetric: &SymScheme{
 				Alg:     AlgAES,
 				KeyBits: 128,
 				Mode:    AlgCFB,
 			},
-			KeyBits:  2048,
-			Exponent: uint32(0x00010001),
-			Modulus:  big.NewInt(0),
+			KeyBits:     2048,
+			ExponentRaw: defaultRSAExponent,
 		},
 	}
 	defaultPassword = "\x01\x02\x03\x04"
@@ -438,9 +435,9 @@ func TestLoadExternalPublicKey(t *testing.T) {
 					Alg:  AlgRSASSA,
 					Hash: AlgSHA1,
 				},
-				KeyBits:  2048,
-				Exponent: uint32(pk.PublicKey.E),
-				Modulus:  pk.PublicKey.N,
+				KeyBits:     2048,
+				ExponentRaw: uint32(pk.PublicKey.E),
+				ModulusRaw:  pk.PublicKey.N.Bytes(),
 			},
 		}
 		private := Private{
@@ -490,7 +487,6 @@ func TestCertify(t *testing.T) {
 				Hash: AlgSHA256,
 			},
 			KeyBits: 2048,
-			Modulus: big.NewInt(0),
 		},
 	}
 	signerHandle, signerPub, err := CreatePrimary(rw, HandleOwner, pcrSelection7, emptyPassword, defaultPassword, params)
@@ -533,7 +529,7 @@ func TestCertify(t *testing.T) {
 				KeyBits: 2048,
 				// Note: we don't include Exponent because CreatePrimary also
 				// returns Public without it.
-				Modulus: subjectPub.(*rsa.PublicKey).N,
+				ModulusRaw: subjectPub.(*rsa.PublicKey).N.Bytes(),
 			},
 		}
 		matches, err := ad.AttestedCertifyInfo.Name.MatchesPublic(params)
@@ -560,7 +556,6 @@ func TestCertifyExternalKey(t *testing.T) {
 				Hash: AlgSHA256,
 			},
 			KeyBits: 2048,
-			Modulus: big.NewInt(0),
 		},
 	}
 	signerHandle, signerPub, err := CreatePrimary(rw, HandleOwner, pcrSelection7, emptyPassword, defaultPassword, params)
@@ -602,9 +597,9 @@ func TestCertifyExternalKey(t *testing.T) {
 					Alg:  AlgRSASSA,
 					Hash: AlgSHA1,
 				},
-				KeyBits:  2048,
-				Exponent: uint32(pk.PublicKey.E),
-				Modulus:  pk.PublicKey.N,
+				KeyBits:     2048,
+				ExponentRaw: uint32(pk.PublicKey.E),
+				ModulusRaw:  pk.PublicKey.N.Bytes(),
 			},
 		}
 		private := Private{
@@ -696,7 +691,6 @@ func TestSign(t *testing.T) {
 					Hash: AlgSHA256,
 				},
 				KeyBits: 2048,
-				Modulus: big.NewInt(0),
 			},
 		})
 	})
@@ -711,7 +705,6 @@ func TestSign(t *testing.T) {
 					Hash: AlgSHA256,
 				},
 				KeyBits: 2048,
-				Modulus: big.NewInt(0),
 			},
 		})
 	})
@@ -897,29 +890,24 @@ func TestEncodeDecodePublicDefaultRSAExponent(t *testing.T) {
 				Alg:  AlgRSASSA,
 				Hash: AlgSHA1,
 			},
-			KeyBits:  2048,
-			Exponent: defaultRSAExponent,
-			Modulus:  new(big.Int).SetBytes([]byte{1, 2, 3, 4, 7, 8, 9, 9}),
+			KeyBits:     2048,
+			ExponentRaw: defaultRSAExponent,
+			ModulusRaw:  []byte{1, 2, 3, 4, 7, 8, 9, 9},
 		},
 	}
 
-	for _, encodeAsZero := range []bool{true, false} {
-		t.Run(fmt.Sprintf("encodeDefaultExponentAsZero = %v", encodeAsZero), func(t *testing.T) {
-			p.RSAParameters.encodeDefaultExponentAsZero = encodeAsZero
-			e, err := p.Encode()
-			if err != nil {
-				t.Fatalf("Public{%+v}.Encode() returned error: %v", p, err)
-			}
-			d, err := DecodePublic(e)
-			if err != nil {
-				t.Fatalf("DecodePublic(%v) returned error: %v", e, err)
-			}
-			if !reflect.DeepEqual(p, d) {
-				t.Errorf("RSA TPMT_PUBLIC with default exponent changed after being encoded+decoded")
-				t.Logf("\tGot:  %+v", d)
-				t.Logf("\tWant: %+v", p)
-			}
-		})
+	e, err := p.Encode()
+	if err != nil {
+		t.Fatalf("Public{%+v}.Encode() returned error: %v", p, err)
+	}
+	d, err := DecodePublic(e)
+	if err != nil {
+		t.Fatalf("DecodePublic(%v) returned error: %v", e, err)
+	}
+	if !reflect.DeepEqual(p, d) {
+		t.Errorf("RSA TPMT_PUBLIC with default exponent changed after being encoded+decoded")
+		t.Logf("\tGot:  %+v", d)
+		t.Logf("\tWant: %+v", p)
 	}
 }
 
@@ -968,7 +956,6 @@ func TestCreateAndCertifyCreation(t *testing.T) {
 				Hash: AlgSHA256,
 			},
 			KeyBits: 2048,
-			Modulus: big.NewInt(0),
 		},
 	}
 	keyHandle, pub, _, creationHash, tix, _, err := CreatePrimaryEx(rw, HandleEndorsement, pcrSelection7, emptyPassword, emptyPassword, params)
@@ -1000,7 +987,7 @@ func TestCreateAndCertifyCreation(t *testing.T) {
 		t.Logf("Name: %v", att.AttestedCreationInfo.Name)
 		t.Logf("Public: %v", p)
 	}
-	rsaPub := rsa.PublicKey{E: int(p.RSAParameters.Exponent), N: p.RSAParameters.Modulus}
+	rsaPub := rsa.PublicKey{E: int(p.RSAParameters.Exponent()), N: p.RSAParameters.Modulus()}
 	hsh := crypto.SHA256.New()
 	hsh.Write(attestation)
 	if err := rsa.VerifyPKCS1v15(&rsaPub, crypto.SHA256, hsh.Sum(nil), signature); err != nil {
@@ -1090,7 +1077,6 @@ func TestQuote(t *testing.T) {
 				Hash: AlgSHA256,
 			},
 			KeyBits: 2048,
-			Modulus: big.NewInt(0),
 		},
 	}
 	keyHandle, pub, _, _, _, _, err := CreatePrimaryEx(rw, HandleEndorsement, pcrSelection7, emptyPassword, emptyPassword, params)
@@ -1118,7 +1104,7 @@ func TestQuote(t *testing.T) {
 	if err != nil {
 		t.Fatalf("DecodePublic failed: %v", err)
 	}
-	rsaPub := rsa.PublicKey{E: int(p.RSAParameters.Exponent), N: p.RSAParameters.Modulus}
+	rsaPub := rsa.PublicKey{E: int(p.RSAParameters.Exponent()), N: p.RSAParameters.Modulus()}
 	hsh := crypto.SHA256.New()
 	hsh.Write(attestation)
 	if err := rsa.VerifyPKCS1v15(&rsaPub, crypto.SHA256, hsh.Sum(nil), signature.RSA.Signature); err != nil {
@@ -1168,9 +1154,9 @@ func TestReadPublicKey(t *testing.T) {
 					Alg:  AlgRSASSA,
 					Hash: AlgSHA1,
 				},
-				KeyBits:  2048,
-				Exponent: uint32(pk.PublicKey.E),
-				Modulus:  pk.PublicKey.N,
+				KeyBits:     2048,
+				ExponentRaw: uint32(pk.PublicKey.E),
+				ModulusRaw:  pk.PublicKey.N.Bytes(),
 			},
 		}
 		private := Private{
@@ -1221,7 +1207,6 @@ func TestEncryptDecrypt(t *testing.T) {
 				Mode:    AlgCFB,
 			},
 			KeyBits: 2048,
-			Modulus: big.NewInt(0),
 		},
 	})
 	if err != nil {
@@ -1282,7 +1267,6 @@ func TestRSAEncryptDecrypt(t *testing.T) {
 				Hash: AlgNull,
 			},
 			KeyBits: 2048,
-			Modulus: big.NewInt(0),
 		},
 	})
 	if err != nil {
@@ -1358,8 +1342,8 @@ func TestCreatePrimaryRawTemplate(t *testing.T) {
 	if gotKeySize != wantKeySize {
 		t.Errorf("got key size %v, want %v", gotKeySize, wantKeySize)
 	}
-	if pubRSA.E != int(defaultKeyParams.RSAParameters.Exponent) {
-		t.Errorf("got key exponent %v, want %v", pubRSA.E, defaultKeyParams.RSAParameters.Exponent)
+	if pubRSA.E != int(defaultKeyParams.RSAParameters.Exponent()) {
+		t.Errorf("got key exponent %v, want %v", pubRSA.E, defaultKeyParams.RSAParameters.Exponent())
 	}
 }
 
@@ -1383,11 +1367,10 @@ func TestMatchesTemplate(t *testing.T) {
 							Hash: AlgSHA256,
 						},
 						KeyBits: 2048,
-						Modulus: big.NewInt(0),
 					},
 				}
 			},
-			func(pub *Public) { pub.RSAParameters.Modulus = big.NewInt(15) },
+			func(pub *Public) { pub.RSAParameters.ModulusRaw = make([]byte, 256) },
 			func(pub *Public) { pub.RSAParameters.KeyBits = 1024 },
 		},
 		{