Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First cut at the DPoP-Inspired Authentication section #19

Merged
merged 6 commits into from
Jun 26, 2024

Conversation

bc-pi
Copy link
Collaborator

@bc-pi bc-pi commented Jun 19, 2024

Yaron Sheffer
[8:31 AM May 30 in slack]
To summarize the near term plan:
Joe: security considerations and interaction with TLS
Brian: ID Token and DPoP-inspired
Yaron: Message Signatures

this is the "DPoP-inspired" part

A preview editors' copy of this PR can be seen at http://www.sheffer.org/wimse-s2s/bc-dpop-esque-pop/draft-sheffer-wimse-s2s-protocol.html

@bc-pi bc-pi requested a review from yaronf as a code owner June 19, 2024 21:35
or other token in the request that might convey end-user identity and authorization context of the request.
The value MUST be the result of a base64url encoding (as defined in {{Section 2 of RFC7515}}) the SHA-256 hash of
the ASCII encoding of the associated token's value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can there be more than one context token hash? would this need to be specified as an array or can you have multiple of the same claim in the jwt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can there be more than one context token hash?

This is a good question that I asked myself while writing the above. But didn't have a great sense for so opted for the simple answer of just one.

The whole construct is admittedly somewhat awkward - trying to account/allow for different kinds of tokens that might show up in different places.

would this need to be specified as an array or can you have multiple of the same claim in the jwt?

Multiple of the same claim in the jwt is not allowed (see https://datatracker.ietf.org/doc/html/rfc7519#section-4) so an array is how that kind of data is expressed.

Copy link
Collaborator Author

@bc-pi bc-pi Jun 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But maybe a few different claims that conditionally show up would be a better approach. Like ath access token hash - a la https://www.rfc-editor.org/rfc/rfc9449.html#section-4.2-6.2 - and then txth transaction token hash and maybe an oth for other token hash...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently rethinking the construct

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we can definitely punt on it for now, I think we could choose a construct that's both simple and extensible.

"signed_headers": {
    "txn-token": "txn token hash",
    "x-custom-context": "x-custom-context hash"
}

Unfortunately we need the WPT to sign miscellaneous stuff, because we don't want a new WIT for every call. Having said that, as far as I can tell it does NOT need to sign the WIT itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue #25 from @yaronf's comment and c5b46a1 tries to do this in a somewhat better way.

* `iss`: The issuer of the token, which is the calling workload, represented by the same value as the `sub` claim
of the associated WIT.
* `aud`: The audience of the token contains the the HTTP target URI ({{Section 7.1 of RFC9110}}) of the request
to which the WPT is attached, without query or fragment parts.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't aud the WIMSE identity of the target? Wouldn't that be more consistent?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That, to me, begs a larger philosophical and scoping question about naming and addressing and identity, which is beyond my competence. And in that lacking of competence, I've used an existing, well-known, and widely used means of identifying the target of an HTTP request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmph, let's discuss offline. In the meantime, please remove the redundant "the" from the sentence.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 0694849 and looking forward to a lively discussion

or other token in the request that might convey end-user identity and authorization context of the request.
The value MUST be the result of a base64url encoding (as defined in {{Section 2 of RFC7515}}) the SHA-256 hash of
the ASCII encoding of the associated token's value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we can definitely punt on it for now, I think we could choose a construct that's both simple and extensible.

"signed_headers": {
    "txn-token": "txn token hash",
    "x-custom-context": "x-custom-context hash"
}

Unfortunately we need the WPT to sign miscellaneous stuff, because we don't want a new WIT for every call. Having said that, as far as I can tell it does NOT need to sign the WIT itself.

draft-sheffer-wimse-s2s-protocol.md Outdated Show resolved Hide resolved
draft-sheffer-wimse-s2s-protocol.md Show resolved Hide resolved
@yaronf yaronf merged commit 8cc275f into main Jun 26, 2024
2 checks passed
@yaronf yaronf deleted the bc-dpop-esque-pop branch July 3, 2024 16:50
@bc-pi bc-pi mentioned this pull request Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants