Re: [secdir] Secdir last call review of draft-ietf-oauth-jwsreq-30

[Watson Ladd <[e-mail]>]

On Sat, Oct 31, 2020 at 6:13 AM Nat Sakimura <[e-mail]> wrote:
>
> Hi Watson,
>
> Thanks very much for the review. I thought I have sent my response
> earlier, which I actually did not. It was sitting in my draft box. I
> apologize for it.

My apologies for missing it in my inbox for a number of months.
>
> My responses inline:
>
> On Sat, Sep 26, 2020 at 9:46 AM Watson Ladd via Datatracker
> <[e-mail]> wrote:
> >
> > Reviewer: Watson Ladd
> > Review result: Serious Issues
> >
> > I generated this review of this document as part of the security directorate's
> > ongoing effort to review all IETF documents being processed by the IESG.  These
> > comments were written with the intent of improving security requirements and
> > considerations in IETF drafts.  Comments not addressed in last call may be
> > included in AD reviews during the IESG review.  Document editors and WG chairs
> > should treat these comments just like any other last call comments.
> >
> > Two minor issues: On page 4, "This offers an additional degree of privacy
> > protection." should be reworded. I don't think it makes sense in context, where
> > authenticity was discussed.
>
>
> In the course of the edit, explanation about two distinct privacy
> benefits was collated in one sentence and has become very difficult to
> parse.
>
> What it is trying to express as privacy benefits are the following.
>
> 1) The authorization request content is sent to the AS in the
> backchannel so it will not be exposed through the browser to the eyes
> of an active or passive outsider observing what is going on in the
> browser.  In the RFC6749 framework case, the authorization request
> goes through the browser redirect and it could leak to the adversary
> via WPAD/PAC Attack, referrer, browser history, etc. Also, if the
> browser was infected by an adversary controlled malware, the content
> can be sniffed by the adversary. In the case of JAR, it does not
> happen. This is one privacy benefit it is trying to explain.
>
> 2) The location that the authorization request is getting pushed to
> does not have to be the AS. A trusted third party that examines the
> content for the conformance to the collection minimization principle
> may act as the party that accepts the authorization request and issues
> the request_uri. AS can then just evaluate the domain part of
> request_uri to evaluate that the authorization request is conformant
> to this principle. This is another privacy benefit from the point of
> view of the individual user.

I'm fine with any fix to the sentence that makes sense. Don't think we
need to insert the above but I very much appreciate the explanation.

>
>
> > It took me a while to understand what the by reference method is: maybe the
> > intro should say via URL instead of by reference.
>
>
> request_uri can be URL or a handle such as URN. That is why the "by
> reference" word is being used, per the suggestion of the WG.

I'm fine with that, just noting my confusion.

>
> >
> >
> > And now for the thorny issues with this draft. Signatures and encryption are
> > different. And encrypting a signed blob doesn't mean the signer encrypted it.
> > Then there are a plethora of methods specified in the draft  to authenticate
> > the blob, which will give different results in maliciously constructed
> > examples. The security considerations section should discuss what the encrypted
> > vs signed choices give in the way of security, and it doesn't. This makes me
> > worry.
>
> We don’t expect the encryption to ensure authenticity, that’s what the
> signatures are used for.

This needs to be very clearly spelled out in the text. Lots of people
will not understand this. The wording of section 10.2 is at best
ambivalent, with multiple alternatives presented as acceptable.

>
<chop>
>
> I didn't quite get what is meant by "plethora of methods specified in
> the draft to authenticate the blob ... "
> There is a bit of text about authenticating the source (=client) but
> not much on the blob itself.
> The discussion around the signature and/or encryption is covered in
> RFC7519 (JWT), the format that the request object assumes.
> This is required reading when implementing this spec, so WG thought it
> is not worth repeating here.
> Attacks etc. on the signature and encryption are covered in RFC7515
> and RFC7516 respectively.

Well, the draft happens to include the following text:
   "The Authorization Server MUST validate the signature of the JSON Web
   Signature [RFC7515] signed Request Object.  The signature MUST be
   validated using the key for that "client_id" and the algorithm
   specified in the "alg" Header Parameter."

Shouldn't the key be associated with a single algorithm? How do we
ensure that the common attack of telling the server to use hmac to
verify the signature doesn't work here?

>
> >
> > Looking at the cited reference for attacks, I see the fix is to include
> > information about which IPD was used by the RP. But the draft before us doesn't
> > mandate that. It's not clear than how the cited attack is prevented by the
> > draft. Saying that the communication through the user-agent is subject to
>
> The mention of mix-up attack was introduced after the Last call by one
> of the comment. It just added it in the sentence with a reference. I
> am ok to remove it.

That works for me.

>
> Having said that, the heart of mix-up attack is that the combination
> of the client believes that it is communicating with the
> attacker-controlled AS (AAS) while it in-fact is talking to Honest AS
> (HAS), AND HAS unable to find out that the client is thinking that it
> is talking to AAS not him.
>
> OAuth JAR seems to mitigate it in two ways:
>
> a) Use request_uri which is hosted by the AS. Then, if the client is
> thinking that it is talking to the AAS, then it will push it to AAS
> and when the user is redirected to HAS, HAS will find out that the
> request_uri is not created by herself and return an error, making the
> mix-up attack fail.
>
> b) Include `aud` in the request. Then, when the HAS will find that the
> request was minted to AAS and not her. So, it will result in an error,
> making the mix-up attack fail.

