feat(oid4vc): mDoc support, OID4VCI 1.0 and DCQL#2871
feat(oid4vc): mDoc support, OID4VCI 1.0 and DCQL#2871burdettadam wants to merge 1 commit intoopenwallet-foundation:mainfrom
Conversation
🔑 Request for Feedback: Key Management, Trust Anchor, and Trust Registry ArchitectureThis PR introduces a new trust infrastructure layer for the Current ArchitectureTrust Anchor Management
Signing Key Management
Signing Key Resolution OrderIn
Verification Flow
Design Decisions We'd Like Feedback On
Any feedback on these points — or anything else in the trust infrastructure — would be greatly appreciated. |
|
@burdettadam This looks great on first glance. Thank you. Answers to the questions from my perspective.
5 and 6. @weiiv can you provide some feedback. Comments:
Has this PR been tested with Bifold (3.0) or Animo Paradym wallet yet? I will try the demo ASAP. Getting some errors on initial start. |
b1109ab to
7df101c
Compare
…efactoring This PR brings the Indicio-tech fork changes into upstream as a single squashed commit with a clean git history. ## Summary - mDoc / mso_mdoc: Full mDoc credential format support using isomdl-uniffi, trust anchor registry, signing key management, status list revocation, and presentation verification - OID4VCI 1.0 Compliance: Token endpoint updated to final spec + HAIP profile, DPoP support, backward compatibility for draft clients, OAuth discovery endpoint - OID4VP / Verification: OID4VP Final with x5c key binding, JAR fixes, did:jwk client_id, UUID4 presentation definition IDs - DCQL: Expanded query language support with multi-credential flows - Routes Refactoring: Split monolithic public_routes.py and routes.py into focused submodules - SD-JWT VC and JWT VC JSON: Selective disclosure fixes, OID4VCI 1.0 pattern alignment - status_list: Endianness fix Note: Demo, integration tests, unit tests, CI workflows, docker files, poetry.lock files, and changes to non-oid4vc plugins are excluded from this PR and will be submitted separately. Signed-off-by: Adam Burdett <burdettadam@gmail.com>
7df101c to
b802b07
Compare
|
@timbl-ont Thank you for the detailed feedback — really helpful context on where things are heading. I've reduced the draft PR to just the core source changes (no demo, integration tests, or CI workflows) to make it easier to review the functional changes in isolation. It sounds like we've diverged a bit on testing strategies — I think we can find common ground once we align on the core implementation direction. Responding to your answers: On HSM support (Q1 & Q4): Good to know PKCS#11 is the target interface. Rather than blocking the core mDoc functionality on that decision, I'm actively updating this draft to introduce a pluggable signing backend interface that allows Key-ID-only storage in Askar and delegates signing to an external HSM — so both models are supported without either blocking the other. On key curves (Q3): Agreed — P-256 is the right default for mDL/AMVAA compliance but import validation should be flexible. I'll add curve validation on import, with Ed25519 as a near-term target and the Longfellow ZKP / post-quantum curves tracked for the future. On immutability (Q2): Makes sense that HSM workflows may require re-import of a certificate against an existing key ID. I'll keep immutable-by-default but add a certificate re-import path. On the attestation PR: Could you share a link to your draft PR? I'd like to track it directly and rebase as needed rather than risk conflicts at merge time. On dPOP: We already have dPOP support wired in. The token endpoint (public_routes/token.py) accepts both On wallet testing: We've had success with Credo and Sphereon. Interoperability always comes with caveats depending on which draft of OID4VCI/OID4VP the wallet implements. Bifold and Paradym are on our list. Action items:
|
|
@burdettadam The attestation PR is here in draft: #2970. Note that this PR updates the standalone auth server and not the token endpoint in the issuer plugin. The decoupling of the auth server (/token) is to support enterprise deployments that may already have an existing IAM solution. As the major vendors don't yet support openid4vci pre-authorized code flow a reference authserver is included to demonstrate how the integration should work and to bridge the gap. @jamshale I think the order of operations is that this PR is merged first and then the draft PR from @weiiv. It looks like the poetry.lock needs to be updated to run the tests. FYI @mepeltier |
|
Hi @mepeltier, while testing this PR we found a few small fixes needed for SD-JWT VC. Happy to open a follow-up PR if preferred.
@@ -157,12 +157,13 @@ async def supported_credential_create(request: web.Request):
body,
)
- vc_additional_data = {
- "vct": body.pop("vct", None),
- "sd_list": body.pop("sd_list", None),
- }
+ vct = body.pop("vct", None)
+ sd_list = body.pop("sd_list", None)
+ format_data = {"vct": vct} if vct is not None else {}
+ vc_additional_data = {"vct": vct, "sd_list": sd_list}
record = SupportedCredential(
**body,
+ format_data=format_data,
vc_additional_data=vc_additional_data,
)
@@ -213,10 +214,10 @@ async def supported_cred_update_helper(
)
record.proof_types_supported = body.get("proof_types_supported", None)
record.credential_metadata = body.get("credential_metadata", None)
- record.vc_additional_data = {
- "vct": body.get("vct", None),
- "sd_list": body.get("sd_list", None),
- }
+ vct = body.get("vct", None)
+ sd_list = body.get("sd_list", None)
+ record.format_data = {"vct": vct} if vct is not None else {}
+ record.vc_additional_data = {"vct": vct, "sd_list": sd_list}
await record.save(session)
return record
@@ -194,8 +194,8 @@ class SdJwtCredIssueProcessor(Issuer, CredVerifier, PresVerifier):
"""Validate the credential subject."""
vc_additional = supported.vc_additional_data
assert vc_additional
- assert supported.format_data
- claims_metadata = supported.format_data.get("claims")
+ # assert supported.format_data
+ claims_metadata = supported.credential_metadata.get("claims")
sd_list = vc_additional.get("sd_list") or []
For strict OID4VCI 1.0 compliance. Note this may break wallets that only read the top-level @@ -296,12 +296,9 @@ async def _issue_cred_inner(context, token_result, refresh_id, req_body):
await ex_record.save(session, reason="Credential issued")
# OID4VCI 1.0 §7.3.1: response MUST contain `credentials` (array) or `transaction_id`.
- # Also include the singular `credential` key (draft-era field) for wallets such as
- # waltid that still parse only the top-level `credential` field and NPE when absent.
- cred_response: dict = {
- "credential": credential,
- # OID4VCI 1.0 §7.3.1: each entry in `credentials` MUST include `format`.
- "credentials": [{"format": supported.format, "credential": credential}],
+ # Only return the `credentials` array (OID4VCI 1.0), not the legacy `credential` key.
+ cred_response = {
+ "credentials": [{"format": supported.format, "credential": credential}]
}
if ex_record.notification_id:
cred_response["notification_id"] = ex_record.notification_id |
|
How about @weiiv and/or @timbl-ont approve the PR, and then I'll approve and merge after that. It is still a work in progress, but if you can clean it up after, we can go from there. Reasonable? |
|
Sounds good -- I'll watch for @weiiv 's approval. |
This PR brings the Indicio-tech fork changes into upstream as a single squashed commit with a clean git history.
Summary
317 files changed, 60,129 insertions(+), 24,141 deletions(-)