Skip to content

Commit 6f22de1

Browse files
kvinwangkvinwang
committed
fix: address PR review - separate policy API, remove tdx_report()
- Remove tdx_report() method, callers use tdx_qvr()+supplemental() instead - Fix ppid access: use supplemental().platform.pck.ppid (no QVR clone) - Split TCB policy into separate API endpoints: - GET /policy/app/:appId and GET /policy/kms - auth_api.get_app_policy() and auth_api.get_kms_policy() in Rust - Remove tcbPolicy from BootResponse (separate concern) - Move policy validation before is_app_allowed check - Fix cargo fmt issues
1 parent bc5c574 commit 6f22de1

7 files changed

Lines changed: 132 additions & 70 deletions

File tree

‎dstack-attest/src/attestation.rs‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -222,22 +222,10 @@ impl DstackVerifiedReport {
222222
/// Returns `None` for non-TDX reports.
223223
pub fn validate_tdx(self, policy: &dyn Policy) -> Result<Option<TdxVerifiedReport>> {
224224
match self {
225-
DstackVerifiedReport::DstackTdx(qvr) => {
226-
Ok(Some(qvr.validate(policy)?))
227-
}
225+
DstackVerifiedReport::DstackTdx(qvr) => Ok(Some(qvr.validate(policy)?)),
228226
_ => Ok(None),
229227
}
230228
}
231-
232-
/// Get a [`TdxVerifiedReport`] without additional policy checks.
233-
///
234-
/// Safe to call because the baseline policy was already validated during verification.
235-
pub fn tdx_report(&self) -> Option<TdxVerifiedReport> {
236-
match self {
237-
DstackVerifiedReport::DstackTdx(qvr) => Some(qvr.clone().into_report_unchecked()),
238-
_ => None,
239-
}
240-
}
241229
}
242230

