Conversation
kvinwang
changed the title
SupplementalData is no longer built during verify() — it's computed on demand via result.supplemental() or internally by validate(). This keeps the verify() path minimal (crypto only), saving ~2 TGas on NEAR (175 TGas, matching pre-policy baseline). - QuoteVerificationResult stores verification intermediates privately - supplemental() method builds the full struct lazily (root_key_id, CRL numbers, tcb_date_tag, earliest_expiration) - into_report() works without ever constructing SupplementalData - Rego path calls supplemental() internally when needed
Rego validation was parsing TcbInfo + QeIdentity JSON twice: once in supplemental() → compute_earliest_expiration(), and again in compute_time_window(). Now Rego path uses build_supplemental(tw.earliest_expiration_date) to reuse the value from compute_time_window() directly.
- Split monolithic src/policy.rs into policy/{mod,simple,rego}.rs
- Rename QuotePolicy to SimplePolicy for clarity
- Advisory skip only during platform grace for pure OutOfDate
(collateral grace and OutOfDateConfigurationNeeded don't skip)
- Update all imports and re-exports across lib, cli, verify
- Create docs/policy.md with comprehensive policy validation guide - Rename into_report() to into_report_unchecked() with warning docs - Add doc comments to CollateralTimeWindow fields - Update README with two-phase API example and policy section
Resolve conflicts: - src/lib.rs: keep pub visibility for qe_identity and tcb_info modules - src/verify.rs: keep QuoteVerificationResult return type, remove convenience verify() functions (policy branch design), retain danger-allow-tcb-override in verify_impl and QuoteVerifier
…C FFI - Move CollateralTimeWindow fields into SupplementalData (8-source time window) - Add Report to SupplementalData so Policy impls have full context - impl Policy for RegoPolicy and RegoPolicySet (replaces separate eval methods) - Add SimplePolicyConfig (JSON-deserializable) for building SimplePolicy - Python bindings: two-phase API (verify → QVR → validate(policy) → VerifiedReport) - C FFI: keep flat dcap_verify_cb/dcap_verify_with_root_ca_cb (no policy in ABI) - Go bindings unchanged from master
…den_* dates
- Register rand.intn as regorus extension with real RNG (getrandom) and
per-evaluation memoization matching OPA semantics (cache key = "{str}-{n}")
- Switch Rego query from final_ret to final_appraisal_result (the standard
Intel QAL output format with nonce, timestamp, appraised_reports)
- Add qe_iden_earliest_issue_date, qe_iden_latest_issue_date,
qe_iden_earliest_expiration_date to SupplementalData, computed from
QE Identity issuer chain + JSON (sources [5]+[7]), matching Intel's
qve_get_collateral_dates() lines 94-96
- QE measurement now uses QE-specific time window instead of global dates
kvinwang
changed the title
The two-phase API refactor removed these functions. Re-add them using QuoteVerifier + into_report_unchecked(), matching the test harness behavior. Add rustcrypto to the js feature since CryptoBackend is now required.
kvinwang
changed the title
…icationResult
Replace flat js_verify/js_verify_with_root_ca/js_get_collateral globals
with class-based API matching the Rust two-phase pattern:
const verifier = new QuoteVerifier(); // or new QuoteVerifier(rootCaDer)
const result = verifier.verify(quote, collateral, now);
const report = result.validate(policy); // or result.into_report_unchecked()
const policy = new SimplePolicy(now)
.allow_status("OutOfDate")
.allow_smt(true);
const collateral = await QuoteVerifier.get_collateral(pccsUrl, quote);
Add rustcrypto to js feature since CryptoBackend is now required.
…sgx_types - Fix get_collateral_web.js, esbuild/main.ts, vite/main.ts to use QuoteVerifier class instead of deleted js_verify/js_get_collateral - Add accepted_sgx_types() to JsSimplePolicy (parity with Python) - Change example files to use validate(policy) as default path instead of into_report_unchecked() - Fix rustfmt formatting in get_collateral and js_parse_tcb_status
…esult Without js_class matching the js_name on the struct, wasm_bindgen doesn't attach methods to the renamed JS class.
Move the collateral_grace_period vs platform/qe_grace_period mutual exclusivity check to the top of SimplePolicy::validate(), before any data-dependent checks run. Previously it was checked after the TCB status check, meaning a misconfigured policy would run partial validation before failing on the config error.
OPA's builtinRandIntn returns 0 for n==0 and uses abs(n) for negative values. Our implementation was returning 0 for all n<=0, diverging from OPA's behavior for negative n.
The collateral_grace_period, platform_grace_period, and qe_grace_period are orthogonal checks (time-based vs version-based). Intel's Rego script enforces mutual exclusivity as a simplification, but it's not a logical necessity. Allow all three to be set independently.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
6 tasks
Lightweight accessor that avoids cloning the entire QVR or calling into_report_unchecked() just to read the PPID.
Comment on lines
+438
to
+440
| accepted_tcb_level_date_tag_ok(bundle) if { | ||
| not bundle.policy.sgx_platform.reference.min_tcb_level_date | ||
| } |
There was a problem hiding this comment.
Bug: The Rego rule accepted_tcb_level_date_tag_ok uses an incorrect path, causing the min_tcb_level_date policy validation to be bypassed entirely.
Severity: HIGH
Suggested Fix
In rego/qal_script.rego, change the path bundle.policy.sgx_platform.reference.min_tcb_level_date to bundle.policy.reference.min_tcb_level_date to match the correct policy data structure.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: rego/qal_script.rego#L438-L440
Potential issue: In the Rego script, the rule `accepted_tcb_level_date_tag_ok` uses an
incorrect object path `bundle.policy.sgx_platform.reference.min_tcb_level_date` to
access a policy setting. The correct path is
`bundle.policy.reference.min_tcb_level_date`. Because the incorrect path is always
undefined in Rego, the `not` expression always evaluates to true. This causes the rule
to always succeed, effectively disabling the policy constraint that is supposed to
enforce a minimum TCB level date. As a result, platform TCB measurements with outdated
dates will pass validation when they should be rejected.
Comment on lines
+282
to
+283
| "appraisal_result": appraise_ret, | ||
| "report": {"environment": item.report.environment, "measurement": item.report.measurement}, |
There was a problem hiding this comment.
Bug: The Rego script uses incorrect paths (item.report.*) to access fields for unmatched reports, leading to malformed diagnostic output.
Severity: MEDIUM
Suggested Fix
In the appraisal_result rule that handles report_not_in_policy, change the output mapping from {"environment": item.report.environment, "measurement": item.report.measurement} to {"environment": item.environment, "measurement": item.measurement}.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: rego/qal_script.rego#L282-L283
Potential issue: When generating an appraisal result for a report that does not match
any policy, the Rego script attempts to access `item.report.environment` and
`item.report.measurement`. However, the `item` in this context is a raw report entry
from `report_not_in_policy` and does not have a nested `report` field. The correct paths
are `item.environment` and `item.measurement`. This error causes the resulting
`appraisal_output` object to contain an incomplete or empty `report` field, breaking the
diagnostic reporting for unmatched reports.
Comment on lines
+797
to
+801
| isvfamilyid_ok(bundle) if { | ||
| is_string(bundle.report.measurement.sgx_isvfamilyid) | ||
| is_string(bundle.policy.reference.sgx_isvfamilyid) | ||
| lower(bundle.report.measurement.sgx_isvfamilyid) == bundle.policy.reference.sgx_isvfamilyid | ||
| } |
There was a problem hiding this comment.
Bug: The isvfamilyid_ok rule performs a case-sensitive comparison by failing to apply lower() to the policy value, unlike other similar rules.
Severity: MEDIUM
Suggested Fix
In the isvfamilyid_ok rule, apply the lower() function to the policy value to ensure a case-insensitive comparison. Change bundle.policy.reference.sgx_isvfamilyid to lower(bundle.policy.reference.sgx_isvfamilyid).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: rego/qal_script.rego#L797-L801
Potential issue: The Rego rule `isvfamilyid_ok` performs a case-sensitive comparison
between the `sgx_isvfamilyid` from the report and the policy. It converts the report
value to lowercase using `lower()` but fails to do the same for the policy value
`bundle.policy.reference.sgx_isvfamilyid`. This is inconsistent with all other similar
hexadecimal field comparisons in the same file, which apply `lower()` to both sides.
This will cause validation to fail incorrectly if a user provides an uppercase or
mixed-case `sgx_isvfamilyid` in their policy, as the report value is always uppercase
before being lowercased for comparison.
src/verify.rs
Outdated
Comment on lines
+769
to
+773
| let pce_id = match pck_ext.pce_id.as_slice() { | ||
| [hi, lo] => u16::from_be_bytes([*hi, *lo]), | ||
| [lo] => u16::from(*lo), | ||
| _ => 0, | ||
| }; |
There was a problem hiding this comment.
Bug: The pce_id is parsed as big-endian using from_be_bytes, but the Intel specification defines it as little-endian, leading to incorrect ID values.
Severity: HIGH
Suggested Fix
In src/verify.rs, change the pce_id conversion from u16::from_be_bytes to u16::from_le_bytes to correctly parse the little-endian value as defined by the Intel specification.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/verify.rs#L769-L773
Potential issue: The `pce_id` field from the PCK certificate is parsed as a big-endian
value using `u16::from_be_bytes`. However, the Intel specification and comments
elsewhere in the codebase confirm that `pce_id` is encoded in little-endian format. This
discrepancy will cause an incorrect `pce_id` value to be calculated and stored in the
`QuoteVerificationResult` for any certificate where the ID is not symmetrical (e.g., a
little-endian value of 1, `[0x01, 0x00]`, would be incorrectly parsed as 256). This
incorrect data is then exposed to consumers and could cause failures in Rego policy
evaluations that rely on this value.
Comment on lines
+743
to
+745
| let result = self | ||
| .inner | ||
| .take() |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Comment on lines
+560
to
+567
| fn validate(&self, data: &SupplementalData) -> Result<()> { | ||
| let measurement = build_merged_measurement(data); | ||
| let qvl_result = vec![json!({ | ||
| "environment": { "class_id": &self.class_id }, | ||
| "measurement": measurement, | ||
| })]; | ||
| eval_rego_engine(&self.engine, &[&self.policy_json], qvl_result) | ||
| } |
There was a problem hiding this comment.
Bug: RegoPolicy::validate() incorrectly uses a merged TCB status for evaluation, rather than selecting the appropriate measurement based on the policy's class_id, causing inconsistent validation results.
Severity: HIGH
Suggested Fix
Modify RegoPolicy::validate() to check the self.class_id and invoke the appropriate measurement builder function (build_platform_measurement, build_qe_measurement, or tenant_measurement), mirroring the logic currently used in to_rego_qvl_result.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/policy/rego.rs#L560-L567
Potential issue: The `RegoPolicy::validate()` function incorrectly uses a merged TCB
status for all policy evaluations by always calling `build_merged_measurement(data)`.
This function combines the platform and QE TCB statuses. However, the validation should
be specific to the policy's `class_id`, as is correctly handled in
`RegoPolicySet::validate()`, which uses different measurement builders
(`build_platform_measurement`, `build_qe_measurement`) accordingly. This discrepancy
leads to incorrect validation outcomes. For example, a platform policy requiring an
`UpToDate` status will be incorrectly rejected if the platform TCB is `UpToDate` but the
QE TCB is `OutOfDate`, because the merged status will be `OutOfDate`. This creates
inconsistent behavior between the `RegoPolicy` and `RegoPolicySet` APIs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces two-phase quote verification: crypto verification (signature chain, TCB matching) followed by policy validation (business rules).
Built-in policy options:
SimplePolicy— Rust-native builder API for straightforward use without external dependenciesRegoPolicy/RegoPolicySet— evaluates Intel'sqal_script.regowith Intel QAL JSON policies for compatibility with Intel's Quote Appraisal frameworkThis PR also exposes
RegoPolicy/RegoPolicySetto Python and WASM/JS bindings.Examples
SimplePolicy (Rust-native, no Rego)
RegoPolicy (Intel QAL JSON, feature
rego)Core Architecture
Policytrait — pluggable validation interface forSupplementalDataSimplePolicy— built-in Rust-native policy covering the common appraisal checks without requiring a Rego engine: TCB status, advisory ID blacklist, collateral grace period, platform grace period, QE grace period, min eval number, dynamic platform, cached keys, SMT, and SGX typesQuoteVerifier::verify()→QuoteVerificationResult(crypto only) →validate(&policy)→VerifiedReportQuoteVerificationResult::supplemental()exposes the computed supplemental data for inspection before committing to a policyRego Policy Support (
regofeature)RegoPolicy— evaluates Intel'sqal_script.regovia theregorusRego interpreterRegoPolicySet— multi-measurement appraisal matching Intel's QAL structureclass_idrand.intnextension — RNG + memoization matching OPA semanticsfinal_appraisal_resultquery — uses Intel's standard appraisal output formatSupplementalData
sgx_ql_qv_supplemental_tfields, plusqe_reportandqe_tcb_eval_data_numberfor multi-measurement supportqe_iden_earliest_issue_date,qe_iden_latest_issue_date,qe_iden_earliest_expiration_date) matching Intel'sqve_get_collateral_dates()behaviorplatform_tcb_levelqe_tcb_level+ QE-specific datesOther Changes
TcbStatusmerge uses Intel's exactconvergeTcbStatusWithQeTcbStatuslogicSupplementalDatatime window computed from 8 sourcesroot_key_idcomputed as SHA-384 of raw public key bytesdynamic_platform,cached_keys,smt_enabled) distinguishFalsefromUndefinedQuoteVerifier,SimplePolicy, andQuoteVerificationResultsrc/policy/as dedicated module withmod.rs,simple.rs,rego.rsTest plan
cargo test --all-featurescargo clippy --all-features -- -D warningscargo build --target wasm32-unknown-unknown --features jsRegoPolicy/RegoPolicySetbindings andSimplePolicyadvisory blacklist behavior