Re: [regext] I-D Action: draft-ietf-regext-rdap-openid-15.txt

[Rick Wilhelm <[e-mail]>]

Scott,

A brief response to the Section 4.5 item, you can search for “[RW]” to find it quickly.

And the suggested edit for Section 5 looks great.  Also marked for clarity.

Thanks,
Rick

From: Hollenbeck, Scott <[e-mail]>
Date: Tuesday, July 5, 2022 at 2:17 PM
To: Rick Wilhelm <[e-mail]>, [e-mail] <[e-mail]>
Subject: [EXTERNAL] RE: [regext] I-D Action: draft-ietf-regext-rdap-openid-15.txt
CAUTION: This email came from outside your organization. Don’t trust emails, links, or attachments from senders that seem suspicious or you are not expecting.
________________________________
Notes below, Rick. Thanks for the feedback.

From: Rick Wilhelm <[e-mail]>
Sent: Thursday, June 23, 2022 4:07 PM
To: [e-mail]; Hollenbeck, Scott <[e-mail]>
Subject: [EXTERNAL] Re: [EXTERNAL] [regext] I-D Action: draft-ietf-regext-rdap-openid-15.txt


Caution: This email originated from outside the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
Scott,

Thanks for your ongoing work on rdap-openid.  I am, by no measure, an expert in OpenID, so some (all?) of this feedback may miss the mark.  But perhaps some will be helpful.

I don’t think that any of the below is new to the -15 version.  (The second item is from -14).  But I’m hoping that this feedback helps to move this draft forward, as I think that it’s an important step for RDAP.

These are in the order provided in the doc, not in any order of importance.

First some more macro things:

Section 3.1.1:  Terminology

For this section, which currently focuses on RDAP terminology, I think that it would be helpful echo something like the terminology section of RFC 8414 (an Informative Reference from 12.2).    Pasting a snippet…

   This specification uses the terms "Access Token", "Authorization
   Code", "Authorization Endpoint", "Authorization Grant",
   "Authorization Server", "Client", "Client Authentication", "Client
   Identifier", "Client Secret", "Grant Type", "Protected Resource",
   "Redirection URI", "Refresh Token", "Resource Owner", "Resource
   Server", "Response Type", and "Token Endpoint" defined by OAuth 2.0
   [RFC6749]; the terms "Claim Name", "Claim Value", and "JSON Web Token
   (JWT)" defined by JSON Web Token (JWT) [JWT];

(I’ll offer that JWT := RFC 7519, already a Normative Reference.)  The actual text would need a careful edit, I just pasted this.

[SAH] OK, good idea. I’ll add this and check the term set.

Section 4.5:

Regarding the text:

   Alternatively, an RDAP server MAY implicitly attempt to refresh an
   access token upon receipt of a query if the access token associated
   with an existing session has expired and the corresponding OP
   supports token refresh.  The default RDAP server behavior is
   described in the "implicitTokenRefreshSupported" value that's include
   in the "roidc1_openidcConfiguration" data structure (see
   Section 4.1.3).  If the value of "implicitTokenRefreshSupported" is
   "true", the client MAY either explicitly attempt to refresh the
   session using the "roidc1_session/refresh" query, or it MAY depend on
   the RDAP server to implicitly attempt to refresh the session as
   necessary when an RDAP query is received by the server.  If the value
   of "implicitTokenRefreshSupported" is "false", the client MUST
   explicitly attempt to refresh the session using the "roidc1_session/
   refresh" query to extend an existing session.  If a session cannot be
   extended for any reason, the client MUST establish a new session to
   continue authenticated query processing by submitting a
   "roidc1_session/login" query.  If the OP does not support token
   refresh, the client MUST submit a new "roidc1_session/login" request
   to establish a new session once an access token has expired.

If the server has “true” for “implicitTokenRefreshSupported”, there doesn’t seem to be a MUST requirement on the server to actually attempt a refresh.   Is that on purpose?
[SAH] No, I’ll add a “MUST”.

