Thanks Denis. Here is my proposed disposition on your comments.

On Fri, Feb 3, 2017 at 8:11 PM Denis denis.ietf@free.fr wrote: Comments on I-D Action: draft-ietf-oauth-jwsreq-11.txt

Two editorial comments first :

1. Guidance is a mass noun, ##

not a count noun, plural doesn't make sense. Please change "guidances" into "guidance" twice in Section 11.

Accepted.

Thanks.

2. In Section 12 : Please remove my name (Denis Pinkas) from this section.

Accepted.

Other comments:

3. Section 1.1 (from boiler plate) states: ##

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119 [RFC2119].

There is not a single other occurrence of the word SHALL within this document. In such case, I wonder how this document can be normative. There are however many (useful) "non-normative examples. A non-normative example does not replace requirements. Noted. There are bunch of MUST. At IETF we usually prefer MUST to SHALL, unlike in ISO.

Section 4 states:

A Request Object (Section 2.1) is used to provide authorization

request parameters for an OAuth 2.0 authorization request. It

contains OAuth 2.0 [RFC6749] authorization request parameters

including extension parameters.

RFC 6749 contains 75 pages, but does not contain a single occurrence of the wording "authorization request parameter" nor of "extension parameter". There should be either references to one or more specific sections of this document or, even better, a list of the mandatory/recommended/possible authorization request parameters as well as a list of mandatory/recommended/possible extension parameters should be included in this document.

A clear distinction should be made between the parameters used to authenticate the request and the other ones.

Reject.

There are 4 flows in RFC6749. In each flow, there is a sub-section dedicated to the Authorization request. In them, the parameters used in the authorization request are very clearly indicated. For example,

4.1.1. Authorization Request

The client constructs the request URI by adding the following parameters to the query component of the authorization endpoint URI ...

It is very difficult to miss.

Then, the possibility for the extension parameters are discussed in 8.2. Needless to say, those extension parameters are going to be discussed in other specifications. Thus, it would be misleading just to say the parameters defined in 4.1.1, 4.2.1, etc. As an editor, I feel better with the current language because it is at least not wrong nor misleading.

4. The introduction states on page 4:

 (a) (integrity protection) The request can be signed so that the integrity of the request can be checked ;

This should be changed into:

 (a) (integrity protection) The request can be authenticated either using a digital signature or using encryption under a secret key 
      so that the integrity of the request can be checked ;

Reject.

This paragraph is talking about the integrity protection and not the source authentication. And even for source authentication, saying that encryption under a secret key is not accurate as it was discussed earlier in the WG mail.

I am not sure if "Introduction" needs to state everything that is explained later. The idea of introduction probably is to give main points. The list is not an exhaustive list of the benefit of using JWT as the authorization request format. For example, being able to encrypt the request, which is not listed there, has an advantage of preventing MITB to eavesdrop the request. So I think it is ok as is.

5. The introduction states on page 4:

(d) (collection minimization) The request can be signed by a third party attesting that the authorization request is compliant to certain policy. The request is not signed by a third party.

However, later on, there is the following explanation:

In addition, it allows requests to be prepared by a third party so that a client application cannot request more permissions than previously agreed.

If it is the intent, the sentence should be rephrased as:

(d) (collection minimization) The request can be verified by a third party attesting that the authorization request is compliant to certain policy.

Reject The third party indeed signs the request on behalf of the client as the result of verification that the permission is the same as previously agreed.

6. Section 10.1. the text states:

When sending the authorization request object through "request"

parameter, it MUST either be signed using JWS [RFC7515] or encrypted

using JWE [RFC7516] with then considered appropriate algorithm.

The wording" with then considered appropriate algorithm" is too vague. This should be changed into:

When sending the authorization request object through "request"

parameter, it MUST either be signed using JWS [RFC7515] or encrypted

using JWE [RFC7516] using a symmetric key algorithm.

Reject. In the above sentence, "with then considered appropriate algorithm" applies both on JWS and JWE. The intent of the phrase is that a vulnerable algorithm should not be used.

Also, I do not understand why the algorithm has to be symmetric key algorithm.

7. Section 10.2 states:

This means that the request object is going to be prepared fresh each

time an authorization request is made and caching cannot be used.

What are the implications ? Is it required/recommended to use a nonce ? The text should be made clearer.

Reject. The implication is given right after the sentence. There is no variable called "nonce" in RFC6749. Since this document is just defining another encoding method for OAuth 2.0 authorization request as a framework, it does not mandate these. An extension specification should define those requirements.

8. Section 10.3 states:

10.3. Request Source Authentication

The source of the Authorization Request MUST always be verified.

There are several ways to do it in this specification.

(a) Verifying the JWS Signature of the Request Object.

It seems that the case of using a JWE encrypted using a secret key algorithm has been forgotten here. Please add it.

Accepted with modification. You mean, symmetric key algorithm, is that right? I would add "or verifying that the symmetric key for the JWE encryption is a correct one"

9. Section 10.3 states at its very end:

An extension specification

should be created as a preventive measure to address potential

vulnerabilities that have not yet been identified.

Writing a document for vulnerabilities that have not yet been identified is speculative. It would rather be better either to remove this sentence or to explain what is meant by it.

Reject.

It is referring to the first paragraph of the sub-section. Also, precaution when security is in question is a good thing.
10. Section 11.1 states:

11.1. Collection limitation

When the Client is being granted access to a protected resource

containing personal data, the Client SHOULD limit the collection of

personal data to that which is within the bounds of applicable law

and strictly necessary for the specified purpose(s).

The presentation of personal data should be limited whether or not the protected resource contains personal data.

It is proposed to change this text into:

When the Client requests an access to a protected resource, the Client

SHOULD limit the presentation of personal data to that which is within

the bounds of applicable law and strictly necessary for the specified

purpose(s).

Reject.

You are not getting what OAuth does. The party that holds personal data is the authorization server / resource. It is not the client. The client is the party who is getting those "resources" which may contain personal data. Yes, the client can provide some personal data to the resource depending on what that resource endpoint is, but that is out of scope for OAuth. As far as OAuth is concerned, what is being sent from the client to the resource is the access token.

11. Section 11.2.1 states:

11.2.1. Request Disclosure

This specification allows extension parameters.

It would be useful to name either all of them or some of them. RFC 6749 is not crystal clear about this.

Noted.

RFC6749 only defines how to define extension parameters. This specification draws from OpenID Connect for some examples of extension parameters such as nonce. See section 4 for example.

Denis Pinkas (DP Security Consulting SAS)

DP Comments for the Last Call

Two editorial comments first :

1. Guidance is a mass noun, ##

2. In Section 12 : Please remove my name (Denis Pinkas) from this section.

Other comments:

3. Section 1.1 (from boiler plate) states: ##

Section 4 states:

4. The introduction states on page 4:

5. The introduction states on page 4:

6. Section 10.1. the text states:

7. Section 10.2 states:

8. Section 10.3 states:

9. Section 10.3 states at its very end:

11.1. Collection limitation

11. Section 11.2.1 states:

Comments (3)