Specify value-matching for ISO mdoc-based credentials. #481
Conversation
davidz25
force-pushed
the
issue-420-value-matching-cbor
branch
2 times, most recently
from
d76936f to
1ea1de7
Compare
| is followed. Therefore, Verifiers MUST treat restrictions expressed using `values` as a | ||
| best-effort way to improve user privacy, but MUST NOT rely on it for security checks. | ||
|
|
||
| If a Wallet implements value matching and the credential to match against is |
There was a problem hiding this comment.
The text is fine, but this should be moved below the values definition in Line 917. The processing rules in the current section define details on selecting claims and credentials, it's not a perfect fit here.
|
@davidz25 would it be possible to incorporate changes, if you agree with them, before monday hybrid dcp wg meeting, if possible? thank you! |
| For example if the value to match against is `2025-03-31` both the values CBOR | ||
| values (in diagnostic notation) `"2025-03-31"` and `#6.1004("2025-03-31")` would | ||
| match. | ||
| - If the value to match against is a integer, only claims encoded as unsigned |
There was a problem hiding this comment.
This text allows negative integers, however the definition of 'values' says that it can "strings, integers or boolean values". Looking at the JSON RFC, integer is defined as "int = zero / ( digit1-9 *DIGIT )" which does not include negative numbers.
I'm not sure which of this was the intention, but with the proposed text there is misalignment between the two formats.
There was a problem hiding this comment.
Hmm, yeah, this is a problem. I think it's possible someone someday creates an ISO mdoc DocType with data elements with negative integers, it's not even that far-fetched... are people really not using negative integers in JSON documents?
There was a problem hiding this comment.
I don't think we can easily change the JSON RFC 🤣 ... so maybe we should also allow e.g. strings "42" and "-42" to also match CBOR major type 0 and 1? I think that would be pretty natural and this way users can use "-42" to match -42 as a CBOR value... it's not pretty but it will work. Thoughts?
| match. | ||
| - If the value to match against is a integer, only claims encoded as unsigned | ||
| integers (major type 0), negative integers (major type 1), and `bignum` (as per | ||
| section 3.4.3 of [@RFC8949]) are considered. The CBOR value must match the |
There was a problem hiding this comment.
We should say that when one of the major types (0 and 1) is tagged, those tags should be ignored as well.
There was a problem hiding this comment.
Should we though? Are there any real use-cases when you use a tagged integer and you then wish to compare against it? Only thing I can think about right now is Epoch-based time and I think for that you want to use the functions that @c2bo talked about during IIW this week for 1.1 so you can say e.g. fn_to_datestring() which returns e.g. an ISO 8601 datestring you can match against.
Either way I guess I'm fine with saying those tags are ignored, I don't think it costs anything. I'll incorporate that change and also add it for booleans too.
| best-effort way to improve user privacy, but MUST NOT rely on it for security checks. | ||
|
|
||
| If a Wallet implements value matching and the credential to match against is | ||
| an ISO mdoc-based credential, the following rules apply: |
There was a problem hiding this comment.
Currently there are a number of items missing that can be encoded in CBOR. Some of these are probably not too critical (e.g. the 'undefined' item), others I'm not sure of (floating points, null), but make sense to have. But most importantly byte strings should be included, as this is a major tag that otherwise does not have a way to be matched against.
There was a problem hiding this comment.
I think we can always add support for matching more values in 1.1, right?
For floating point comparison, this could be especially tricky when you only have == so that's why I didn't include this originally. Comparison against bstr seems weird, when would you ever do this? I guess it's possible someone could design a doctype that encodes e.g. bitmasks using a bstr but I think that's just bad doctype design...
There was a problem hiding this comment.
I think bytestring is important to have as that’s a fairly common type.
It may require a normative change in how to identify the type in the request, so we should do that now instead of later.
There was a problem hiding this comment.
Other solution would be to match against encoded cbor for all cases?
davidz25
force-pushed
the
issue-420-value-matching-cbor
branch
from
145f2ca to
3334007
Compare
|
Updated PR to incorporate Kristina's changes and Daniel's suggestion of moving it to below values. Still need to address Martijn's comments, doing that now. |
Fixes issue openid#420. Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com> Signed-off-by: David Zeuthen <zeuthen@google.com>
davidz25
force-pushed
the
issue-420-value-matching-cbor
branch
from
3334007 to
d4bb318
Compare
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
|
WG discussion.
|
|
Superseded by #538 |
) * Specifies value matching for mdocs via a reference to cbor-to-json Replacement for #481 as discussed in the working group call. Resolves #535 * Update openid-4-verifiable-presentations-1_0.md Co-authored-by: Stefan Charsley <charsleysa@gmail.com> * Update openid-4-verifiable-presentations-1_0.md Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com> * specify undefined behavior based on the WG discussion --------- Co-authored-by: Stefan Charsley <charsleysa@gmail.com> Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
5 participants
closes #420