243231
/// Represents a verified attestation
@@ -402,9 +390,10 @@ impl GetDeviceId for () {
402390
impl GetDeviceId for DstackVerifiedReport {
403391
fn get_devide_id(&self) -> Vec<u8> {
404392
match self {
405-
DstackVerifiedReport::DstackTdx(qvr) => {
406-
qvr.clone().into_report_unchecked().ppid
407-
}
393+
DstackVerifiedReport::DstackTdx(qvr) => qvr
394+
.supplemental()
395+
.map(|s| s.platform.pck.ppid)
396+
.unwrap_or_default(),
408397
DstackVerifiedReport::DstackGcpTdx => Vec::new(),
409398
DstackVerifiedReport::DstackNitroEnclave => Vec::new(),
410399
}
@@ -730,13 +719,14 @@ impl Attestation {
730719
.duration_since(SystemTime::UNIX_EPOCH)
731720
.context("system time before epoch")?
732721
.as_secs();
733-
let qvr =
734-
dcap_qvl::collateral::get_collateral_and_verify(quote, Some(pccs_url.as_ref()))
735-
.await
736-
.context("Failed to get collateral")?;
722+
let qvr = dcap_qvl::collateral::get_collateral_and_verify(quote, Some(pccs_url.as_ref()))
723+
.await
724+
.context("Failed to get collateral")?;
737725

738726
// Baseline policy validation (business layer can apply stricter policies on the returned QVR)
739-
let supplemental = qvr.supplemental().context("Failed to build supplemental data")?;
727+
let supplemental = qvr
728+
.supplemental()
729+
.context("Failed to build supplemental data")?;
740730
default_policy(now_secs)
741731
.validate(&supplemental)
742732
.context("TCB policy validation failed")?;
@@ -809,7 +799,7 @@ pub fn default_policy(now_secs: u64) -> SimplePolicy {
809799
SimplePolicy::strict(now_secs)
810800
.allow_status(TcbStatus::OutOfDate)
811801
.platform_grace_period(Duration::from_secs(30 * 24 * 3600))
812-
.qe_grace_period(Duration::from_secs(14 * 24 * 3600))
802+
.qe_grace_period(Duration::from_secs(30 * 24 * 3600))
813803
.allow_smt(true)
814804
.reject_advisory("INTEL-SA-01397")
815805
.reject_advisory("INTEL-SA-01367")

‎kms/auth-eth/src/ethereum.ts‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// SPDX-License-Identifier: Apache-2.0
44

55
import { ethers } from 'ethers';
6-
import { BootInfo, BootResponse } from './types';
6+
import { BootInfo, BootResponse, PolicyResponse } from './types';
77
import { DstackKms__factory } from '../typechain-types/factories/contracts/DstackKms__factory';
88
import { DstackKms } from '../typechain-types/contracts/DstackKms';
99
import { IAppTcbPolicy__factory } from '../typechain-types/factories/contracts/IAppTcbPolicy__factory';
@@ -68,23 +68,10 @@ export class EthereumBackend {
6868
const [isAllowed, reason] = response;
6969
const gatewayAppId = await this.kmsContract.gatewayAppId();
7070

71-
// Read TCB policy from the relevant contract
72-
let tcbPolicy = '';
73-
if (isAllowed) {
74-
if (isKms) {
75-
// KMS boot: read policy from KMS contract itself
76-
tcbPolicy = await this.tryReadTcbPolicy(await this.kmsContract.getAddress());
77-
} else {
78-
// App boot: read policy from the app contract
79-
tcbPolicy = await this.tryReadTcbPolicy(bootInfoStruct.appId);
80-
}
81-
}
82-
8371
return {
8472
isAllowed,
8573
reason,
8674
gatewayAppId,
87-
tcbPolicy,
8875
}
8976
}
9077

@@ -97,6 +84,16 @@ export class EthereumBackend {
9784
return Number(chainId);
9885
}
9986

87+
async getAppPolicy(appId: string): Promise<PolicyResponse> {
88+
const tcbPolicy = await this.tryReadTcbPolicy(appId);
89+
return { tcbPolicy };
90+
}
91+
92+
async getKmsPolicy(): Promise<PolicyResponse> {
93+
const tcbPolicy = await this.tryReadTcbPolicy(await this.kmsContract.getAddress());
94+
return { tcbPolicy };
95+
}
96+
10097
async getAppImplementation(): Promise<string> {
10198
return await this.kmsContract.appImplementation();
10299
}

‎kms/auth-eth/src/server.ts‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import fastify, { FastifyInstance } from 'fastify';
66
import { EthereumBackend } from './ethereum';
7-
import { BootInfo, BootResponse } from './types';
7+
import { BootInfo, BootResponse, PolicyResponse } from './types';
88
import { ethers } from 'ethers';
99

1010
declare module 'fastify' {
@@ -41,6 +41,14 @@ export async function build(): Promise<FastifyInstance> {
4141
isAllowed: { type: 'boolean' },
4242
reason: { type: 'string' },
4343
gatewayAppId: { type: 'string' },
44+
}
45+
});
46+
47+
server.addSchema({
48+
$id: 'policyResponse',
49+
type: 'object',
50+
required: ['tcbPolicy'],
51+
properties: {
4452
tcbPolicy: { type: 'string' },
4553
}
4654
});
@@ -86,8 +94,7 @@ export async function build(): Promise<FastifyInstance> {
8694
reply.code(200).send({
8795
isAllowed: false,
8896
gatewayAppId: '',
89-
reason: `${error instanceof Error ? error.message : String(error)}`,
90-
tcbPolicy: ''
97+
reason: `${error instanceof Error ? error.message : String(error)}`
9198
});
9299
}
93100
});
@@ -112,11 +119,37 @@ export async function build(): Promise<FastifyInstance> {
112119
reply.code(200).send({
113120
isAllowed: false,
114121
gatewayAppId: '',
115-
reason: `${error instanceof Error ? error.message : String(error)}`,
116-
tcbPolicy: ''
122+
reason: `${error instanceof Error ? error.message : String(error)}`
117123
});
118124
}
119125
});
120126

