Add comments on key discovery #111

OR13 · 2023-06-26T15:22:59Z

index.html

dlongley · 2023-06-26T23:28:58Z

index.html

+If <code>iss</code> is present in the <a data-cite="RFC7519#section-4.1.1">JWT Claims </a>,
+a <a data-cite="VC-DATA-MODEL#dfn-verifier">verifier</a> can use this parameter
+to obtain a <a data-cite="RFC7517#section-4">JSON Web Key</a> to use in the 
+<a data-cite="VC-DATA-MODEL#dfn-verify">verification</a> process.


Does this mean in the payload? Like if iss were to appear in the Verifiable Credential expressed as application/vc+ld+json? Or does it mean if iss appears alongside a vc claim in the 1.1 style?

I would think we'd want to avoid both cases in 2.0 and require these things to be in the protected header to keep layers separate for this external securing mechanism. Wouldn't getting key information from something in the payload also be incompatible with many existing JWT libraries?

I would think we'd want to avoid both cases in 2.0 and require these things to be in the protected header to keep layers separate for this external securing mechanism.

avoid both cases would mean banning the json member iss from application/vc+ld+json right?

I don't think that would get consensus... we tried similar things with proof... which is still both allowed to be present or optional.

There's a clear (and I think, agreed upon) way to do the security in this particular case, right? We want people to move on from the VC JWT 1.1 stuff and get a clean separation? If so, it seems we might have consensus here to clearly direct people not to put iss directly into the Verifiable Credential, but instead, if they are going to use it, put it in the protected header.

There's a clear (and I think, agreed upon) way to do the security in this particular case, right?

Yes, I think so, the authority is IETF for terms registered with IANA that are from IETF RFCs... They define this behavior we profile on top of it.

We want people to move on from the VC JWT 1.1 stuff and get a clean separation?

Yes, we made typ and cty values that solved this.

If so, it seems we might have consensus here to clearly direct people not to put iss directly into the Verifiable Credential, but instead, if they are going to use it, put it in the protected header.

Disagree, we are not going to constrain the open world model... people WILL put iss wherever they are allowed to (which is everywhere)... We have to explain what doing that means (document existing usage).

We won't get consensus to "ban terms in specific places", so it will happen in the payload and the header.

By default it should be interpreted by the security conventions from the relevant RFCs, that is what this PR does. That is why the language just mirrors the RFCs, and doesn't constrain further.

This PR does not change anything an implementer can do today, it acknowledges that there are thing they can do that have special meaning today (registered names in header and claimset), and reminds them to be aware of how those terms are used today.

dlongley · 2023-06-26T23:30:40Z

index.html

+to obtain a <a data-cite="RFC7517#section-4">JSON Web Key</a> to use in the 
+<a data-cite="VC-DATA-MODEL#dfn-verify">verification</a> process.
+      </p>
+If <code>kid</code> is also present, it is expected to be useful to distinguish the specific key used.


Does this mean kid could be in the payload? It's defined as a header parameter:

https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.4

Or is this just referring to kid appearing in the header and iss in the payload? How should "also present" be interpreted here?

Also, how is kid to be useful to distinguish the specific key used? This cries out for an example, or a fair amount of additional prose.

AFAIK, kid is legal in payload, in the same way that baz or alg is legal in payload.

The sentence is not attempting to be specific about where kid can be found, but kid does have special meaning when its in the protected header.

can you make a concrete change suggestion?

Yes, I've made a suggestion to clarify that we're talking about kid being present in the protected header.

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

TallTed

A bunch of typical cleanup, and a few questions to be answered.

index.html

TallTed · 2023-06-27T21:41:11Z

index.html

+to obtain a <a data-cite="RFC7517#section-4">JSON Web Key</a> to use in the 
+<a data-cite="VC-DATA-MODEL#dfn-verify">verification</a> process.
+      </p>
+If <code>kid</code> is also present, it is expected to be useful to distinguish the specific key used.


