Repository: google/go-tpm
Pull request #118 (2019-07-23 22:32:17)

Update ECPoint type
Per https://github.com/google/go-tpm/issues/115
Allow the ECCParams to supply buffers as X and Y for the Point field, in order to comply with the Default ECC EK Public Area Template.

When needed, the buffer will be converted to Int.

This change is not backward compatible, existing tests/code using ECPoint will need to be updated.

josephlr: Also, please update the PR description to reflect the new changes
googlebot: We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors.  If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA ([login here](https://cla.developers.google.com/) to double check)?  If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fgo-tpm%2Fpull%2F118) for more info**.

<!-- need_author_cla -->
googlebot: CLAs look good, thanks!

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fgo-tpm%2Fpull%2F118) for more info**.

<!-- ok -->
josephlr: @awly, this is a backwards incompatible change, but makes dealing with encoding/decoding issues much easier.

Would you want to do the same thing for `RSAParams.Modulus` (in a followup PR)? The main downside is that RSA keys are more common, so it might break a little more code, but I think we should do it.
awly: SGTM, we always assume that importers will lock the version of this library.
Breaking API isn't a problem.

Patch:
diff --git a/tpm2/structures.go b/tpm2/structures.go
index 9a6e1cc..0a080fd 100644
--- a/tpm2/structures.go
+++ b/tpm2/structures.go
@@ -107,8 +107,8 @@ func (p Public) Key() (crypto.PublicKey, error) {
 			return nil, fmt.Errorf("can't map TPM EC curve ID 0x%x to Go elliptic.Curve value", p.ECCParameters.CurveID)
 		}
 		pubKey = &ecdsa.PublicKey{
-			X:     p.ECCParameters.Point.X,
-			Y:     p.ECCParameters.Point.Y,
+			X:     p.ECCParameters.Point.X(),
+			Y:     p.ECCParameters.Point.Y(),
 			Curve: curve,
 		}
 	default:
@@ -256,23 +256,17 @@ type ECCParams struct {
 	Point     ECPoint
 }
 
-// ECPoint represents a ECC coordinates for a point.
+// ECPoint represents a ECC coordinates for a point using byte buffers.
 type ECPoint struct {
-	X, Y *big.Int
+	XRaw, YRaw tpmutil.U16Bytes
 }
 
-func (p *ECPoint) x() *big.Int {
-	if p == nil || p.X == nil {
-		return big.NewInt(0)
-	}
-	return p.X
+func (p ECPoint) X() *big.Int {
+	return new(big.Int).SetBytes(p.XRaw)
 }
 
-func (p *ECPoint) y() *big.Int {
-	if p == nil || p.Y == nil {
-		return big.NewInt(0)
-	}
-	return p.Y
+func (p ECPoint) Y() *big.Int {
+	return new(big.Int).SetBytes(p.YRaw)
 }
 
 func (p *ECCParams) encode() ([]byte, error) {
@@ -295,8 +289,7 @@ func (p *ECCParams) encode() ([]byte, error) {
 	if err != nil {
 		return nil, fmt.Errorf("encoding KDF: %v", err)
 	}
-	x, y := p.Point.x().Bytes(), p.Point.y().Bytes()
-	point, err := tpmutil.Pack(tpmutil.U16Bytes(x), tpmutil.U16Bytes(y))
+	point, err := tpmutil.Pack(p.Point.XRaw, p.Point.YRaw)
 	if err != nil {
 		return nil, fmt.Errorf("encoding Point: %v", err)
 	}
@@ -319,12 +312,9 @@ func decodeECCParams(in *bytes.Buffer) (*ECCParams, error) {
 	if params.KDF, err = decodeKDFScheme(in); err != nil {
 		return nil, fmt.Errorf("decoding KDF: %v", err)
 	}
-	var x, y tpmutil.U16Bytes
-	if err := tpmutil.UnpackBuf(in, &x, &y); err != nil {
+	if err := tpmutil.UnpackBuf(in, &params.Point.XRaw, &params.Point.YRaw); err != nil {
 		return nil, fmt.Errorf("decoding Point: %v", err)
 	}
-	params.Point.X = new(big.Int).SetBytes(x)
-	params.Point.Y = new(big.Int).SetBytes(y)
 	return &params, nil
 }
 


Test patch:
diff --git a/tpm2/encoding_test.go b/tpm2/encoding_test.go
index 3dc6bba..52c5410 100644
--- a/tpm2/encoding_test.go
+++ b/tpm2/encoding_test.go
@@ -373,7 +373,7 @@ func TestECCParamsEncodeDecode(t *testing.T) {
 			Hash: AlgSHA1,
 		},
 		CurveID: CurveNISTP256,
-		Point:   ECPoint{X: pk.PublicKey.X, Y: pk.PublicKey.Y},
+		Point:   ECPoint{XRaw: pk.PublicKey.X.Bytes(), YRaw: pk.PublicKey.Y.Bytes()},
 	}
 
 	buf, err := params.encode()
@@ -385,6 +385,19 @@ func TestECCParamsEncodeDecode(t *testing.T) {
 		t.Fatalf("decodeECCParams: %v", err)
 	}
 
+	if params.Point.X().Cmp(pk.PublicKey.X) != 0 {
+		t.Fatalf("the deserialized X(big.Int) from ECCParams didn't match the X(Int) generated by ecdsa. got: %+v\nwant: %+v", params.Point.X(), pk.PublicKey.X)
+	}
+	if params.Point.Y().Cmp(pk.PublicKey.Y) != 0 {
+		t.Fatalf("the deserialized Y(big.Int) from ECCParams didn't match the Y(Int) generated by ecdsa. got: %+v\nwant: %+v", params.Point.Y(), pk.PublicKey.Y)
+	}
+	if params.Point.X().Cmp(got.Point.X()) != 0 {
+		t.Fatalf("the deserialized X(big.Int) from ECCParams after encode/decode didn't match the X(big.Int) before. got: %+v\nwant: %+v", got.Point.X(), params.Point.X())
+	}
+	if params.Point.Y().Cmp(got.Point.Y()) != 0 {
+		t.Fatalf("the deserialized Y(big.Int) from ECCParams after encode/decode didn't match the Y(big.Int) before. got: %+v\nwant: %+v", got.Point.Y(), params.Point.Y())
+	}
+
 	if !reflect.DeepEqual(got, params) {
 		t.Fatalf("got: %+v\nwant: %+v", got, params)
 	}
diff --git a/tpm2/tpm2_test.go b/tpm2/tpm2_test.go
index 4bf0981..5ca14f2 100644
--- a/tpm2/tpm2_test.go
+++ b/tpm2/tpm2_test.go
@@ -465,7 +465,7 @@ func TestLoadExternalPublicKey(t *testing.T) {
 					Hash: AlgSHA1,
 				},
 				CurveID: CurveNISTP256,
-				Point:   ECPoint{X: pk.PublicKey.X, Y: pk.PublicKey.Y},
+				Point:   ECPoint{XRaw: pk.PublicKey.X.Bytes(), YRaw: pk.PublicKey.Y.Bytes()},
 			},
 		}
 		private := Private{
@@ -629,7 +629,7 @@ func TestCertifyExternalKey(t *testing.T) {
 					Hash: AlgSHA1,
 				},
 				CurveID: CurveNISTP256,
-				Point:   ECPoint{X: pk.PublicKey.X, Y: pk.PublicKey.Y},
+				Point:   ECPoint{XRaw: pk.PublicKey.X.Bytes(), YRaw: pk.PublicKey.Y.Bytes()},
 			},
 		}
 		private := Private{
@@ -727,7 +727,6 @@ func TestSign(t *testing.T) {
 					Hash: AlgSHA256,
 				},
 				CurveID: CurveNISTP256,
-				Point:   ECPoint{X: big.NewInt(0), Y: big.NewInt(0)},
 			},
 		})
 	})
@@ -1196,7 +1195,7 @@ func TestReadPublicKey(t *testing.T) {
 					Hash: AlgSHA1,
 				},
 				CurveID: CurveNISTP256,
-				Point:   ECPoint{X: pk.PublicKey.X, Y: pk.PublicKey.Y},
+				Point:   ECPoint{XRaw: pk.PublicKey.X.Bytes(), YRaw: pk.PublicKey.Y.Bytes()},
 			},
 		}
 		private := Private{