Merge pull request #267 from smallstep/herman/tpmkms-ak-attestation

Support for AK-only attestation through CreateAttestation
smallstep · Jun 20, 2023 · 02557c0 · 02557c0
2 parents 03fb560 + 67901da
commit 02557c0
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 73 deletions.
diff --git a/kms/tpmkms/tpmkms.go b/kms/tpmkms/tpmkms.go
@@ -534,58 +534,33 @@ func (k *TPMKMS) CreateAttestation(req *apiv1.CreateAttestationRequest) (*apiv1.
 	ekKeyURL := ekURL(ekKeyID)
 	permanentIdentifier := ekKeyURL.String()
 
-	if properties.ak {
-		// TODO(hs): decide if we actually want to support this case? TPM attestation
-		// is about attesting application keys using attestation keys.
-		ak, err := k.tpm.GetAK(ctx, properties.name)
+	// check if the derived EK URI fingerprint representation matches the provided
+	// permanent identifier value. The current implementation requires the EK URI to
+	// be used as the AK identity, so an error is returned if there's no match. This
+	// could be changed in the future, so that another attestation flow takes place,
+	// instead, for example.
+	if k.permanentIdentifier != "" && permanentIdentifier != k.permanentIdentifier {
+		return nil, fmt.Errorf("the provided permanent identifier %q does not match the EK URL %q", k.permanentIdentifier, permanentIdentifier)
+	}
+
+	var key *tpm.Key
+	akName := properties.name
+	if !properties.ak {
+		key, err = k.tpm.GetKey(ctx, properties.name)
 		if err != nil {
 			return nil, err
 		}
-		akPub := ak.Public()
-		if akPub == nil {
-			return nil, fmt.Errorf("failed getting AK public key")
-		}
-		akChain := ak.CertificateChain()
-		if len(akChain) == 0 {
-			return nil, fmt.Errorf("no certificate chain available for AK %q", properties.name)
+		if !key.WasAttested() {
+			return nil, fmt.Errorf("key %q was not attested", key.Name())
 		}
-		// TODO(hs): decide if we want/need to return these; their purpose is slightly
-		// different from the key certification parameters.
-		_, err = ak.AttestationParameters(ctx)
-		if err != nil {
-			return nil, fmt.Errorf("failed getting AK attestation parameters: %w", err)
-		}
-		return &apiv1.CreateAttestationResponse{
-			Certificate:         akChain[0], // certificate for the AK
-			CertificateChain:    akChain,    // chain for the AK, including the leaf
-			PublicKey:           akPub,      // returns the public key of the attestation key
-			PermanentIdentifier: permanentIdentifier,
-		}, nil
+		akName = key.AttestedBy()
 	}
 
-	key, err := k.tpm.GetKey(ctx, properties.name)
+	ak, err := k.tpm.GetAK(ctx, akName)
 	if err != nil {
 		return nil, err
 	}
 
-	if !key.WasAttested() {
-		return nil, fmt.Errorf("key %q was not attested", key.Name())
-	}
-
-	ak, err := k.tpm.GetAK(ctx, key.AttestedBy())
-	if err != nil {
-		return nil, fmt.Errorf("failed getting AK for key %q: %w", key.Name(), err)
-	}
-
-	// check if the derived EK URI fingerprint representation matches the provided
-	// permanent identifier value. The current implementation requires the EK URI to
-	// be used as the AK identity, so an error is returned if there's no match. This
-	// could be changed in the future, so that another attestation flow takes place,
-	// instead, for example.
-	if k.permanentIdentifier != "" && ekKeyURL.String() != k.permanentIdentifier {
-		return nil, fmt.Errorf("the provided permanent identifier %q does not match the EK URL %q", k.permanentIdentifier, ekKeyURL.String())
-	}
-
 	// check if a (valid) AK certificate (chain) is available. Perform attestation flow
 	// otherwise. If an AK certificate is available, but not considered valid, e.g. due
 	// to it not having the right identity, a new attestation flow will be performed and
@@ -625,6 +600,25 @@ func (k *TPMKMS) CreateAttestation(req *apiv1.CreateAttestationRequest) (*apiv1.
 		return nil, fmt.Errorf("AK certificate (chain) not valid for EK %q", ekKeyURL)
 	}
 
+	if properties.ak {
+		akPub := ak.Public()
+		if akPub == nil {
+			return nil, fmt.Errorf("failed getting AK public key")
+		}
+		// TODO(hs): decide if we want/need to return these; their purpose is slightly
+		// different from the key certification parameters.
+		_, err = ak.AttestationParameters(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("failed getting AK attestation parameters: %w", err)
+		}
+		return &apiv1.CreateAttestationResponse{
+			Certificate:         akChain[0], // certificate for the AK
+			CertificateChain:    akChain,    // chain for the AK, including the leaf
+			PublicKey:           akPub,      // returns the public key of the attestation key
+			PermanentIdentifier: permanentIdentifier,
+		}, nil
+	}
+
 	signer, err := key.Signer(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed getting signer for key %q: %w", properties.name, err)
@@ -645,9 +639,9 @@ func (k *TPMKMS) CreateAttestation(req *apiv1.CreateAttestationRequest) (*apiv1.
 			Public:            params.Public,
 			CreateData:        params.CreateData,
 			CreateAttestation: params.CreateAttestation,
-			CreateSignature:   params.CreateSignature, // NOTE: should always match the valid value of the AK identity (for now)
+			CreateSignature:   params.CreateSignature,
 		},
-		PermanentIdentifier: permanentIdentifier,
+		PermanentIdentifier: permanentIdentifier, // NOTE: should always match the valid value of the AK identity (for now)
 	}, nil
 }
 

diff --git a/kms/tpmkms/tpmkms_simulator_test.go b/kms/tpmkms/tpmkms_simulator_test.go
@@ -1382,35 +1382,22 @@ func TestTPMKMS_CreateAttestation(t *testing.T) {
 				expErr: errors.New(`failed parsing "tpmkms:name=keyx;ak=true;attest-by=ak1": "ak" and "attest-by" are mutually exclusive`),
 			}
 		},
-		"fail/unknown-ak": func(t *testing.T) test {
-			return test{
-				fields: fields{
-					tpm: tpm,
-				},
-				args: args{
-					req: &apiv1.CreateAttestationRequest{
-						Name: "tpmkms:name=unknownAK;ak=true",
-					},
-				},
-				expErr: errors.New(`failed getting AK "unknownAK": not found`),
-			}
-		},
-		"fail/ak-withoutCertificate": func(t *testing.T) test {
-			akWithoutCert, err := tpm.CreateAK(ctx, "anotherAKWithoutCert")
+		"fail/non-matching-permanent-identifier": func(t *testing.T) test {
+			_, err = tpm.CreateAK(ctx, "newAKWithoutCert")
+			require.NoError(t, err)
+			_, err = tpm.AttestKey(ctx, "newAKWithoutCert", "newkey", config)
 			require.NoError(t, err)
-			akPub := akWithoutCert.Public()
-			require.Implements(t, (*crypto.PublicKey)(nil), akPub)
 			return test{
 				fields: fields{
 					tpm:                 tpm,
-					permanentIdentifier: ekKeyURL.String(),
+					permanentIdentifier: "wrong-provided-permanent-identifier",
 				},
 				args: args{
 					req: &apiv1.CreateAttestationRequest{
-						Name: "tpmkms:name=anotherAKWithoutCert;ak=true", // key1 was attested by the akWithExistingCert at creation time
+						Name: "tpmkms:name=newkey", // newkey was attested by the newAKWithoutCert at creation time
 					},
 				},
-				expErr: errors.New(`no certificate chain available for AK "anotherAKWithoutCert"`),
+				expErr: fmt.Errorf(`the provided permanent identifier "wrong-provided-permanent-identifier" does not match the EK URL %q`, ekKeyURL.String()),
 			}
 		},
 		"fail/unknown-key": func(t *testing.T) test {
@@ -1441,22 +1428,17 @@ func TestTPMKMS_CreateAttestation(t *testing.T) {
 				expErr: errors.New(`key "nonAttestedKey" was not attested`),
 			}
 		},
-		"fail/non-matching-permanent-identifier": func(t *testing.T) test {
-			_, err = tpm.CreateAK(ctx, "newAKWithoutCert")
-			require.NoError(t, err)
-			_, err = tpm.AttestKey(ctx, "newAKWithoutCert", "newkey", config)
-			require.NoError(t, err)
+		"fail/unknown-ak": func(t *testing.T) test {
 			return test{
 				fields: fields{
-					tpm:                 tpm,
-					permanentIdentifier: "wrong-provided-permanent-identifier",
+					tpm: tpm,
 				},
 				args: args{
 					req: &apiv1.CreateAttestationRequest{
-						Name: "tpmkms:name=newkey", // newkey was attested by the newAKWithoutCert at creation time
+						Name: "tpmkms:name=unknownAK;ak=true",
 					},
 				},
-				expErr: fmt.Errorf(`the provided permanent identifier "wrong-provided-permanent-identifier" does not match the EK URL %q`, ekKeyURL.String()),
+				expErr: errors.New(`failed getting AK "unknownAK": not found`),
 			}
 		},
 		"fail/create-attestor-client": func(t *testing.T) test {