Re: [GNAP] Summarizing status from SECDIR IETF LC review of draft-ietf-gnap-core-protocol-16

[Justin Richer <[e-mail]>]

Hi Roman (and Russ), answers inline below.

— Justin

On Feb 7, 2024, at 1:53 PM, Roman Danyliw <[e-mail]> wrote:

Hi!

During IETF LC, SECDIR (Russ Housley) performed a review on draft-ietf-gnap-core-protocol-16.  See https://mailarchive.ietf.org/arch/msg/txauth/i9NEvnr2l1p8hW7U_l382NZbLxg/.  Justin (Richer) responded and iterated on this review. Here are the relevant messages in the thread since it is not linked from the SECDIR review link in the Datatracker.

(Justin-2023-11-27) https://mailarchive.ietf.org/arch/msg/txauth/-Zl-mKGKC0WgujAlj8cd3sOgr2Q/
(Russ-2023-11-28) https://mailarchive.ietf.org/arch/msg/txauth/ZArrGJN06Tsyi1o9g48P2J4fzIo/
(Justin-2023-11-29) https://mailarchive.ietf.org/arch/msg/txauth/Ek0NCQeiUfuiJv7EdSFzPq2s-ao/

A -17 of the document has been released.  I very much appreciate the deep review and iteration.  I am trying to re-review the document and identify unresolved issues.  The following is my assessment.  The inline thread was broken at some point making it difficult to attribute text so I'm repeating the initial feedback from the SECDIR review here and annotating it with [Roman].  Please consult the above links for some of the details.  I'm looking for resolution of the issues below with further discussion or a document change; or a correction that that this issue been addressed in a way I didn't immediately recognize.

** (Russ on -16) Section 1.2: I am having trouble getting my head around the idea that
the AS role grants subject information to client instances.  This feels
like a purposeful confusion between authentication and authorization.

** (Russ on -16) Section 1.3: Elsewhere in the document, there is a careful distinction
between software that acts on behalf of a subject and the natural person
that uses the software.  This distinction is missing in the definition
of Subject.  It is made worse by including organizations.  Please say
how people and organizations "decide whether and under which conditions
its attributes can be disclosed to other parties."  I believe this is
done by interacting with the software or initial configuration of the
software.

[Roman] Is it possible to be specific on where there is a mismatch?

GNAP is fundamentally a delegation protocol, which can delegate both authorization to call APIs (a la OAuth) and the identity of the user (a la OpenID Connect) in the same flows. I do not see a confusion between the authorization and authentication aspects here, especially as they are deliberately requested and returned separately from each other.


** (Russ on -16)  Section 2.1.2: I expected a parallel structure to Section 2.1.1.  I
think it would really help the reader if the sections had the same
structure.

These two sections are not intended to have parallel structure. Section 2.1.1 says "these are the bits you need to use to get an access token" and 2.1.2 says "to get more than one access token use an array of the things in 2.1.1".  The "one or more" language was a bug and was fixed in -17; otherwise, all the fields are used the same way and I don’t see a value in repeating them in this section — I think that would actually confuse the reader more.


** (Russ on -16) Section 2.3.2: I am not worried about logo_uri if only data: URIs are
allowed, but that does not seem to be the case.  Since the logo might
be fetched, there need to be an integrity protection mechanism to
ensure that the web server does not provide a different image than
was intended.  RFC 3709 has this same concern and it uses a hash of
the image to ensure that the intended image was provided.

[Roman] Section 13.15 has several relevant considerations.  Building on Russ’s point of having confidence in fetched image and the intent to provide flexibility, isn’t there a residual security considerations to document where an AS provides a URI which

Both the value and security consideration come from the client being able to swap out the image without updating the AS.

I’ve proposed an additional security consideration just for this particular aspect in this PR:

https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/524


** (Russ on -16)  Section 2.5.1: I do not understand what an implementer is supposed to
do based on this MUST statement:

  "All interaction start method definitions MUST provide enough
  information to uniquely identify the grant request during the
  interaction."

[Roman] These seems like guidance which should be included (moved to?) Section 11.8 which contains the evaluation criteria for the DE.  That sections already has a number of items to consider.


I’ve put together a PR to move this to the IANA section.

https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/525

** (Russ on -16)  Section 2.5.2.1: How does the use of an application-specific URI scheme
provide for the same security as HTTPS?  It seems like an way to avoid
them.  This text appears several places in the document.

[Roman] This doesn’t appear to have been resolved.  There was an indication that improved definitions would be added to Section 1.1

An application-specific URL is not loaded across an untrusted network connection and is therefore equivalent in protection to plain HTTP on localhost. I’ve added that text explicitly in all the places I could find:

https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/526

I don’t believe it needs an additional definition.


** (Russ on -16) Section 7: Please consider a reference to RFC 4107.  I'm not sure where
in this section is the best place to add a cite.

[Roman] -16 already contained a reference to RFC4107 in Section 13.7.  The discussion thread seems to indicate that additional pointers would be added in a version after -16.  I don't see it.

It seemed to make more sense to keep the reference in the security considerations section as opposed to making a reference to it (normative or otherwise) in the client-signing section (7). Yes, clients and AS’s need to manage and store their keys well. But the thrust of section 7 is how those are presented on the wire and used to bind request messages to keys, not how the keys themselves are managed. There’s already a forward reference to the security section that mentions RFC4107 in section 7, so we felt that was sufficient.


** (Russ on -16) Section 9.1: I do not understand what an implementer is supposed to
do based on this MUST statement:

  "client instance MUST check its value to protect itself"

(Justin) The client instance checks whether the value of the `referrer` parameter is the URI of the resource server that it made the request to. The rationale for this is discussed a little more in §13.36 (https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-16.html#security-compromised-rs).

(Russ) What checks MUST the client instance perform?  Section 13.36 does not address my concern.  Section 13.36 is about protections that are implemented at the resource server.

(Justin) The client instance compares the referrer parameter returned to the URL of the RS.

[Roman] Perhaps this clarifying guidance can be added directly into the text.

I’ve put together a PR to clarify this, as well as straighten out the language around the requirements of what’s returned from the RS and what the client is supposed to do with them.

https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/527


** (Russ on -16) Section 13.24: This section does not seem like a Security Consideration.
If the calculation of the interaction hash is not done the same, then
there will be interoperability issues.

[Roman] Doesn’t seem like we have resolution on the issue.

It’s a security consideration because the client can choose to not calculate and check the value, which would leave it open to the described attack. The mitigation of the attack is to perform the function as specified in the document. This section explains why it’s important, which I believe is a valid use of a security considerations section.


Regards,
Roman