Also, how is kid to be useful to distinguish the specific key used? This cries out for an example, or a fair amount of additional prose.

index.html

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

OR13 · 2023-06-29T20:48:38Z

@TallTed thank you for the suggestions, your text was much better.

I have merged all except for this one: #111 (comment)

I am dreading the conflict resolution for this, so I want to get all your changes merged into the branch before I attempt to resolve that, since I might have an easier time just lifting the section into a new PR.

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

OR13 · 2023-06-29T22:30:32Z

@TallTed I have accepted all your changes, before I attempt to resolve merge conflicts.

@dlongley @TallTed do you have any more suggestions?

Regarding your comment on kid, I can't do much more on that front given the RFCs don't do much more, but I am happy to review suggestions if you have any.

TallTed · 2023-06-29T23:15:34Z

This is good for this go-round. I don't have a suggestion on kid, so I suggest creating a fresh issue about that need, fixing this PR's conflicts, and merging this PR. Someone else may then come up with a suggestion on kid.

OR13 · 2023-06-30T00:27:47Z

@TallTed

There are a lot of open issues related to kid

Most of them have some relationship to your question, here is an example:

#31

Are you asking for an issue marker?

Or is it ok to just use the issue above?

TallTed · 2023-06-30T14:36:28Z

I've created #117 for this.

index.html

mprorock · 2023-06-30T19:33:03Z

@TallTed

There are a lot of open issues related to kid

Most of them have some relationship to your question, here is an example:

#31

Are you asking for an issue marker?

Or is it ok to just use the issue above?

i think referencing issue #31 works for me

index.html

selfissued

This makes sense to me.

Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>

Co-authored-by: Mike Prorock <mprorock@users.noreply.github.com>

OR13 · 2023-06-30T21:16:06Z

@TallTed I think I have your suggestions, let me know if I missed anything.

@dlongley I applied most of your suggestions, but this one is outstanding: #111 (comment)

Is the issue marker I proposed acceptable?

I'm trying to resolve all suggestions before addressing the merge conflicts, because I am bad at git.

OR13 · 2023-06-30T21:20:59Z

This list of related issues needs to go in to an issue marker, before this is merged:

dlongley

Approving -- I've left a comment here: https://github.com/w3c/vc-jwt/pull/111/files#r1248318896. Thanks!

OR13 · 2023-06-30T23:17:42Z

Ok, I think I am ready to tackle the merge conflict, wish me luck.

OR13 · 2023-07-05T18:42:35Z

@mprorock I believe all feedback has been addressed, and issue markers have been filed, I suggest we merge this.

Add comments on key discovery

c5741bd

OR13 commented Jun 26, 2023

View reviewed changes

index.html Outdated Show resolved Hide resolved

Update index.html

b86dda3

This was referenced Jun 26, 2023

remove requirement to have kid in cnf oauth-wg/oauth-sd-jwt-vc#141

Merged

breaking change: .well-known/jwt-issuer renamed to .well-known/jwt-vc-issuer oauth-wg/oauth-sd-jwt-vc#118

Closed

TallTed reviewed Jun 26, 2023

View reviewed changes

index.html Outdated Show resolved Hide resolved

dlongley reviewed Jun 26, 2023

View reviewed changes

Update index.html

07427cb

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

OR13 mentioned this pull request Jun 27, 2023

Define how to obtain verification keys for vc-jwt #70

Closed

TallTed suggested changes Jun 27, 2023

View reviewed changes

OR13 and others added 7 commits June 29, 2023 15:43

Update index.html

ea70a52

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

1dc05ff

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

dad4ff7

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

79e45d1

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

6b0288d

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

aeb49e2

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

b9beba9

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Update index.html

b04c4b2

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

TallTed mentioned this pull request Jun 30, 2023

how is kid to be useful to distinguish the specific key used? #117

Closed

