Implement basic unit/instance attestation by md4096 · Pull Request #487 · a-sit-plus/vck

md4096 · 2026-01-21T12:24:59Z

Related:
a-sit-plus/signum#400
a-sit-plus/valera#419

openid-data-classes/src/commonMain/kotlin/at/asitplus/openid/KeyAttestationRequired.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

openid-data-classes/src/commonMain/kotlin/at/asitplus/openid/KeyAttestationRequired.kt

vck-openid-ktor/src/commonMain/kotlin/at/asitplus/wallet/lib/ktor/openid/OAuth2KtorClient.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oauth2/SimpleAuthorizationService.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialIssuer.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/ProofValidator.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

nodh · 2026-02-12T14:25:13Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4d86a20b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oauth2/SimpleAuthorizationService.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/ProofValidator.kt

chatgpt-codex-connector · 2026-02-19T09:39:42Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

nodh · 2026-02-19T11:09:14Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e46e847869

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

vck-openid-ktor/src/commonMain/kotlin/at/asitplus/wallet/lib/ktor/openid/OAuth2KtorClient.kt

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

nodh · 2026-02-20T10:08:57Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f5965ae9f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-02-20T10:14:15Z

vck-openid-ktor/src/commonMain/kotlin/at/asitplus/wallet/lib/ktor/openid/OAuth2KtorClient.kt

+        val clientAttJwt = loadInstanceAttestation?.let { it().getOrNull()?.serialize() }
+        val clientAttPop = loadInstanceAttestationPop?.let { it().getOrNull()?.serialize() }


Preserve deprecated client attestation auth path

This code now always reads instance attestation headers from loadInstanceAttestation*, but the constructor still accepts loadClientAttestationJwt/signClientAttestationPop (marked deprecated, not removed). Callers that have not migrated yet will compile and instantiate this class, but token requests will silently omit attestation headers and fail with invalid_client against servers requiring attestation-based client auth.

Useful? React with 👍 / 👎.

Would be fine to use the fallback:

val clientAttJwt = loadInstanceAttestation?.let { it().getOrNull()?.serialize() } ?: loadClientAttestationJwt?.invoke()

chatgpt-codex-connector · 2026-02-20T10:14:15Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

+        if (addKeyAttestation && loadUnitAttestation == null && loadKeyAttestation == null) {
+            throw IllegalArgumentException("Key attestation required, none provided")
+        }
+        return CredentialRequestProofContainer(
+            jwt = setOf(loadUnitAttestation?.invoke(


Honor legacy key attestation loader when required

When addKeyAttestation is true, the guard explicitly treats loadKeyAttestation as a valid source, but the JWT proof creation path never uses it and only calls loadUnitAttestation. If a wallet still provides only loadKeyAttestation (which is not deprecated here), this branch will pass the precondition and then emit a fallback proof without key attestation, causing issuer-side proof rejection.

Useful? React with 👍 / 👎.

Also sounds fine to add a fallback to loadKeyAttestation when loadUnitAttestation is null.

chatgpt-codex-connector · 2026-02-20T10:14:15Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

+                        nonce = clientNonce,
+                    )
+                )
+            )?.getOrNull()?.serialize() ?: fallbackProof(clientNonce, credentialIssuer))


Do not swallow unit attestation generation errors

Using getOrNull() here hides failures from loadUnitAttestation and silently falls back to fallbackProof. In key-attestation-required flows this converts a concrete attestation acquisition error into a generic downstream invalid_proof/issuance failure, making recovery and debugging much harder and sending a request that is known to violate issuer requirements.

Useful? React with 👍 / 👎.

Seems about right, because addKeyAttestation might be true but the call returns an error ... right @md4096?

nodh · 2026-02-20T10:13:25Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialIssuer.kt

+            when (tokenStatusValid && signatureValid) {
+                true -> return@ProofValidator true
+                false -> return@ProofValidator false


It looks like this could be simplified to tokenStatusValid && signatureValid

nodh · 2026-02-20T10:14:00Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

    /** Handles credential request encryption and credential response decryption. */
    private val encryptionService: WalletEncryptionService = WalletEncryptionService(),
+    /** Returns a new unit attestation to use during credential issuance. */
+    val loadUnitAttestation: (suspend (input: LoadUnitAttestationInput) -> KmmResult<JwsSigned<JsonWebToken>>)? = null,


Why isn't that private?

nodh · 2026-02-20T10:14:48Z

CHANGELOG.md

 - ISO mdoc:
   - Preserve `Document.errors` in parsed ISO document results instead of failing validation
 - Change: Executing unsatisfiable DCQL queries no longer throws on matching, only on submission.
+ - Add method `loadUnitAttestation` to `WalletService`


Please group your entries under OpenID for Verifiable Credential Issuance:

nodh · 2026-02-20T10:18:57Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialIssuer.kt

+                    }
+                }
+            }.isSuccess
+            val signatureValid = true //TODO: Add list with trusted keys