127+
// TCB policy endpoints
128+
server.get<{
129+
Params: { appId: string };
130+
Reply: PolicyResponse;
131+
}>('/policy/app/:appId', {
132+
schema: {
133+
response: {
134+
200: { $ref: 'policyResponse#' }
135+
}
136+
}
137+
}, async (request, reply) => {
138+
const appId = ethers.getAddress(request.params.appId);
139+
return await server.ethereum.getAppPolicy(appId);
140+
});
141+
142+
server.get<{
143+
Reply: PolicyResponse;
144+
}>('/policy/kms', {
145+
schema: {
146+
response: {
147+
200: { $ref: 'policyResponse#' }
148+
}
149+
}
150+
}, async (request, reply) => {
151+
return await server.ethereum.getKmsPolicy();
152+
});
153+
121154
return server;
122155
}

‎kms/auth-eth/src/types.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ export interface BootResponse {
1818
isAllowed: boolean;
1919
gatewayAppId: string;
2020
reason: string;
21+
}
22+
23+
export interface PolicyResponse {
2124
tcbPolicy: string;
2225
}
2326

‎kms/src/main_service.rs‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,7 @@ struct TcbPolicyDoc {
4949
///
5050
/// If `policy_json` is empty, no additional validation is performed (backward compatible
5151
/// with old contracts that don't set a policy).
52-
fn validate_onchain_tcb_policy(
53-
qvr: &QuoteVerificationResult,
54-
policy_json: &str,
55-
) -> Result<()> {
52+
fn validate_onchain_tcb_policy(qvr: &QuoteVerificationResult, policy_json: &str) -> Result<()> {
5653
if policy_json.is_empty() {
5754
return Ok(());
5855
}
@@ -68,8 +65,8 @@ fn validate_onchain_tcb_policy(
6865
return Ok(());
6966
}
7067
let policy_refs: Vec<&str> = doc.intel_qal.iter().map(|s| s.as_str()).collect();
71-
let policy_set =
72-
RegoPolicySet::new(&policy_refs).context("Failed to build RegoPolicySet from on-chain policy")?;
68+
let policy_set = RegoPolicySet::new(&policy_refs)
69+
.context("Failed to build RegoPolicySet from on-chain policy")?;
7370
let supplemental = qvr
7471
.supplemental()
7572
.context("Failed to build supplemental data for on-chain policy validation")?;
@@ -216,17 +213,15 @@ impl RpcHandler {
216213
use_boottime_mr: bool,
217214
vm_config_str: &str,
218215
) -> Result<BootConfig> {
219-
let tcb_status;
220-
let advisory_ids;
221-
match att.report.tdx_report() {
222-
Some(report) => {
223-
tcb_status = report.status;
224-
advisory_ids = report.advisory_ids;
225-
}
226-
None => {
227-
tcb_status = "".to_string();
228-
advisory_ids = Vec::new();
216+
let (tcb_status, advisory_ids) = match att.report.tdx_qvr() {
217+
Some(qvr) => {
218+
let supplemental = qvr.supplemental().context("Failed to build supplemental")?;
219+
(
220+
supplemental.tcb.status.to_string(),
221+
supplemental.tcb.advisory_ids,
222+
)
229223
}
224+
None => (String::new(), Vec::new()),
230225
};
231226
let app_info = att.decode_app_info_ex(use_boottime_mr, vm_config_str)?;
232227
let boot_info = BootInfo {
@@ -242,6 +237,23 @@ impl RpcHandler {
242237
tcb_status,
243238
advisory_ids,
244239
};
240+
241+
// Validate on-chain TCB policy before contract-level boot authorization
242+
if let Some(qvr) = att.report.tdx_qvr() {
243+
let policy = if is_kms {
244+
self.state.config.auth_api.get_kms_policy().await?
245+
} else {
246+
let app_id_hex = hex::encode(&boot_info.app_id);
247+
self.state
248+
.config
249+
.auth_api
250+
.get_app_policy(&format!("0x{app_id_hex}"))
251+
.await?
252+
};
253+
validate_onchain_tcb_policy(qvr, &policy.tcb_policy)
254+
.context("On-chain TCB policy check failed")?;
255+
}
256+
245257
let response = self
246258
.state
247259
.config
@@ -251,11 +263,6 @@ impl RpcHandler {
251263
if !response.is_allowed {
252264
bail!("Boot denied: {}", response.reason);
253265
}
254-
// Apply on-chain TCB policy (if set) to the TDX QuoteVerificationResult
255-
if let Some(qvr) = att.report.tdx_qvr() {
256-
validate_onchain_tcb_policy(qvr, &response.tcb_policy)
257-
.context("On-chain TCB policy check failed")?;
258-
}
259266
self.verify_os_image_hash(vm_config_str.into(), att)
260267
.await
261268
.context("Failed to verify os image hash")?;

‎kms/src/main_service/upgrade_authority.rs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ pub(crate) struct BootResponse {
3939
pub is_allowed: bool,
4040
pub gateway_app_id: String,
4141
pub reason: String,
42+
}
43+
44+
#[derive(Debug, Serialize, Deserialize)]
45+
#[serde(rename_all = "camelCase")]
46+
pub(crate) struct PolicyResponse {
4247
#[serde(default)]
4348
pub tcb_policy: String,
4449
}
@@ -92,7 +97,6 @@ impl AuthApi {
9297
is_allowed: true,
9398
reason: "".to_string(),
9499
gateway_app_id: dev.gateway_app_id.clone(),
95-
tcb_policy: String::new(),
96100
}),
97101
AuthApi::Webhook { webhook } => {
98102
let path = if is_kms {
@@ -106,6 +110,30 @@ impl AuthApi {
106110
}
107111
}
108112

113+
pub async fn get_app_policy(&self, app_id: &str) -> Result<PolicyResponse> {
114+
match self {
115+
AuthApi::Dev { .. } => Ok(PolicyResponse {
116+
tcb_policy: String::new(),
117+
}),
118+
AuthApi::Webhook { webhook } => {
119+
let url = url_join(&webhook.url, &format!("policy/app/{app_id}"));
120+
http_get(&url).await
121+
}
122+
}
123+
}
124+
125+
pub async fn get_kms_policy(&self) -> Result<PolicyResponse> {
126+
match self {
127+
AuthApi::Dev { .. } => Ok(PolicyResponse {
128+
tcb_policy: String::new(),
129+
}),
130+
AuthApi::Webhook { webhook } => {
131+
let url = url_join(&webhook.url, "policy/kms");
132+
http_get(&url).await
133+
}
134+
}
135+
}
136+
109137
pub async fn get_info(&self) -> Result<GetInfoResponse> {
110138
match self {
111139
AuthApi::Dev { dev } => Ok(GetInfoResponse {

‎verifier/src/verification.rs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -419,9 +419,11 @@ impl CvmVerifier {
419419
let verified_attestation = match verified {
420420
Ok(att) => {
421421
details.quote_verified = true;
422-
if let Some(report) = att.report.tdx_report() {
423-
details.tcb_status = Some(report.status);
424-
details.advisory_ids = report.advisory_ids;
422+
if let Some(qvr) = att.report.tdx_qvr() {
423+
if let Ok(supplemental) = qvr.supplemental() {
424+
details.tcb_status = Some(supplemental.tcb.status.to_string());
425+
details.advisory_ids = supplemental.tcb.advisory_ids;
426+
}
425427
}
426428
details.report_data = Some(hex::encode(att.report_data));
427429
att
@@ -508,15 +510,17 @@ impl CvmVerifier {
508510
debug: bool,
509511
details: &mut VerificationDetails,
510512
) -> Result<()> {
511-
let Some(tdx_report) = attestation.report.tdx_report() else {
512-
bail!("No TDX report");
513-
};
513+
let qvr = attestation
514+
.report
515+
.tdx_qvr()
516+
.context("No TDX QuoteVerificationResult")?;
517+
let supplemental = qvr.supplemental().context("Failed to build supplemental")?;
514518
let Some(tdx_quote) = attestation.tdx_quote() else {
515519
bail!("No TDX quote");
516520
};
517521
let event_log = &tdx_quote.event_log;
518522
// Get boot info from attestation
519-
let report = tdx_report
523+
let report = supplemental
520524
.report
521525
.as_td10()
522526
.context("Failed to decode TD report")?;

0 commit comments

Comments
 (0)