If the draft mandates doing this it addresses the attack and the
sentence can stay.

>
> So, I added mix-up attack to the sentence thinking the commenter's
> request to add it is fine, but I am fine with removing it.
>
> > manipulation, and this prevents it, ignores that the attacker in that position
> > sees a lot more. The user-agent as resource owner modifying the requested
> > resources is a very funny sort of attack: can't they do what they want with the
> > resources since they control the access?
>
> If the client is in the browser, yes.
> But in the mainstream case, the client is not in the browser but the
> web-server that the browser is communicating with and the resource
> access happens without being mediated by the browser.

My concern on this point is resolved.

>
> >
> >
> > Key management is ignored. This is a very important issue, especially
>
> A lot of ground is covered by RFC 7515, 7516, 7517, 7518, 7519, 7591,
> and 8414 so this document is not specifically restating them.
>
> >
> > considering the potential problems with the reuse of JWT. I'd like to see a
>
> Are you talking about the reuse of the request object by an adversary
> trying to act as an honest client?
> Even if it happens, the malicious client does not have the proper
> client credential so it cannot redeem the code it obtained with the
> token. It is no different than RFC6749 code grant. Protocols that
> extend it, such as OpenID Connect, have introduced nonce to prevent
> the reuse and used JAR (it is called request object there) to further
> protect tampering and achieve client authentication even in the front
> channel.
>
> > recommendation that keys be separated by intended uses, rather than limiting
> > particular fields in an ad-hoc manner.
>
> Could you kindly elaborate on the "ad-hoc manner" part so that I can
> understand it more fully?

10.8, Cross-JWT Confusion discusses avoiding signing certain fields,
rather than suggesting good key usage as a solution.

>
> >
> >
> > Then we have section 11. What section 11 introduces is an entire new dramatis
> > personae, the Trust Framework Provider, with no prior discussion of what it is
> > or a reference to where it is defined and a good number of statements about how
> > it works that aren't really  clear what they mean from the document to me.
>
> Trust Framework Provider first appears in 5.2.1.
> At the time of writing the related text, it was a pretty well-known
> concept. In the United State, it was part of its National Strategy
> (NSTIC) and internationally, it was even taken up at WEF Davos
> meeting. It is quite surprising that such a mainstream concept faded
> into obscurity so quickly. The reason for introducing it was to a)
> justify request_uri as some WG members wanted it to be removed, b)
> justify that requst_uri to be served from a different domain. Now that
> people appreciate it, e.g., it can be seen from PAR, the justification
> for a) probably is no longer required. A full explanation for b) would
> probably be a much longer text but I doubt if it belongs to this
> document. I am fine with removing the reference to Trust framework
> etc. as long as the capability to push the authorization request to a
> place other than the client or the authorization server is not
> removed.

Let's remove the text then, but keep the capability.


>
> >
> > My biggest concern is that these issues are signs that the problem this draft
> > is trying to solve and the mechanisms to solve it haven't been analyzed as
> > thoroughly as they should have been. Without that sort of thorough analysis
> > it's not certain that the mechanisms actually solve the problem and it's not
> > clear what the recommendations to implementers have to be to preserve those
> > properties.
>
> OAuth JAR, as the name "The OAuth 2.0 Authorization Framework: JWT
> Secured Authorization Request (JAR)" suggests, is a framework and not
> a house itself. One such example is FAPI [1] which was formally
> verified [2].

"It's possible to use this draft security" I don't think should be
enough anymore. Rather it should be impossible to use insecurely.

>
> [1] https://bitbucket.org/openid/fapi/src/master/Financial_API_WD_002.md
> [2] https://arxiv.org/abs/1901.11520
>
> >
> > Obviously this draft has had a long and tortured history with multiple reviews,
> >  and what I'm suggesting needs to happen is a lot of work. But it's essential
> > in any security protocol to do this analysis and be clear about what is, and
> > what is not, protected by the protocol.
>
> OAuth JAR is nothing but just another binding to OAuth 2.0. Where RFC6749
> binds it to form encoding, it provides two additional bindings:
>     1) binding to JWT, and
>     2) binding to the pushed authorization request that is referenced by a URI.
> It is this simple. As such, it would also inherit some of the
> shortcomings in RFC6749. However, it is not this document to address
> them. It should be done by other documents so that the result can be
> encoded using the mechanisms provided in this document.

This is not a simple matter. JWT has a long and twisted history with
some pervasive errors in common libraries, and is a fairly large
standard. OAuth 2.0 is also large. Ensuring that the mapping has the
right properties is going to be a mess. If the encoding does not
respect the semantics we have a serious security issue. If
implementors assume the encoding provides properties it does not, we
again have a security issue.

>
> It is quite surprising that this fact is not getting appreciated and
> is taking such a long time to complete.
> Maybe I should delete all the explanation text and leave it with just
> the core text. Explanation and justification text for defining
> additional bindings probably are just distractions now as it is now
> appreciated and used all over the world unlike when the project was
> started.

>
> >
> > Sincerely,
> > Watson Ladd
> >
>
> Thanks again for your detailed comments.
>
> Best wishes,
>
> --
> Nat Sakimura
> NAT.Consulting LLC



--
Astra mortemque praestare gradatim