Add guidance around using JWK

[decentralgabe]

fix #210

---

Preview | Diff

[TallTed]

OPTIONAL or REQUIRED should be clearly stated for all properties

[msporny]

@selfissued noted that alg is not optional.

[iherman]

The issue was discussed in a meeting on 2024-01-24

• no resolutions were taken

View the transcript

1.2. adjust language in example 13 (pr vc-jose-cose#220)

See github pull request vc-jose-cose#220.

Brent Zundel: Adjust language in example 13 -- again an editorial change.
… Ted has reviewed and his change requests have been made. I encourage other folks to check out this PR.

Manu Sporny: Just a question -- I approved the PR fine as is. There's this fully specified algorithm stuff that is making it's way through IETF. How does that spec involve the language here?
… I think this one doesn't make mention of fully specified algorithms or not ... it probably doesn't need to but do we need any updates around this?

Michael Jones: For the language in example 13?

Manu Sporny: Yeah. The PR also adds some normative text around the alg property. It says the alg property is optional and is recommended to be included.
… It says that crv and kid need to be specified.

Michael Jones: I need to review that then because alg is never optional.

Manu Sporny: This PR says it is. I'll put your comment into it.

Brent Zundel: Sorry, my understanding is that the normative bits have just been moved around, so it may be more normative than I expected.

Gabe Cohen: Yeah, I was going to say what you said -- I think I made a mistake with that first sentence so I will adjust that after Mike gives it a review.

Manu Sporny: I don't think you made a mistake, it's confusing or wrong original text that you didn't add Gabe.

Ted Thibodeau Jr.: I added the all-caps OPTIONAL based on the following text -- it needs to be consistent whatever it is.

Michael Jones: JOSE requires alg.

Brent Zundel: Good feedback on that PR, it's actionable, encourage folks to keep an eye on it as it is reviewed and updated.

[decentralgabe]

@msporny noting that after discussion we realized a misunderstanding, alg is optional in a JWK.

[selfissued]

I misunderstood the context when we were discussing this on the call. alg is a required header parameter, which is what I thought we were discussing. alg is not required in JWKs.

[TallTed]

[@selfissued] I misunderstood the context when we were discussing this on the call. alg is a required header parameter, which is what I thought we were discussing. alg is not required in JWKs.

Reverting incorrect changes made since #220 (review), and improving the result

Suggested change

	The `alg` property identifies the algorithm intended
	for use with the public key, and is
	included to prevent security issues that can arise when using the same key with multiple
	The `alg` property is OPTIONAL in JWKs, and identifies the algorithm intended
	for use with the public key. Although optional, it is RECOMMENDED that `alg`
	be included, to avoid security issues that can arise when using the same key with multiple

[decentralgabe]

@TallTed this change was intentional, as we do not want to overlap with normative guidance provided by the JWK spec

[TallTed]

@decentralgabe — WHICH change was intentional?

This specific change request is to bring us back to where we were before @selfissued's misunderstanding of alg property vs alg header parameter, where there were no complaints about any "overlap with normative guidance provided by the JWK spec".

If we're now (near to) overlapping the JWK spec, we should have a citation here which points to the specific segment of the JWK spec, such that implementers of vc-jose-cose do not have to read the entirety of the JWK spec.

[decentralgabe]

the change to remove the OPTIONAL and RECOMMENDED - as you note - before the misunderstanding - was intentional.

If we're now (near to) overlapping the JWK spec, we should have a citation here which points to the specific segment of the JWK spec, such that implementers of vc-jose-cose do not have to read the entirety of the JWK spec.

This is a good idea, I will update the PR

[brentzundel]

@msporny and @TallTed I believe the changes you've requested have been made, could you please re-review?

[TallTed]

This clarity of presentation was discussed and approved earlier; its removal occurred through misunderstanding of what was being discussed. I haven't seen an argument that I think sufficiently justifies omitting these 5 words (~38 characters, depending on whether you count spaces or not).

[iherman]

The issue was discussed in a meeting on 2024-02-14

• no resolutions were taken

View the transcript

1.3. Add guidance around using JWK (pr vc-jose-cose#220)

See github pull request vc-jose-cose#220.

Gabe Cohen: One thing I forgot to mention -- there's some outstanding discussion around 220 ... around the JsonWebKey text that we should discuss to get clarity around some confusion that came up.
… This PR is clarifying language on the JsonWebKey spec on how to use the JsonWebKey type. There was discussion on a call a few weeks ago on whether we should add language on including properties like alg and kid and at first there was agreement to add back normative guidance on those properties.
… But then Mike and I agreed we didn't want to repeat language from another RFC -- and so we removed that. Ted said he wanted the language back.

Ted Thibodeau Jr.: the language that was removed was removed during misunderstanding of what was being discussed...point being the four words were added with intent and removed without that intent, which is why I've asked them to be re-added.

Manu Sporny: the language being modified is normative language that is significant. need to update the title of the PR, since it's broader than the example. somewhat confused...had said we'd have explicit guidance on iss, kid, etc. that guidance was not provided...may be a different issue. if we're talking about keys and just a JWT, and if we're just repeating what's said in the other spec we don't need to repeat it here. somewhat confusing...since kid matters.

See github pull request vc-jose-cose#226.

Gabe Cohen: The changes you're referring to Manu, went into 226. The changes we're talking about ... the PR is unfortunately named. The language I moved was originally in an example.
… The issue was to move it to normative guidance, outside of the example.
… The PR adds some guidance around using the JsonWebKey.

Ted Thibodeau Jr.: On Jan 18th, I said optional or required should be clearly stated for all properties, that's generally true what's happening but not true for the couple that were added with this PR.
… Those changes went in and were merged but then Mike said that some requirements were wrong.
… alg is optional in JWKs -- and that's what I want put back.

Michael Jones: Are we talking about a change to header params or JWKs?

Gabe Cohen: JWKs.

Michael Jones: It's optional there, what does it say now?

Gabe Cohen: Nothing.

Michael Jones: It's fine to say that.
… This is one of the PRs I was trying to get a sense of ... is this one controversial or is that another one?

Gabe Cohen: It sounds like we're clear on this one, I'll apply Ted's suggestion and then we're good.
… The other has a rendering script problem.

Michael Jones: Ok, 220 should be ready once we get the suggestion in.

[decentralgabe]

changes addressed, merging.