Even now we should at least verify the cryptographic signature of the JWS, and simply trust all keys.

nodh · 2026-02-20T10:19:42Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

+    /** Returns a new unit attestation to use during credential issuance. */
+    val loadUnitAttestation: (suspend (input: LoadUnitAttestationInput) -> KmmResult<JwsSigned<JsonWebToken>>)? = null,
+
+    private val keyMaterial: KeyMaterial = EphemeralKeyWithoutCert(),


Please move that up the parameter list, to its original position

nodh · 2026-02-20T10:21:28Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

                    issuer = clientId, // omit when token was pre-authn?
                    audience = credentialIssuer,
-                    issuedAt = clock.now().truncateToSeconds(),
+                    issuedAt = Clock.System.now().truncateToSeconds(),


should add a clock parameter to that function

nodh · 2026-02-20T10:21:49Z

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt

+        }
+        return CredentialRequestProofContainer(
+            jwt = setOf(loadUnitAttestation?.invoke(
+                LoadUnitAttestationInput(


Seems like duplicated code from createCredentialRequestProofAttestation()

md4096 self-assigned this Jan 21, 2026

This was referenced Jan 21, 2026

Implement basic unit/instance attestation a-sit-plus/valera#419

Draft

Add data fields for Instance/Unit Attestation a-sit-plus/signum#400

Merged

md4096 force-pushed the feature/wia-wua branch from 190b492 to 829a0bb Compare January 21, 2026 15:13

nodh reviewed Jan 22, 2026

View reviewed changes

openid-data-classes/src/commonMain/kotlin/at/asitplus/openid/KeyAttestationRequired.kt Outdated Show resolved Hide resolved

nodh reviewed Jan 22, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt Outdated Show resolved Hide resolved

md4096 force-pushed the feature/wia-wua branch from 87158d8 to 8c5dd46 Compare January 22, 2026 12:47

md4096 force-pushed the feature/wia-wua branch from 8c5dd46 to f87d2cc Compare February 3, 2026 13:56

nodh reviewed Feb 3, 2026

View reviewed changes

openid-data-classes/src/commonMain/kotlin/at/asitplus/openid/KeyAttestationRequired.kt Show resolved Hide resolved

nodh reviewed Feb 3, 2026

View reviewed changes

vck-openid-ktor/src/commonMain/kotlin/at/asitplus/wallet/lib/ktor/openid/OAuth2KtorClient.kt Show resolved Hide resolved

nodh reviewed Feb 3, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt Outdated Show resolved Hide resolved

nodh reviewed Feb 3, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt Outdated Show resolved Hide resolved

md4096 force-pushed the feature/wia-wua branch 2 times, most recently from c4550f7 to 8130d1d Compare February 4, 2026 13:44

nodh reviewed Feb 4, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oauth2/SimpleAuthorizationService.kt Outdated Show resolved Hide resolved

nodh reviewed Feb 4, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialIssuer.kt Outdated Show resolved Hide resolved

nodh reviewed Feb 4, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/ProofValidator.kt Outdated Show resolved Hide resolved

nodh reviewed Feb 4, 2026

View reviewed changes

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt Outdated Show resolved Hide resolved

md4096 force-pushed the feature/wia-wua branch from 156f8f7 to ce57bdf Compare February 11, 2026 13:55

chatgpt-codex-connector bot reviewed Feb 12, 2026

View reviewed changes

md4096 force-pushed the feature/wia-wua branch from c4d86a2 to ed51cba Compare February 19, 2026 09:37

md4096 marked this pull request as ready for review February 19, 2026 09:39

md4096 force-pushed the feature/wia-wua branch 2 times, most recently from 5657425 to e46e847 Compare February 19, 2026 09:59

chatgpt-codex-connector bot reviewed Feb 19, 2026

View reviewed changes

vck-openid-ktor/src/commonMain/kotlin/at/asitplus/wallet/lib/ktor/openid/OAuth2KtorClient.kt Outdated Show resolved Hide resolved

vck-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt Outdated Show resolved Hide resolved

md4096 force-pushed the feature/wia-wua branch 2 times, most recently from 38b704f to 49bf8e8 Compare February 20, 2026 09:44

Implement basic unit/instance attestation

0f5965a

md4096 force-pushed the feature/wia-wua branch from 49bf8e8 to 0f5965a Compare February 20, 2026 09:57

chatgpt-codex-connector bot reviewed Feb 20, 2026

View reviewed changes

nodh requested changes Feb 20, 2026

View reviewed changes

Update CHANGELOG.md

09bdc97

		val clientAttJwt = loadInstanceAttestation?.let { it().getOrNull()?.serialize() }
		val clientAttPop = loadInstanceAttestationPop?.let { it().getOrNull()?.serialize() }

Conversation

md4096 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodh commented Feb 12, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector bot commented Feb 19, 2026

Uh oh!

nodh commented Feb 19, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

nodh commented Feb 20, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Feb 20, 2026

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

md4096 commented Jan 21, 2026 •

edited

Loading