dlongley suggested changes Jun 30, 2023

View reviewed changes

index.html Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

andresuribe87 reviewed Jun 30, 2023

View reviewed changes

index.html Show resolved Hide resolved

mprorock reviewed Jun 30, 2023

View reviewed changes

index.html Show resolved Hide resolved

selfissued approved these changes Jun 30, 2023

View reviewed changes

Update index.html

7c7edbd

Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>

OR13 and others added 2 commits June 30, 2023 16:07

Update index.html

74ec3e4

Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>

Update index.html

47fb837

Co-authored-by: Mike Prorock <mprorock@users.noreply.github.com>

This was referenced Jun 30, 2023

Clarify relationship between "iss" and "issuer" #22

Closed

Proposal: use did:jwk for testing dids and vc-jwt in the vc-jwt-test-suite w3c/vc-jose-cose-test-suite#10

Open

dlongley approved these changes Jun 30, 2023

View reviewed changes

OR13 added 3 commits July 5, 2023 13:34

Merge branch 'main' of github.com:w3c/vc-jwt into feat/key-discovery

89d431b

Fix broken tag

65741c1

Add issue markers for open issues

a4bed8c

mprorock merged commit 57ce7e3 into main Jul 5, 2023
1 of 2 checks passed

OR13 deleted the feat/key-discovery branch July 7, 2023 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add comments on key discovery #111

Add comments on key discovery #111

OR13 commented Jun 26, 2023 •

edited by pr-preview bot

dlongley Jun 26, 2023

OR13 Jun 29, 2023

dlongley Jun 30, 2023

OR13 Jun 30, 2023 •

edited

dlongley Jun 26, 2023

TallTed Jun 27, 2023

OR13 Jun 29, 2023 •

edited

dlongley Jun 30, 2023

TallTed left a comment

TallTed Jun 27, 2023

OR13 commented Jun 29, 2023

OR13 commented Jun 29, 2023

TallTed commented Jun 29, 2023

OR13 commented Jun 30, 2023

TallTed commented Jun 30, 2023

mprorock commented Jun 30, 2023

selfissued left a comment

OR13 commented Jun 30, 2023 •

edited

OR13 commented Jun 30, 2023

dlongley left a comment

OR13 commented Jun 30, 2023

OR13 commented Jul 5, 2023

Add comments on key discovery #111

Add comments on key discovery #111

Conversation

OR13 commented Jun 26, 2023 • edited by pr-preview bot

dlongley Jun 26, 2023

Choose a reason for hiding this comment

OR13 Jun 29, 2023

Choose a reason for hiding this comment

dlongley Jun 30, 2023

Choose a reason for hiding this comment

OR13 Jun 30, 2023 • edited

Choose a reason for hiding this comment

dlongley Jun 26, 2023

Choose a reason for hiding this comment

TallTed Jun 27, 2023

Choose a reason for hiding this comment

OR13 Jun 29, 2023 • edited

Choose a reason for hiding this comment

dlongley Jun 30, 2023

Choose a reason for hiding this comment

TallTed left a comment

Choose a reason for hiding this comment

TallTed Jun 27, 2023

Choose a reason for hiding this comment

OR13 commented Jun 29, 2023

OR13 commented Jun 29, 2023

TallTed commented Jun 29, 2023

OR13 commented Jun 30, 2023

TallTed commented Jun 30, 2023

mprorock commented Jun 30, 2023

selfissued left a comment

Choose a reason for hiding this comment

OR13 commented Jun 30, 2023 • edited

OR13 commented Jun 30, 2023

dlongley left a comment

Choose a reason for hiding this comment

OR13 commented Jun 30, 2023

OR13 commented Jul 5, 2023

OR13 commented Jun 26, 2023 •

edited by pr-preview bot

OR13 Jun 30, 2023 •

edited

OR13 Jun 29, 2023 •

edited

OR13 commented Jun 30, 2023 •

edited