In the last sentence: Is the client required to wait until the access token has expired before submitting the new login request?  Or can it send logout and login back-to-back?  (Or even just a login command while currently logged in?)
[SAH] Let’s talk about this. What’s appropriate behavior? IF the server gets a “login” during an active session, it can either ignore the second “login”, or it can return an error. Similarly, it the server gets a “logout” when there’s no active session, it can either ignore the “logout” or return an error. I’m inclined to return an error to explicitly note that the submitted query/command wasn’t processed as requested.

[RW] First off, I will certainly defer to those with more implementation in this realm.  However, based on my experience as a user, I would expect a login that happens during an active session to “just work” and override the previous active session.  This could happen when I have an active session at the server but the client browser (with the session) crashes or is otherwise inaccessible.  This seems better than the alternative:  If the new login request is refused, then the user is (essentially) locked out until the session timeout value expires.  Related, if the server gets a “logout” when there is no active session, I think that it should ignore the “logout” (rather than returning an error).  The thinking being that returning an error is at best useless and at worst could be an information leak (aka security risk).

Nit:  This block might be easier to parse if broken into multiple paragraphs.
[SAH] Will do.

Section 10.  Security Considerations:

This section defers the security considerations for OIDCC to that specification.  Section 3.1.2 contains the statement:  “Communication with the Authorization Endpoint MUST use TLS.” Section 5.3 makes a similar statement related to the UserInfo Endpoint However, due to its publication date (2014) the document does not appear to have a prohibition on older versions of TLS.  Perhaps there should be some mention of BCP 195 (RFC 8996) in this draft?
[SAH] Agreed! Will add.


Now on to nits/semi-nits:

Section 3.1.3.1:  para 3:

Regarding:
An RDAP server MUST support at least one of these
   methods of OP discovery.

I think that would be more clear as:
An RDAP server/RP MUST support at least one of these
   methods of OP discovery.
(which would be consistent with para 1 in this section).  The main point is that we don’t want to imply that all RDAP servers need to support this.  You might choose to clarify by just switching to “RP”?
[SAH] I’ll make the change to “RDAP server/RP”.


Section 3.1.3.3:

I think that for clarity, it would be helpful to use the word “consent” somewhere in the first sentence of this section.  This makes for stronger linkage with 3.1.2 Step 6 and the heading in OIDCC 3.1.2.4.
[SAH] OK, I’ll change the first sentence to “After the End-User is authenticated, the OP MUST obtain consent from the End-User to release authorization information to the RDAP Server/RP.”.


Section 3.1.3.4:

Regarding:
   After the End-User is authenticated, the OP will send a response to
   the RP that describes the result of the authorization process in the
   form of an Authorization Grant.

The beginning of the sentence seems to focus on authentication (not authorization) and only on the successful path.  It also uses the term “Authorization Grant”, which is inconsistent with 3.1.2 Step 7 and my reading of OIDCC 3.1.1.  Suggest clarifying with:

   After obtaining an authorization result, the OP will send a response to
  the RP that provides the result of the authorization process using an
  Authorization Code.
[SAH] Agreed, I’ll make that change.


Section 3.1.3.5:

Regarding:

The RP MUST validate the Token
   Response.  This process is described in Section 3.1.3 of the OpenID
   Connect Core protocol [OIDCC].

I think that the reference could be more specific and point to OIDC 3.1.3.5
[SAH] OK.


Section 3.1.3.6:

Regarding:

   The set of claims can be retrieved by sending a request to a UserInfo
   Endpoint using the Access Token.  The claims MAY be returned in the
   ID Token.

I think that the structure of the second sentence and the use of the capitalized “MAY” doesn’t quite capture the intent.  My read of OIDCC 5.3 indicates that the UserInfo Endpoint is not required to return UserInfo Claims (for various reasons), but if they are going to be returned, they are going to be returned in the ID Token.

Perhaps replace the second sentence with:
   The server provides returned claims in the ID Token.
[SAH] Agreed.


Section 3.1.4:

Regarding:

   OpenID Connect claims are pieces of information used to make
   assertions about an entity.  Section 5 of the OpenID Connect Core
   protocol [OIDCC] describes a set of standard claims that can be used
   to identify a person.

