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

[Russ Housley <[e-mail]>]

Justin:

>>>> 
>>>> 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.
>> 
>> I got this from the discussion that we had following the SECDIR review.  However, I think that saying something about authentication in the role definition in Section 1.2 would be very helpful.
>> 
>> I am still confused about how a device can be an RO.  See my message from earlier today.
>> 
> 
> As per discussion on the other thread, I’ve changed the introduction here that hopefully makes this intent more clear without changing the intent of the text:
> 
> https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/529

I find this very helpful.  Thanks.  I hope it helps future readers too.

>>>> ** (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
>> 
>> Great improvement!
>> 
>> I am not sure what you mean by a "sanitized and generic URI".  Do you have a reference or short description?
> 
> I really just meant a URI that doesn’t have security sensitive elements in it, I’ve changed the PR to say that instead.

Thanks. This one is resolved.

>>>> ** (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.
>> 
>> Nit:  Would it change your intent to say "that already exists on the end user's device."
>> 
>> The point is that it is already there, so no network access is needed at all.
> 
> Since the context of each of these statements is a URL that the client instance knows it can serve, I think it stands to reason that the application already exists on the device since it’s the same application that’s creating the initial request. Furthermore, there’s no way for an AS to enforce the installation state on the end-user’s device. With that, I don’t think it’s helpful to add the additional text here.

Okay.  I accept that point.

>>>> ** (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.
>> 
>> I really dislike the notion of a symmetric key being used for "signing".  In my view, digital signatures require asymmetric cryptography.  Is there a way to avoid this in the first paragraph of Section 13.7.
> 
> I think we can add same definitions paragraph that’s in the HTTP Message Signatures spec introduction, which I’ve added in this PR:
> 
> https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/528

Thanks.  I'd like to see a sentence or two about the difference in the security service offered by a MAC.  RFC 5652 has these words that capture my point:

   When more than two parties share the same message-authentication key,
   data origin authentication is not provided.  Any party that knows the
   message-authentication key can compute a valid MAC; therefore, the
   contents could originate from any one of the parties.

You might also want to say that MAC computation is oftem much more efficient than an asymmetric digital signature.

With these two points, an implementer gets the information to decide which type of algorithm to use.

>>>> ** (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
>> 
>> This is now answering my concern.  However, the sentence is incomplete: "... plain string comparison method in ."  I assume you meant to insert a reference at the end of that sentence.
> 
> Oh thanks, I’ve added the reference to the existing PR — it was meant to point to the simple string comparison of URI.

Thanks. This one is resolved.

>>>> ** (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.
>> 
>> I think you do not understand my comment.  You specify the way that the interaction hash is calculated.  Everybody needs to do it as specified.  Failure to do so is an interoperability issue.  This section talks about the protections that are offered by checking the interaction hash.  Failure to perform the check is a security issue.
> 
> I’ve added some framing text to the PR here, hopefully this helps clarify the purpose of the section in question.
> 
> https://github.com/ietf-wg-gnap/gnap-core-protocol/pull/530

Thanks. This one is resolved.

Russ