I wrote this feedback right after I had written the reworded introduction, as I had just read over the full document.
I appreciate that you don't want to change what you have done, but if there is going to be a change, now is the time. When I was leading the design of what became OAuth 2.0 (OAuth-WRAP) and JOSE (Simple Web Tokens) - our high order bit was ease of implementation and to learn from the implementation challenges in OAuth 1.0 and XML-DSIG respectively. I share the feedback below not to infer that your choices are wrong, but wearing my implementation hat (I spend half my time now writing production code) and pointing out areas that I found confusing and in my opinion need polish. While it may be frustrating, WG-LC is often the only time many people fully read a spec. Such is life. You are welcome to ignore my feedback of course and push it through. I'm happy to provide clarification on my feedback and engage in productive discussion -- I'm not interested in trying to convince any of you to change anything. /Dick On Sat, Sep 21, 2024 at 6:15 PM Dick Hardt <dick.ha...@gmail.com> wrote: > > > On Sat, Sep 21, 2024 at 4:28 PM Daniel Fett <m...@danielfett.de> wrote: > >> Hi Dick, >> Am 21.09.24 um 06:41 schrieb Dick Hardt: >> >> Hey Brian, Kristina, Daniel >> >> I appreciate you have been working on this for a while, and this feedback >> is last minute, and people have already working code that works with it -- >> so this is unlikely to be welcome feedback -- but in the spirit of wanting >> what is best long term, here it is. >> >> You are unfortunately correct - not only have we been working on this for >> a while now, we also discussed many of the points you list below already. >> Since not all of those discussions happened here on the mailing list, I'll >> link to the relevant Github issues where appropriate. >> >> >> *Token Separators* >> It is not clear to me why the disclosures are an arbitrary array of '~' >> separated items. It seems very odd to me as a developer. Why not have a 4th >> '.' separated string that is base64 URL encoded JSON array of the >> disclosures? The header includes the 'typ' so that a parser knows what all >> the following components are and how many there are. >> >> If you use a JSON array for the disclosures, you need to >> double-base64-url-encode all disclosures, needlessly increasing the size of >> the SD-JWT(-KB). Please read these issues: >> >> https://github.com/oauth-wg/oauth-selective-disclosure-jwt/issues/36 >> >> https://github.com/oauth-wg/oauth-selective-disclosure-jwt/issues/180 >> >> https://github.com/oauth-wg/oauth-selective-disclosure-jwt/issues/37 >> > I realized after sending that each disclosure should be its own component > so it is easy to compute the digest. > I had to realize that myself as I thought about implementing ... it did > not come from reading the doc. > >> >> *Disclosure Format* >> On the format of the disclosures, the implicit semantics of the order of >> items in an array is a throwback to data formats of the last century. We >> are using JSON, let's have it be an object where we name each item. >> >> We had that in the beginning, but the consensus was that we don't want to >> increase the size by using named object properties. Short single-letter >> properties were considered as well, but didn't add much value over an array. >> > I disagree -- but not a big deal. > >> >> *Array Disclosure* >> Is this really needed? The only example I saw was for multiple >> citizenships. Would the issuer of an SD-JWT that contains a subject's >> citizenship not be the country who they are a citizen of? Why would another >> country be authoritative that the subject is a citizen of another country? >> In the example, it is hard to imagine a verifier trusting the US to say >> someone is a DE citizen, or vice versa. The array of age claims in example >> A.3 does not need the non intuitive '...' array mechanism. >> >> Yes, it is needed. First of all, SD-JWT can be used universally, not just >> for credentials. >> > I don't know what that sentence means. It is alluded to in the current > introduction, and makes no sense to me. Are you saying that an SD-JWT could > be used to selectively disclose claims that are not user identity claims? > Sure. > >> Second, even if used for credentials, there are many types of credentials >> where there is a use case for disclosing only some array values. Take an >> education credential as an example, where the holder may want to disclose >> only relevant courses and grades. You'll find more use cases in the >> examples in the appendix. >> >> A bit besides the point, but in the EU context we do in fact plan for >> credentials attesting other nationalities (and have good reasons to do so, >> here PID credentials derived from the eID card for EU citizens, see >> https://bmi.usercontent.opencode.de/eudi-wallet/eidas-2.0-architekturkonzept/functions/00-pid-issuance-and-presentation/#pid-contents >> for details). >> > I'm going to dig in on this point. SD-JWT is NOT taking a JSON array as > input. You have an array of digests and an array of disclosures. You can > have as many of the same property as you want like you are doing with the > age claims in the example. > > IE you can have a digests for "nationality"="DE" and for > "nationality"="US" and then have a disclosure for each one. No '...' magic > needed. > > > >> Taking this out would simplify implementations. >> >> Fluff -- are there features that, when removed, would not simplify >> implementations? >> > > I'll restate it more strongly, I think the array support will double the > effort for no real benefit since as noted above, you can already include > multiples of the same property in an SD-JWT > >> >> *Claim Names* >> Why do the claims start with '_'? Why not just 'sd' and 'sda'? Why is >> '_sd_alg' in the payload and not in the header? >> >> While the underscore doesn't officially have any special meaning, adding >> it reduces the chance for collisions with existing claims and makes the >> SD-JWT-related claims sort nicely. All SD-related claims are in the >> payload, that's why we put _sd_alg there as well. >> > Do you have data that shows it will reduce collisions? I have seen many > implementations that created their own claims that start with _ to reduce > collisions with the same rationale! > > There is an IANA registry for claim names to avoid collisions. > > The _ reminds me of internal C variables that others were not supposed to > use, but eventually did. > > _sd_alg is NOT a claim. It is a signal for which algorithm to use and > should be in the header. > > I'm unclear on the sorting advantage. They would sort together if they > started with sd as well. > > >> >> *Explicit Typing* >> Why leave the typing in the header to be determined by the application >> (10.11), and not just be 'sd-jwt' and be REQUIRED? >> >> We had extensive discussions around typing, please refer to the following >> issues: >> >> - https://github.com/oauth-wg/oauth-selective-disclosure-jwt/issues/267 >> >> - https://github.com/oauth-wg/oauth-selective-disclosure-jwt/issues/327 >> >> - https://github.com/oauth-wg/oauth-selective-disclosure-jwt/issues/345 >> > > Those issues don't really address the point. > > Per RFC 8725: JSON Web Token Best Current Practices (rfc-editor.org) > <https://www.rfc-editor.org/rfc/rfc8725.html#name-use-explicit-typing> -- > the best practice would be to have a single type that would allow a library > to know it is an SD-JWT. If additional context is needed, perhaps that > should be a different header property? > > >
_______________________________________________ OAuth mailing list -- oauth@ietf.org To unsubscribe send an email to oauth-le...@ietf.org
