Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for TPM1.2 enrollment. #52

Merged
merged 7 commits into from
Jan 17, 2025
Merged

Add support for TPM1.2 enrollment. #52

merged 7 commits into from
Jan 17, 2025

Conversation

marcushines
Copy link
Contributor

Create the identity request. This takes the public key of the attestation certificate authority as input and returns a TPM struct with AIK (note: TPM structures are different for TPM1.2 & TPM2.0) Send the AIK cert encrypted by the EK back to the TPM. If the TPM can successfully decrypt the payload, the enrollment is complete.

Create the identity request. This takes the public key of the attestation certificate authority as input and returns a TPM struct with AIK (note: TPM structures are different for TPM1.2 & TPM2.0)
Send the AIK cert encrypted by the EK back to the TPM. If the TPM can successfully decrypt the payload, the enrollment is complete.
@marcushines marcushines requested review from morrowc and a team as code owners January 13, 2025 20:10
@coveralls
Copy link

coveralls commented Jan 13, 2025

Pull Request Test Coverage Report for Build 12834135710

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.885%

Totals Coverage Status
Change from base Build 12834112673: 0.0%
Covered Lines: 318
Relevant Lines: 366

💛 - Coveralls

@marcushines marcushines requested a review from betuls January 13, 2025 20:21
proto/tpm_enrollz.proto Outdated Show resolved Hide resolved
Copy link

@betuls betuls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked with the service side and attestation key is not stored by the service. It is sent by the device in AttestResponse. So we need an additional API to get the attestation key from the device.
In this case would it make sense to have GetAIKKey (or cert), GenerateAIK and RotateAIKCert where GenerateAIKRequest will send issuer_public_key and RotateAIKCert will send the aik_cert instead of having single RotateAIKCert?

proto/tpm_enrollz.proto Outdated Show resolved Hide resolved
proto/tpm_enrollz.proto Outdated Show resolved Hide resolved
proto/tpm_enrollz.proto Outdated Show resolved Hide resolved
@nmahabaleshwar
Copy link

nmahabaleshwar commented Jan 16, 2025

I checked with the service side and attestation key is not stored by the service. It is sent by the device in AttestResponse. So we need an additional API to get the attestation key from the device. In this case would it make sense to have GetAIKKey (or cert), GenerateAIK and RotateAIKCert where GenerateAIKRequest will send issuer_public_key and RotateAIKCert will send the aik_cert instead of having single RotateAIKCert?

We don't think the public portion of the AIK is required. In the AttestResponse, the AIK certificate should be returned which can be used by the issuer to verify the validity. Does this sound right ?

Proposing a minor change to the AttestResponse.

message AttestResponse {
   message AttestationCert {
      oneof value {
         string oiak_cert = 1;
         string aik_cert = 2;
      }
   }
   
   ControlCardVendorId control_card_id = 1;
   string oiak_cert = 2 [deprecated = true];
   map<int32, bytes> pcr_values = 3;
   bytes quoted = 4;
   bytes quote_signature = 5;
   string oidevid_cert = 6;
   AttestationCert attestation_cert = 7;
}

@marcushines
Copy link
Contributor Author

I checked with the service side and attestation key is not stored by the service. It is sent by the device in AttestResponse. So we need an additional API to get the attestation key from the device. In this case would it make sense to have GetAIKKey (or cert), GenerateAIK and RotateAIKCert where GenerateAIKRequest will send issuer_public_key and RotateAIKCert will send the aik_cert instead of having single RotateAIKCert?

We don't think the public portion of the AIK is required. In the AttestResponse, the AIK certificate should be returned which can be used by the issuer to verify the validity. Does this sound right ?

Proposing a minor change to the AttestResponse.

message AttestResponse {
   message AttestationCert {
      oneof value {
         string oiak_cert = 1;
         string aik_cert = 2;
      }
   }
   
   ControlCardVendorId control_card_id = 1;
   string oiak_cert = 2 [deprecated = true];
   map<int32, bytes> pcr_values = 3;
   bytes quoted = 4;
   bytes quote_signature = 5;
   string oidevid_cert = 6;
   AttestationCert attestation_cert = 7;
}

Completely agree that was going to be the implementation

@betuls
Copy link

betuls commented Jan 16, 2025

I checked with the service side and attestation key is not stored by the service. It is sent by the device in AttestResponse. So we need an additional API to get the attestation key from the device. In this case would it make sense to have GetAIKKey (or cert), GenerateAIK and RotateAIKCert where GenerateAIKRequest will send issuer_public_key and RotateAIKCert will send the aik_cert instead of having single RotateAIKCert?

We don't think the public portion of the AIK is required. In the AttestResponse, the AIK certificate should be returned which can be used by the issuer to verify the validity. Does this sound right ?
Proposing a minor change to the AttestResponse.

message AttestResponse {
   message AttestationCert {
      oneof value {
         string oiak_cert = 1;
         string aik_cert = 2;
      }
   }
   
   ControlCardVendorId control_card_id = 1;
   string oiak_cert = 2 [deprecated = true];
   map<int32, bytes> pcr_values = 3;
   bytes quoted = 4;
   bytes quote_signature = 5;
   string oidevid_cert = 6;
   AttestationCert attestation_cert = 7;
}

Completely agree that was going to be the implementation

This sounds right.

To my initial comment about separating the API, seems like TPM 1.2 doesn't provide any command to return AIK cert. Per offline discussion with Marcus RotateAIKCert will generate a new AIK each time. Therefore we don't need the additional API to request the cert.

@marcushines marcushines merged commit f6ec501 into main Jan 17, 2025
8 checks passed
@marcushines marcushines deleted the enrollz branch January 17, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants