Extract RS-focused protocol elements #246

jricher · 2021-04-13T20:31:24Z

This change set extracts the resource-server-focused elements of GNAP to allow the core protocol to focus on the client-AS relationship while this new document
can focus on the RS-AS relationship.

Closes #114

remove RS section for refactoring

RS section now at: https://github.com/ietf-wg-gnap/gnap-resource-servers

netlify · 2021-04-13T20:32:17Z

Deploy preview for gnap-core-protocol-editors-draft ready!

Built with commit 19f2ae9

https://deploy-preview-246--gnap-core-protocol-editors-draft.netlify.app

draft-ietf-gnap-resource-servers.md

adeinega · 2021-04-13T21:23:21Z

Regarding

This specification also discusses methods for an RS to derive a downstream token for calling another chained RS...

Why this specification can't state it even in a more generic way similar to what https://tools.ietf.org/html/rfc8693 does? There are use cases when we may need to exchange an access token and then use it with something completely different than chained RSs.

adeinega · 2021-04-13T21:26:37Z

Maybe this is a bit unrelated but... The self-contained tokens usually contain one or another form of the issuer. What would be the issuer for GNAP? Is it the grant endpoint?

jricher · 2021-04-13T21:27:31Z

@adeinega I'd be happy to see proposed language to discuss! The limitation is to keep parity with the "token request" mechanism in the core draft: the goal is to get an access token, but based on the context of an existing access token.

One of the problems I've always had with RFC8693 is that it tries to be many things at once, from access token exchange to assertion exchange to likely other use cases. If other functions should be added to this spec then we can do that, but hopefully we won't run the same risk of putting everything through the same narrow pipe. Technically since it's a basic GNAP request, you can already return subject identifiers or assertions as well, but we don't currently talk about any of that. Should we?

jricher · 2021-04-13T21:29:02Z

@adeinega The concept of the "issuer" entirely depends on the type of token. The grant endpoint would be an obvious value for GNAP though, and if we're going to go down the route of discussing token contents from a structural standpoint (or even profile things like JWT or Biscuits or PASETO) then we can make that recommendation. However, the current text is intended to be at a much higher level than that, right now.

adeinega · 2021-04-13T21:33:26Z

@jricher, I also wonder if GNAP is going to support encrypted token introspection responses on its core?

adeinega · 2021-04-13T21:45:54Z

Another thing I would like to understand better if GNAP is going to rely completely on TLS... or it would

sign its own responses such as token introspect responses say using Detached-JWS (or other means)
provide a way to cryptographically link a request from the client to AS's response

TLS isn't going anywhere... It's a must. Although, in certain forms of envs, it's great to have an additional layer on top of it.

adeinega · 2021-04-13T21:51:41Z

Regarding,
However, the current text is intended to be at a much higher level than that, right now.

It's understandable. It's entirely possible I missed something in the current spec but it would be nice to say that a URL of the grant endpoint also represents something the issuer does in other identity-related specs.

I hope it makes some sense.

adeinega · 2021-04-13T22:12:24Z

draft-ietf-gnap-resource-servers.md

+}
+~~~
+
+The AS responds with a token for the downstream RS2 as described in 


I would suggest omitting the downstream word for #246 (comment) to make this section more generic. An RS 1 may call RS 2 that belongs to a completely different sec domain. RS 2 may also trust only to its own AS 2... and there is some form of trust established between AS 1 and AS 2. The same applies to the name of this section (Deriving a downstream token). I do hope it makes sense.

Fair point, I think the current section is tightly focused on the RS going back to the same issuing AS that the original token came from -- this doesn't have to be the case, of course, but for the cases where it's not the issuing AS then as you point out there needs to be some kind of trust happening between the two AS's such that the second AS can make any sense of things. I am a little afraid of the model becoming overly abstract and not giving developers enough to hook on to, but I think it's worth pursuing. I'm going to suggest that if the WG does accept this PR then we continue that conversation in a new issue dedicated to that topic.

adeinega · 2021-04-14T04:22:30Z

draft-ietf-gnap-resource-servers.md

+    "access": [
+        "dolphin-metadata", "some other thing"
+    ],
+    "key": {


What's about the rotation of keys? Shouldn't it be a list of keys? Without that, we won't have a smooth transition when the client decides to rotate its key.

I still suggest requiring the JWK thumbprint as a value for kid in jwk as protection against misuse.

In this design, if the key is rotated then whatever the "current" key is will be returned here, replacing the "old" key. I'm not sure I see the value in the overhead that multiple keys would have here just to address this one particular corner. To me it feels like having multiple keys tied to a single access token at any time would be problematic and lead to potential security gaps. Introspection already has to deal with cache inconsistency problems, so an RS is going to need to be robust against something like that anyway. This kind of situation can be talked about in some kind of implementation guide section, perhaps.

Interestingly... so, your point is that an RS should be robust to handle this. I get it. Although we may still have & deal with an extremely chatty application and there is an unavoidable delay between the decision to rotate its key by the client and what both AS and RS known.

I still like to hear your thoughts on

I still suggest requiring the JWK thumbprint as a value for kid in jwk as protection against misuse.

Regarding with cache inconsistency problems, to be completely pedantic, the response, as well as the spec, includes Cache-Control: no-store. So, it's forbidden. You may allow omitting it for the introspection endpoint's response.

HTTP Cache and functional application cache are totally separate from each other. You can disable an HTTP cache but still have an application save the results and simply not make another HTTP call again.

JWK identifiers are varied and can have meaning inside of an environment (such as date/timestamps, service identifiers, etc), requiring a specific format will limit the utility of the protocol unnecessarily as it adds no additional benefits.

fimbault · 2021-04-14T07:14:23Z

draft-ietf-gnap-core-protocol.md

-    type of request is used only for calls to the RS during a discovery phase as
-    described in {{rs-request-without-token}}.
+- When neither an access token nor key proof are used, this is an unsecured request. This type
+    of request is not used in the core protocol of GNAP.


Ok, but the previous text was clearer as to what we're referring to. Could lonk to the proper section in I-D.draft-ietf-gnap-resource-servers as an example?

We could point externally to that, but I took the reference out because one should never be making this kind of unprotected call in any of GNAP's core. So, this bullet was really just supposed to complete the matrix of possibilities and say "don't do this" -- the exception is in the extension that allows bootstrapping.

fimbault · 2021-04-14T07:15:45Z

draft-ietf-gnap-resource-servers.md

+    email: [email protected]
+    uri: https://acert.io/
+
+normative:


Probably should review that list, I guess there's a lot we don't use in here specifically, except from gnap core.

Yes, and the tools will throw warnings for unused references that we can use to trim things -- I just left the list as-is for now as we settle on which text goes where to avoid compilation errors for now.

draft-ietf-gnap-resource-servers.md

fimbault

I think that's a good starting point.

A few general remarks:

Seems to me that 3.3 is more an AS facing API. We could have a paragraph 3 focused on RS APIs (well known, registering a resource handle, etc.), and regroup the other items in a section that discusses how the RS reacts when a client calls it (i.e. current sections 3.3, 4 and 5)
Will we need some kind of resource_server instance to declare a stable "resource_server": "7C7C4AZ9KHRS6X63AJAO"? (beyond tne reference, which seems chosen here by the RS?)

jricher · 2021-04-14T14:39:25Z

@fimbault :

Section 3.3 (introspection) is put here right now because introspection is an API presented to the RS by the AS, just like the others. I'm very open to reorganizing the whole document though as it does need to be more cohesive. This pull was a first step.
This is currently covered in 3.2, the AS can pre-configure the identifiers or let the RS just present a key at runtime. This discussion needs to be expanded along with security considerations for both practices, and ways to do the binding dynamically (like return the value at runtime, the way a client instance ID is returned)

adeinega · 2021-04-15T18:48:54Z

draft-ietf-gnap-resource-servers.md

+POST /introspect HTTP/1.1
+Host: server.example.com
+Content-Type: application/json
+Detached-JWS: ejy0...


resource_server identifier could be a part of Detached-JWS. In this case, you might want to omit it from the actual request. I don't see any reason to specify it twice.

The identifier is not defined as part of that structure, it's defined as part of the body of the request where needed. There's no need to overload it. DPoP overloads the key identifier here because it has no other choice.

adeinega · 2021-04-15T18:50:28Z

draft-ietf-gnap-resource-servers.md

+Detached-JWS: ejy0...
+
+{
+    "access_token": "OS9M2PMHKUR64TB8N6BW7OZB8CDFONP219RP1LT0",


This section doesn't specify the required field in the introspection request. At least, the "access_token" field should be required, right?

Yes, all these sections need to be more clearly defined, the goal of this PR is just to get the right separation in place, so that work like this point can continue in the right place.

I am guessing that resource_server could be also provided by value. Is this right?

@adeinega Yes, that's the intent of the "authenticating the RS" section, it would be parallel to how clients are identified in the core document.

yaronf · 2021-04-19T13:28:02Z

Folks, I have a strong preference for the new document to live in its own repo (see https://www.rfc-editor.org/rfc/rfc8874.html#name-repositories for pros and cons). Any objections?

jricher · 2021-04-19T14:05:00Z

@yaronf I'm fine with this being in its own repository or in the same one -- I've used both in different groups and each has its benefits and drawbacks. If the chairs set up the repo I can import the support code and check in this document in its own PR.

yaronf · 2021-04-19T14:20:20Z

https://github.com/ietf-wg-gnap/gnap-resource-servers

jricher · 2021-04-20T19:22:40Z

RS document now moved to https://github.com/ietf-wg-gnap/gnap-resource-servers

jricher requested review from aaronpk and fimbault April 13, 2021 20:31

adeinega reviewed Apr 13, 2021

View reviewed changes

draft-ietf-gnap-resource-servers.md Outdated Show resolved Hide resolved

adeinega reviewed Apr 13, 2021

View reviewed changes

adeinega reviewed Apr 14, 2021

View reviewed changes

fimbault reviewed Apr 14, 2021

View reviewed changes

draft-ietf-gnap-resource-servers.md Outdated Show resolved Hide resolved

fimbault reviewed Apr 14, 2021

View reviewed changes

draft-ietf-gnap-resource-servers.md Outdated Show resolved Hide resolved

fimbault reviewed Apr 14, 2021

View reviewed changes

draft-ietf-gnap-resource-servers.md Outdated Show resolved Hide resolved

fimbault reviewed Apr 14, 2021

View reviewed changes

draft-ietf-gnap-resource-servers.md Outdated Show resolved Hide resolved

fimbault reviewed Apr 14, 2021

View reviewed changes

draft-ietf-gnap-resource-servers.md Outdated Show resolved Hide resolved

fimbault reviewed Apr 14, 2021

View reviewed changes

jricher added the Pending Merge PR will be merged unless there are changes to consensus label Apr 14, 2021

jricher mentioned this pull request Apr 14, 2021

Validity of an access token #191

Closed

adeinega reviewed Apr 15, 2021

View reviewed changes

jricher added 2 commits April 20, 2021 15:20

remove RS section for refactoring

6d19dcd

editorial cleanup

19f2ae9

jricher force-pushed the extract-rs branch from dd12b56 to 19f2ae9 Compare April 20, 2021 19:21

Merge branch 'main' into extract-rs

fbc29c8

jricher merged commit 6c6b38c into ietf-wg-gnap:main Apr 21, 2021

jricher removed the Pending Merge PR will be merged unless there are changes to consensus label Apr 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract RS-focused protocol elements #246

Extract RS-focused protocol elements #246

jricher commented Apr 13, 2021 •

edited

Loading

netlify bot commented Apr 13, 2021 •

edited

Loading

adeinega commented Apr 13, 2021

adeinega commented Apr 13, 2021 •

edited

Loading

jricher commented Apr 13, 2021

jricher commented Apr 13, 2021

adeinega commented Apr 13, 2021

adeinega commented Apr 13, 2021

adeinega commented Apr 13, 2021

adeinega Apr 13, 2021 •

edited

Loading

jricher Apr 13, 2021

adeinega Apr 14, 2021

jricher Apr 14, 2021

adeinega Apr 15, 2021

adeinega Apr 15, 2021

adeinega Apr 15, 2021

jricher Apr 15, 2021

jricher Apr 15, 2021

fimbault Apr 14, 2021

jricher Apr 14, 2021

fimbault Apr 14, 2021 •

edited

Loading

jricher Apr 14, 2021

fimbault left a comment

jricher commented Apr 14, 2021

adeinega Apr 15, 2021

jricher Apr 15, 2021

adeinega Apr 15, 2021 •

edited

Loading

jricher Apr 15, 2021

adeinega Apr 16, 2021

jricher Apr 19, 2021

yaronf commented Apr 19, 2021

jricher commented Apr 19, 2021

yaronf commented Apr 19, 2021

jricher commented Apr 20, 2021

Extract RS-focused protocol elements #246

Extract RS-focused protocol elements #246

Conversation

jricher commented Apr 13, 2021 • edited Loading

netlify bot commented Apr 13, 2021 • edited Loading

adeinega commented Apr 13, 2021

adeinega commented Apr 13, 2021 • edited Loading

jricher commented Apr 13, 2021

jricher commented Apr 13, 2021

adeinega commented Apr 13, 2021

adeinega commented Apr 13, 2021

adeinega commented Apr 13, 2021

adeinega Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fimbault Apr 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fimbault left a comment

Choose a reason for hiding this comment

jricher commented Apr 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adeinega Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaronf commented Apr 19, 2021

jricher commented Apr 19, 2021

yaronf commented Apr 19, 2021

jricher commented Apr 20, 2021

jricher commented Apr 13, 2021 •

edited

Loading

netlify bot commented Apr 13, 2021 •

edited

Loading

adeinega commented Apr 13, 2021 •

edited

Loading

adeinega Apr 13, 2021 •

edited

Loading

fimbault Apr 14, 2021 •

edited

Loading

adeinega Apr 15, 2021 •

edited

Loading