MAL-9769 Include latest pckcert in fallback pckcerts request #801

dzil123 · 2025-08-19T01:31:17Z

No description provided.

intel-sgx/dcap-artifact-retrieval/src/provisioning_client/mod.rs

jethrogb · 2025-08-19T21:26:29Z

Let's get additional certs from:

PCKID CPUSVN (this is what this PR currently does)
CPUSVN all 1's
For every CPUSVN we try where the late microcode value is higher than the early microcode value, the CPUSVN where the early microcode value is set to the late microcode value:

            {
              "svn": 7,
              "category": "BIOS",
              "type": "Early Microcode Update"
            },
            {
              "svn": 9,
              "category": "OS/VMM",
              "type": "SGX Late Microcode Update"
            },

Taowyoo · 2025-08-25T19:17:09Z

Let's get additional certs from:

* PCKID CPUSVN (this is what this PR currently does)

* CPUSVN all 1's

* For every CPUSVN we try where the late microcode value is higher than the early microcode value, the CPUSVN where the early microcode value is set to the late microcode value:

            {
              "svn": 7,
              "category": "BIOS",
              "type": "Early Microcode Update"
            },
            {
              "svn": 9,
              "category": "OS/VMM",
              "type": "SGX Late Microcode Update"
            },

HI @jethrogb , why the 3rd option is needed ?

jethrogb · 2025-08-25T21:05:55Z

I want to capture the specific edge case we've identified for our cert iteration logic.

jethrogb · 2025-08-25T21:19:35Z

intel-sgx/pcs/src/tcb_info.rs

+                (overridden_svn, tcb_level.tcb.pce_svn())
+            } else {
+                (cpu_svn, tcb_level.tcb.pce_svn())
+            }


For cpu_svn != overridden_svn, I was expecting this iterator yields two items: (overridden_svn, tcb_level.tcb.pce_svn()) and (cpu_svn, tcb_level.tcb.pce_svn())

The comment above hasn't been resolved yet. This is also highly specific to the pckcerts_with_fallback function in the dcap-artifact-retrieval crate. It's highly unlikely that there will be other users of this function. Can we move it to that crate/function? By adding it very close to the chaining of the iterators, without a dedicated function, there won't be an expectation anymore that this specific part yields two items.

The problem is self.tcb_levels is private. So I cannot move any logic specific to it to other crates.

Hi @jethrogb , my current approach is chaining the iterators from iter_tcb_components and iter_tcb_components_with_late_tcb_override_only in pckcerts_with_fallback.
Do you mean you want to change it to one iterator that always return overridden_svn and cpu_svn and PceIsvsvn ?
Do you also means, in pckcerts_with_fallback, we do not need to get pckcert with cpu_svn is overridden_svn` is higher?

The problem is self.tcb_levels is private. So I cannot move any logic specific to it to other crates.

We can easily make tcb_level public for TcbData<Verified>

intel-sgx/pcs/src/pckcrt.rs

intel-sgx/dcap-artifact-retrieval/src/provisioning_client/mod.rs

raoulstrackx · 2025-09-03T12:22:13Z

intel-sgx/pcs/src/pckcrt.rs

+            .position(|comp| comp.comp_type == "Early Microcode Update");
+        let late_idx = tcb_components
+            .iter()
+            .position(|comp| comp.comp_type == "SGX Late Microcode Update");


This likely will go well, but I'm hesitant to rely on the exact string matches here. I think it's more likely that we re-use this code for other platforms/technologies (e.g., PCS v5, TDX, ...) where the descriptions have changed, than that the meaning of cpu svn components change. We could just have take position 0 and 1 in the cpusvn, instead of searching for exact strings. (see also comment below)

We definitely can't rely on the specific ordering of SVN components. Intel makes no guarantees about that.

@raoulstrackx This PR was approved but this comment wasn't addressed.

Created #821 for this

raoulstrackx · 2025-09-03T12:32:38Z

intel-sgx/pcs/src/tcb_info.rs

+                (overridden_svn, tcb_level.tcb.pce_svn())
+            } else {
+                (cpu_svn, tcb_level.tcb.pce_svn())
+            }


The comment above hasn't been resolved yet. This is also highly specific to the pckcerts_with_fallback function in the dcap-artifact-retrieval crate. It's highly unlikely that there will be other users of this function. Can we move it to that crate/function? By adding it very close to the chaining of the iterators, without a dedicated function, there won't be an expectation anymore that this specific part yields two items.

New logic will get following PCK certs: - PCKID CPUSVN - CPUSVN all 1's - For every CPUSVN we try where the late microcode value is higher than the early microcode value, the CPUSVN where the early microcode value is set to the late microcode value.

intel-sgx/dcap-artifact-retrieval/src/provisioning_client/mod.rs

in issue #811

raoulstrackx · 2025-09-15T12:29:36Z

I've tested the different ways to get the "problematic" PckCert when the early and late stage microcode code update versions differ, and they all work. I've approved the PR, but I feel someone else should take another look as I wrote large parts myself

Taowyoo

@raoulstrackx changed part looks good to me

dzil123 requested review from aditijannu, jethrogb and raoulstrackx August 19, 2025 01:31

raoulstrackx reviewed Aug 19, 2025

View reviewed changes

intel-sgx/dcap-artifact-retrieval/src/provisioning_client/mod.rs Outdated Show resolved Hide resolved

Taowyoo previously approved these changes Aug 19, 2025

View reviewed changes

Taowyoo self-requested a review August 20, 2025 21:09

Taowyoo dismissed their stale review via 2bed856 August 25, 2025 19:14

Taowyoo force-pushed the dz/MAL-9769 branch from a6697a3 to 2bed856 Compare August 25, 2025 19:14

jethrogb reviewed Aug 25, 2025

View reviewed changes

Taowyoo force-pushed the dz/MAL-9769 branch 2 times, most recently from bc08090 to b5185cf Compare August 25, 2025 21:58

Taowyoo requested review from jethrogb and raoulstrackx August 25, 2025 22:01

jethrogb reviewed Aug 25, 2025

View reviewed changes

intel-sgx/pcs/src/pckcrt.rs Outdated Show resolved Hide resolved

Taowyoo force-pushed the dz/MAL-9769 branch from f5733e2 to 8f24f96 Compare August 27, 2025 20:04

Taowyoo requested a review from jethrogb August 27, 2025 20:05

raoulstrackx reviewed Sep 3, 2025

View reviewed changes

Taowyoo force-pushed the dz/MAL-9769 branch from 8f24f96 to c2ad654 Compare September 4, 2025 18:57

Taowyoo requested a review from raoulstrackx September 4, 2025 18:58

dzil123 and others added 5 commits September 12, 2025 10:22

MAL-9769 Include latest pckcert in fallback pckcerts request

51a7bf4

update azure pckid for tests

591df15

update fn pckcerts_with_fallback

a98eaf8

New logic will get following PCK certs: - PCKID CPUSVN - CPUSVN all 1's - For every CPUSVN we try where the late microcode value is higher than the early microcode value, the CPUSVN where the early microcode value is set to the late microcode value.

Address reviewer comments

8a476b6

fix compile

7b20901

Taowyoo force-pushed the dz/MAL-9769 branch from 5daff92 to 7b20901 Compare September 12, 2025 17:26

Taowyoo reviewed Sep 12, 2025

View reviewed changes

intel-sgx/dcap-artifact-retrieval/src/provisioning_client/mod.rs Show resolved Hide resolved

Taowyoo previously approved these changes Sep 12, 2025

View reviewed changes

temporarily fix pccs test failure, todo is added

f09a262

in issue #811

Taowyoo dismissed their stale review via f09a262 September 12, 2025 18:38

raoulstrackx approved these changes Sep 15, 2025

View reviewed changes

Taowyoo approved these changes Sep 15, 2025

View reviewed changes

Taowyoo added this pull request to the merge queue Sep 15, 2025

Merged via the queue into master with commit 89af311 Sep 15, 2025
1 check passed

Taowyoo deleted the dz/MAL-9769 branch September 15, 2025 20:43

MAL-9769 Include latest pckcert in fallback pckcerts request #801

MAL-9769 Include latest pckcert in fallback pckcerts request #801

Uh oh!

Conversation

dzil123 commented Aug 19, 2025

Uh oh!

Uh oh!

jethrogb commented Aug 19, 2025

Uh oh!

Taowyoo commented Aug 25, 2025

Uh oh!

jethrogb commented Aug 25, 2025

Uh oh!

jethrogb Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

raoulstrackx commented Sep 15, 2025

Uh oh!

Taowyoo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jethrogb Aug 25, 2025 •

edited

Loading

Taowyoo left a comment •

edited

Loading