Repository: google/go-tpm
Pull request #353 (2024-02-27 21:29:49)

Bump go to 1.22 and golangci-lint to 1.56.2
- [x] Update Go to 1.22
- [x] Update golangci-lint to 1.56.2
- [x] Correct new syntax lint errors
- [x] Refactor crypto that was using deprecated functions
- [x] Move `golangci-lint` run arguments to a configuration file

NOTE: Rather than disable the `dot-imports` rule for revive, an `exclude` directive should be used, however https://github.com/golangci/golangci-lint/pull/4365 was not included in 1.56.2 so this PR disables a rule check that was added since 1.52.  After the next release of `golangci-lint` the following can be applied:

```diff
diff --git a/.golangci.yml b/.golangci.yml
index 46a8a73..e4d95d5 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -10,7 +10,7 @@ linters-settings:
   revive:
     rules:
     - name: dot-imports
-      disabled: true
+      exclude: ["**/*_test.go"]
 issues:
   exclude-use-default: false
   exclude:
```

If this PR is merged before the next `golangci-lint` release I will create a new draft PR to capture the update.



Foxboron: [6c3f164](https://github.com/google/go-tpm/pull/353/commits/6c3f164c3f7c39899a6502e78246398b579e2ab2) solves https://github.com/google/go-tpm/issues/327
nckrss: @chrisfenner are there other changes that you would like to see?
chrisfenner: So sorry for the delays in review @nckrss, this and other active PRs here are in my backlog and I hope to get to it soon. Thank you for sending this!

Patch:
diff --git a/.cirrus.yml b/.cirrus.yml
index f98596d..63caa0b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -18,17 +18,4 @@ lint_task:
       - GOOS: linux
       - GOOS: windows
   lint_script:
-    golangci-lint run
-      -D errcheck
-      -E stylecheck
-      -E goimports
-      -E misspell
-      -E revive
-      --exclude-use-default=false
-      --exclude stutters
-      --exclude underscores
-      --exclude unexported-return
-      --max-same-issues=0
-      --max-issues-per-linter=0
-      ./tpmutil/...
-      ./tpm2/...
+    golangci-lint run ./tpmutil/... ./tpm2/...
diff --git a/.golangci.yml b/.golangci.yml
new file mode 100644
index 0000000..46a8a73
--- /dev/null
+++ b/.golangci.yml
@@ -0,0 +1,21 @@
+linters:
+  disable:
+    - errcheck
+  enable:
+    - stylecheck
+    - goimports
+    - misspell
+    - revive
+linters-settings:
+  revive:
+    rules:
+    - name: dot-imports
+      disabled: true
+issues:
+  exclude-use-default: false
+  exclude:
+    - stutters
+    - underscores
+    - unexported-return
+  max-issues-per-linter: 0
+  max-same-issues: 0
diff --git a/Dockerfile b/Dockerfile
index a5f2021..d236165 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,10 +1,10 @@
-FROM golang:1.21
+FROM golang:1.22
 # We need OpenSSL headers to build the simulator
 RUN apt-get update && apt-get install -y \
     libssl-dev \
  && rm -rf /var/lib/apt/lists/*
 # We need golangci-lint for linting
-ARG VERSION=1.52.2
+ARG VERSION=1.56.2
 RUN curl -SL \
     https://github.com/golangci/golangci-lint/releases/download/v${VERSION}/golangci-lint-${VERSION}-linux-amd64.tar.gz \
     --output golangci.tar.gz \
diff --git a/go.mod b/go.mod
index b7bb002..04ef731 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/google/go-tpm
 
-go 1.20
+go 1.22
 
 require (
 	github.com/google/go-cmp v0.5.9
diff --git a/go.sum b/go.sum
index 974eb39..d7a1b31 100644
--- a/go.sum
+++ b/go.sum
@@ -1,13 +1,20 @@
 github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-sev-guest v0.6.1 h1:NajHkAaLqN9/aW7bCFSUplUMtDgk2+HcN7jC2btFtk0=
+github.com/google/go-sev-guest v0.6.1/go.mod h1:UEi9uwoPbLdKGl1QHaq1G8pfCbQ4QP0swWX4J0k6r+Q=
 github.com/google/go-tpm-tools v0.3.13-0.20230620182252-4639ecce2aba h1:qJEJcuLzH5KDR0gKc0zcktin6KSAwL7+jWKBYceddTc=
 github.com/google/go-tpm-tools v0.3.13-0.20230620182252-4639ecce2aba/go.mod h1:EFYHy8/1y2KfgTAsx7Luu7NGhoxtuVHnNo8jE7FikKc=
 github.com/google/logger v1.1.1 h1:+6Z2geNxc9G+4D4oDO9njjjn2d0wN5d7uOo0vOIW1NQ=
+github.com/google/logger v1.1.1/go.mod h1:BkeJZ+1FhQ+/d087r4dzojEg1u2ZX+ZqG1jTUrLM+zQ=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
+github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g=
+github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e h1:T8NU3HyQ8ClP4SEE+KbFlg6n0NhuTsN4MyznaarGsZM=
+golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
 golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
 golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw=
+google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
diff --git a/tpm2/crypto.go b/tpm2/crypto.go
index c8eb2bc..3082ccc 100644
--- a/tpm2/crypto.go
+++ b/tpm2/crypto.go
@@ -1,8 +1,11 @@
 package tpm2
 
 import (
+	"crypto/ecdh"
+	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rsa"
+	"fmt"
 	"math/big"
 )
 
@@ -21,23 +24,53 @@ func RSAPub(parms *TPMSRSAParms, pub *TPM2BPublicKeyRSA) (*rsa.PublicKey, error)
 	return &result, nil
 }
 
-// ECDHPub is a convenience wrapper around the necessary info to perform point
-// multiplication with the elliptic package.
-type ECDHPub struct {
-	Curve elliptic.Curve
-	X, Y  *big.Int
-}
+// ECDHPubKey converts a TPM ECC public key into one recognized by the ecdh package
+func ECDHPubKey(curve ecdh.Curve, pub *TPMSECCPoint) (*ecdh.PublicKey, error) {
 
-// ECCPub converts a TPM ECC public key into one recognized by the elliptic
-// package's point-multiplication functions, for use in ECDH.
-func ECCPub(parms *TPMSECCParms, pub *TPMSECCPoint) (*ECDHPub, error) {
-	curve, err := parms.CurveID.Curve()
-	if err != nil {
-		return nil, err
+	var c elliptic.Curve
+	switch curve {
+	case ecdh.P256():
+		c = elliptic.P256()
+	case ecdh.P384():
+		c = elliptic.P384()
+	case ecdh.P521():
+		c = elliptic.P521()
+	default:
+		return nil, fmt.Errorf("unknown curve: %v", curve)
 	}
-	return &ECDHPub{
-		Curve: curve,
+
+	pubKey := ecdsa.PublicKey{
+		Curve: c,
 		X:     big.NewInt(0).SetBytes(pub.X.Buffer),
 		Y:     big.NewInt(0).SetBytes(pub.Y.Buffer),
-	}, nil
+	}
+
+	return pubKey.ECDH()
+}
+
+// ECCPoint returns an uncompressed ECC Point
+func ECCPoint(pubKey *ecdh.PublicKey) (*big.Int, *big.Int, error) {
+	b := pubKey.Bytes()
+	size, err := elementLength(pubKey.Curve())
+	if err != nil {
+		return nil, nil, fmt.Errorf("ECCPoint: %w", err)
+	}
+	return big.NewInt(0).SetBytes(b[1 : size+1]),
+		big.NewInt(0).SetBytes(b[size+1:]), nil
+}
+
+func elementLength(c ecdh.Curve) (int, error) {
+	switch c {
+	case ecdh.P256():
+		// crypto/internal/nistec/fiat.p256ElementLen
+		return 32, nil
+	case ecdh.P384():
+		// crypto/internal/nistec/fiat.p384ElementLen
+		return 48, nil
+	case ecdh.P521():
+		// crypto/internal/nistec/fiat.p521ElementLen
+		return 66, nil
+	default:
+		return 0, fmt.Errorf("unknown element length for curve: %v", c)
+	}
 }
diff --git a/tpm2/reflect.go b/tpm2/reflect.go
index c4be24e..60c9d58 100644
--- a/tpm2/reflect.go
+++ b/tpm2/reflect.go
@@ -839,9 +839,9 @@ func marshalParameter[R any](buf *bytes.Buffer, cmd Command[R, *R], i int) error
 		return marshal(buf, reflect.ValueOf(TPMRHNull))
 	} else if parm.IsZero() && parm.Kind() == reflect.Uint16 && hasTag(field, "nullable") {
 		return marshal(buf, reflect.ValueOf(TPMAlgNull))
-	} else {
-		return marshal(buf, parm)
 	}
+
+	return marshal(buf, parm)
 }
 
 // cmdParameters returns the parameters area of the command.
diff --git a/tpm2/sessions.go b/tpm2/sessions.go
index 171adf8..bee12b2 100644
--- a/tpm2/sessions.go
+++ b/tpm2/sessions.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"crypto/aes"
 	"crypto/cipher"
-	"crypto/elliptic"
 	"crypto/hmac"
 	"crypto/rand"
 	"crypto/rsa"
@@ -425,24 +424,31 @@ func getEncryptedSaltRSA(nameAlg TPMIAlgHash, parms *TPMSRSAParms, pub *TPM2BPub
 
 // Part 1, 19.6.13
 func getEncryptedSaltECC(nameAlg TPMIAlgHash, parms *TPMSECCParms, pub *TPMSECCPoint) (*TPM2BEncryptedSecret, []byte, error) {
-	curve, err := parms.CurveID.Curve()
+	curve, err := parms.CurveID.ECDHCurve()
 	if err != nil {
-		return nil, nil, fmt.Errorf("could not encrypt salt to ECC key: %w", err)
+		return nil, nil, fmt.Errorf("ecc salt: param curve: %w", err)
 	}
-	eccPub, err := ECCPub(parms, pub)
+	eccPub, err := ECDHPubKey(curve, pub)
 	if err != nil {
-		return nil, nil, fmt.Errorf("could not encrypt salt to ECC key: %w", err)
+		return nil, nil, fmt.Errorf("ecc salt: unmarshalling tpm ecc key: %w", err)
 	}
-	ephPriv, ephPubX, ephPubY, err := elliptic.GenerateKey(curve, rand.Reader)
+
+	// Generate new ECDH key
+	ephPriv, err := curve.GenerateKey(rand.Reader)
+	if err != nil {
+		return nil, nil, fmt.Errorf("ecc salt: generating ecc private key: %w", err)
+	}
+	ephPubX, ephPubY, err := ECCPoint(ephPriv.PublicKey())
+	if err != nil {
+		return nil, nil, fmt.Errorf("ecc salt: ecc pubkey: %w", err)
+	}
+
+	// Calculate Z (ECDH key * TPM pub)
+	z, err := ephPriv.ECDH(eccPub)
 	if err != nil {
-		return nil, nil, fmt.Errorf("could not encrypt salt to ECC key: %w", err)
-	}
-	zx, _ := curve.Params().ScalarMult(eccPub.X, eccPub.Y, ephPriv)
-	// ScalarMult returns a big.Int, whose Bytes() function may return the
-	// compacted form. In our case, we want to left-pad zx to the size of
-	// the curve.
-	z := make([]byte, (curve.Params().BitSize+7)/8)
-	zx.FillBytes(z)
+		return nil, nil, fmt.Errorf("ecc salt: z calc: %w", err)
+	}
+
 	ha, err := nameAlg.Hash()
 	if err != nil {
 		return nil, nil, err
diff --git a/tpm2/structures.go b/tpm2/structures.go
index b8b9d33..fea2077 100644
--- a/tpm2/structures.go
+++ b/tpm2/structures.go
@@ -4,6 +4,7 @@ package tpm2
 import (
 	"bytes"
 	"crypto"
+	"crypto/ecdh"
 	"crypto/elliptic"
 	"encoding/binary"
 	"reflect"
@@ -96,6 +97,20 @@ func (c TPMECCCurve) Curve() (elliptic.Curve, error) {
 	}
 }
 
+// ECDHCurve returns the ecdh.Curve associated with a TPMECCCurve.
+func (c TPMECCCurve) ECDHCurve() (ecdh.Curve, error) {
+	switch c {
+	case TPMECCNistP256:
+		return ecdh.P256(), nil
+	case TPMECCNistP384:
+		return ecdh.P384(), nil
+	case TPMECCNistP521:
+		return ecdh.P521(), nil
+	default:
+		return nil, fmt.Errorf("unsupported ECC curve: %v", c)
+	}
+}
+
 // HandleValue returns the handle value. This behavior is intended to satisfy
 // an interface that can be implemented by other, more complex types as well.
 func (h TPMHandle) HandleValue() uint32 {


Test patch:
diff --git a/tpm2/test/ecdh_test.go b/tpm2/test/ecdh_test.go
index fbf333f..248e973 100644
--- a/tpm2/test/ecdh_test.go
+++ b/tpm2/test/ecdh_test.go
@@ -1,9 +1,8 @@
 package tpm2test
 
 import (
-	"crypto/elliptic"
+	"crypto/ecdh"
 	"crypto/rand"
-	"math/big"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -57,6 +56,9 @@ func TestECDH(t *testing.T) {
 		}),
 	}
 
+	// Use NIST P-256
+	curve := ecdh.P256()
+
 	tpmCreateRsp, err := tpmCreate.Execute(thetpm)
 	if err != nil {
 		t.Fatalf("could not create the TPM key: %v", err)
@@ -69,24 +71,33 @@ func TestECDH(t *testing.T) {
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
-	tpmX := big.NewInt(0).SetBytes(tpmPub.X.Buffer)
-	tpmY := big.NewInt(0).SetBytes(tpmPub.Y.Buffer)
+	tpmPubKey, err := ECDHPubKey(curve, tpmPub)
+	if err != nil {
+		t.Fatalf("could not unmarshall pubkey: %v", err)
+	}
 
 	// Create a SW ECDH key
-	priv, x, y, err := elliptic.GenerateKey(elliptic.P256(), rand.Reader)
+	swPriv, err := curve.GenerateKey(rand.Reader)
 	if err != nil {
 		t.Fatalf("could not create the SW key: %v", err)
 	}
+	x, y, err := ECCPoint(swPriv.PublicKey())
+	if err != nil {
+		t.Fatalf("could not get SW key point: %v", err)
+	}
 	swPub := TPMSECCPoint{
 		X: TPM2BECCParameter{Buffer: x.FillBytes(make([]byte, 32))},
 		Y: TPM2BECCParameter{Buffer: y.FillBytes(make([]byte, 32))},
 	}
 
 	// Calculate Z based on the SW priv * TPM pub
-	zx, zy := elliptic.P256().ScalarMult(tpmX, tpmY, priv)
+	zx, err := swPriv.ECDH(tpmPubKey)
+	if err != nil {
+		t.Fatalf("ecdh exchange: %v", err)
+	}
+
 	z := TPMSECCPoint{
-		X: TPM2BECCParameter{Buffer: zx.FillBytes(make([]byte, 32))},
-		Y: TPM2BECCParameter{Buffer: zy.FillBytes(make([]byte, 32))},
+		X: TPM2BECCParameter{Buffer: zx},
 	}
 
 	// Calculate Z based on TPM priv * SW pub