My read of OIDCC Section 5 didn’t indicate anything about a person.  Suggest the following edit:

   OpenID Connect claims are pieces of information used to make
   assertions about an entity.  Section 5 of the OpenID Connect Core
   protocol [OIDCC] describes a set of standard claims.

(Which also happens to align with OIDCC 5.1, almost verbatim.)
[SAH] Agreed.


Section 3.1.4.1:

Regarding:

   There are communities of RDAP users and operators who wish to make
   and validate claims about a user's "need to know" when it comes to
   requesting access to a resource.  For example, a law enforcement
   agent or a trademark attorney may wish to be able to assert that they
   have a legal right to access a protected resource, and a server
   operator will need to be able to receive and validate that claim.

Suggest minor edits to avoid needing to support certain statements.  As in:

   Communities of RDAP users and operators may wish to make
   and validate claims about a user's "need to know" when it comes to
   requesting access to a resource.  For example, a law enforcement
   agent or a trademark attorney may wish to be able to assert that they
   have a legal right to access a protected resource, and a server
   operator may need to be able to receive and validate that claim.
[SAH] Agreed.


Section 3.1.4.2:

Regarding:

   There are also communities of RDAP users and operators who wish to
   make and validate claims about a user's wish to not have their
   queries logged, tracked, or recorded.  For example, a law enforcement
   agent may wish to be able to assert that their queries are part of a
   criminal investigation and should not be tracked due to a risk of
   query exposure compromising the investigation, and a server operator
   will need to be able to receive and validate that claim.

As above, suggest minor edits to avoid needing to support certain statements.  As in:

   Communities of RDAP users and operators may wish to
   make and validate claims about a user's wish to not have their
   queries logged, tracked, or recorded.  For example, a law enforcement
   agent may wish to be able to assert that their queries are part of a
   criminal investigation and should not be tracked due to a risk of
   query exposure compromising the investigation, and a server operator
   may need to be able to receive and validate that claim.
[SAH] Agreed.


Section 4.3.1:

Regarding:

If this parameter is not present, the server
   MUST proces the query and make an acces control decision based on any
   other information known to the server about the End-User and the
   information they are requesting.

Suggested minor edit to remove the ambiguity of “this parameter” (and fix two typos)

If the “roidc1_qp” parameter is not present, the server
   MUST process the query and make an access control decision based on any
   other information known to the server about the End-User and the
   information they are requesting.
[SAH] Agreed.


Section 4.6:

Regarding:

   The server should also take appropriate steps to ensure that the
   tokens associated with the terminated session cannot be reused.

Suggest an edit to move this “should” to “SHOULD”
[SAH] Agreed.


Section 4.7:

Regarding:

Servers MUST reject queries
   that include identification information that is not associated with a
   supported OP by returning an HTTP 501 (Not Implemented) response.

For clarity, suggest adding “RDAP” before “Servers”… as in:

RDAP servers MUST reject queries
   that include identification information that is not associated with a
   supported OP by returning an HTTP 501 (Not Implemented) response.

(Or if this doesn’t refer to the RDAP Server, then something else to disambiguate.)
[SAH] Agreed.


Section 5:

Regarding:

   ID tokens include an audience parameter that contains the OAuth 2.0
   client_id of the RP as an audience value.

>From my read of OIDCC Section 2 (“ID Token”), it appears that this “audience parameter” refers to an item that is described as a “claim” in that document.

However, later in Section 5, there is a reference to RFC 8693, which in Section 2.1 describes an “audience parameter”.

So, I’m not sure which is correct… perhaps something to direct the reader.  (Or maybe they are largely equivalent and my ignorance is on full display!)
[SAH] They’re related to each other (8693 describes the request, OIDCC describes the output), but yes, the ID Token contains a claim so the text can be clarified. How about this:

“ID tokens include an "aud" (audience) claim that contains the OAuth 2.0 client_id of the RP as an audience value. In some operational scenarios (such as a client that is providing a proxy service), an RP can receive tokens with an "aud" value that does not include the RP's client_id. These tokens might not be trusted by the RP, and the RP might refuse to accept the tokens. This situation can be remedied by having the RP exchange these tokens with the OP for a set of trusted tokens that reset the "aud" claim.”

[RW] Good upgrade!

Hope these help…

Thanks,

Rick