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

feat: receive Intel & AMD CRLs in attestation report #159

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

rjzak
Copy link
Member

@rjzak rjzak commented Dec 23, 2022

Depends on:

Refs:

Blocking:

Edit: CRL validation disabled for now to permit new deployments of Enarx, Steward, Drawbridge. Enabling of CRL validation will be a subsequent PR.

Signed-off-by: Richard Zak [email protected]

@rjzak rjzak added the attestation Issues related to attestation label Dec 23, 2022
@rjzak rjzak force-pushed the receive_crl_in_attestation branch 3 times, most recently from dd0f51d to 9afd39a Compare December 24, 2022 01:08
@rjzak rjzak marked this pull request as ready for review December 24, 2022 01:09
@rjzak rjzak requested review from a team, bstrie and haraldh as code owners December 24, 2022 01:09
@rjzak rjzak force-pushed the receive_crl_in_attestation branch 2 times, most recently from 8a8dacf to a4eed6d Compare January 10, 2023 20:58
@rjzak
Copy link
Member Author

rjzak commented Jan 11, 2023

@puiterwijk please double-check the loop, as I noticed that some intermediate certs are failing the CRL signature check. The logic:

  1. Get the URLs from the TbsCertificate, can be more than one.
  2. Identify the CertificateList objects to use based on the found URLs, or bail! if no CRLs are found.
  3. Get the raw bytes of the CRL by converting the CertificateList.tbs_cert_list back to DER
  4. Use the Certificate's public key to check the DER bytes above with the signature in the CRL.

Example of when it succeeds:

CRL validation success!
        Cert algo: 1.2.840.113549.1.1.10
        Cert subject: OU=Engineering,C=US,L=Santa Clara,STATEORPROVINCENAME=CA,O=Advanced Micro Devices,CN=ARK-Milan
        CRL issuer: OU=Engineering,C=US,L=Santa Clara,STATEORPROVINCENAME=CA,O=Advanced Micro Devices,CN=ARK-Milan
        CRL algo: 1.2.840.113549.1.1.10
        URL: ["https://kdsintf.amd.com/vcek/v1/Milan/crl"]

Example when it fails:

CRL validation error: signature error: verification error
        Cert algo: 1.2.840.113549.1.1.10
        Cert subject: OU=Engineering,C=US,L=Santa Clara,STATEORPROVINCENAME=CA,O=Advanced Micro Devices,CN=SEV-Milan
        CRL issuer: OU=Engineering,C=US,L=Santa Clara,STATEORPROVINCENAME=CA,O=Advanced Micro Devices,CN=ARK-Milan
        CRL algo: 1.2.840.113549.1.1.10
        URL: ["https://kdsintf.amd.com/vcek/v1/Milan/crl"]

The issuer is different in the failure case, but the cert had the same CRL URL.

@rjzak rjzak marked this pull request as draft January 11, 2023 20:48
@rjzak
Copy link
Member Author

rjzak commented Jan 11, 2023

Seems that I have an issue where the wrong CRL is used to validate intermediate certs. I've made a tool to help me debug what's going on: https://github.com/rjzak/cert-buddy. So far, the CRL format on-disk is fine.

@rjzak rjzak force-pushed the receive_crl_in_attestation branch 2 times, most recently from 9e894d1 to c1efdf1 Compare January 12, 2023 18:51
@rjzak
Copy link
Member Author

rjzak commented Jan 12, 2023

Progress: CRLs are now passing for Intel. AMD isn't working as the root CRL is used for intermediate & end certificates, so a different CRL selection strategy is needed.

@rjzak rjzak force-pushed the receive_crl_in_attestation branch 4 times, most recently from d9cfe78 to 60a09ed Compare January 13, 2023 15:15
@rjzak rjzak marked this pull request as ready for review January 13, 2023 15:15
@rjzak rjzak requested a review from puiterwijk January 13, 2023 15:16
@rjzak rjzak enabled auto-merge (rebase) January 13, 2023 16:15
@rjzak rjzak force-pushed the receive_crl_in_attestation branch 3 times, most recently from 9dd9ffa to 584d387 Compare January 13, 2023 16:26
@rjzak rjzak requested a review from puiterwijk January 13, 2023 17:11
@dpal dpal self-requested a review January 13, 2023 21:10
@rjzak rjzak merged commit 5868aa7 into enarx:main Jan 13, 2023
@rjzak rjzak deleted the receive_crl_in_attestation branch January 13, 2023 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attestation Issues related to attestation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants