Subject: Re: [Ace] AD review of draft-ietf-ace-oauth-authz-24

From: Ludwig Seitz <[email-masked]>
Date: Tue, 1 Oct 2019 10:50:22 +0200

On 01/10/2019 05:13, Benjamin Kaduk wrote:
> On Fri, Sep 27, 2019 at 03:22:45AM -0700, Jim Schaad wrote:
>> 
>> 
>> -----Original Message----- From: Ludwig Seitz <[email-masked]> 
>> Sent: Friday, September 27, 2019 12:03 AM To: Benjamin Kaduk
>> <[email-masked]>; [email-masked] Cc:
>> [email-masked] Subject: Re: AD review of
>> draft-ietf-ace-oauth-authz-24
>> 
>> On 27/09/2019 03:51, Benjamin Kaduk wrote:
>>> Hi all,
>>> 
>>> The length of this review notwithstanding, this document is
>>> generally in good shape -- there's a bunch of localized items to
>>> tighten up, and we can flesh out the security considerations some
>>> more, but nothing too drastic should be needed.  Perhaps the most
>>> far-reaching changes needed will be to rename the "profile"
>>> claim, since that has already been allocated to OIDC Core for a
>>> very different usage.  I also made a pull request with a bunch of
>>> trivial editorial stuff that's easier to fix than describe how to
>>> fix, up at https://github.com/ace-wg/ace-oauth/pull/175 .
>>> 
>> 
>> I have a non-trivial comment on your pull request: In appendix B
>> we summarize the steps taken by an RS to process a freshly received
>> access token. You changed the suggested sequence from:
> 
> First off, thank you for spotting it and bringing the discussion
> here!  I'm sorry that you had to do so; I wasn't trying to sneak it
> in :)
> 

I didn't think so.

>> * Verify the token is from a recognized AS. * Verify that the token
>> applies to this RS. * Check that the token has not expired (if the
>> token provides expiration information). * Check the token's
>> integrity. * Store the token so that it can be retrieved in the
>> context of a matching request.
>> 
>> To
>> 
>> * Verify the token is from a recognized AS. * Check the token's
>> integrity. * Verify that the token applies to this RS. * Check that
>> the token has not expired (if the token provides expiration 
>> information). * Store the token so that it can be retrieved in the
>> context of a matching request.
>> 
>> 
>> I don't think this is a big deal, but I put the integrity check
>> later for a good reason (or so I believe): The integrity check is
>> a potentially expensive cryptographic operation. Checking that the
>> token applies to the RS is a matter of checking the audience claim,
>> and checking that the token is not expired is a matter of comparing
>> two timestamps, I consider both to be computationally much lighter
>> and therefore quicker to execute. A failure of any of those two may
>> make it unnecessary to verify the token integrity.
>> 
>> BUT! My suggested sequence only works if the token is signed or
>> MACed and not if it is encrypted. If the token is encrypted the
>> AEAD integrity check (and decryption) is necessarily the first
>> processing step.
>> 
>> Any ideas how to resolve this gracefully (i.e. without adding a
>> large amount of text) are most welcome.
>> 
>> [JLS] I normally get out of this type of problem by saying "The
>> order of the steps is not normative, any process which produces an
>> equivalent result can be used."  And then you can add a comment
>> about switching the two steps for signed items.
> 
> That seems like a reasonable approach.  FWIW, my thinking was that 
> signature validation is a great way to toss out a bunch of malformed
> input, and we tend to do it first in non-constrained environments,
> but I can see the tradeoff running the other way in some cases.
> 
> -Ben
> 

I'll formulate some text to capture this. It's an appendix that is not 
supposed to be normative anyways, rather it's intention was to serve a 
guideline to help implementers.

/Ludwig

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Ludwig Seitz <[email-masked]>
Date: Tue, 15 Oct 2019 16:07:48 +0200

Hello Ben,

thank you for your thorough review.

I have taken the liberty to add numbers to your comments in order to 
refer to them in a easier way.

I have fixed 93 your 113 and there are 20 left where I am asking for 
clarifications. These are:

  6.), 12.), 16.), 19.), 34.), 39.), 41.), 45.), 52.), 57.), 58.), 65.), 
66.), 68.), 76.), 78.), 79.), 88.), 99.), 111.)

Note that 39.) Requires input from OAuth experts. Hannes?

If you whish to inspect the changes made on other review issues, please 
consult the commits here:
https://github.com/ace-wg/ace-oauth/commits/master

Detailed comments below.

/Ludwig

> 
> Hi all,
> 
> The length of this review notwithstanding, this document is generally in
> good shape -- there's a bunch of localized items to tighten up, and we
> can flesh out the security considerations some more, but nothing too
> drastic should be needed.
> 
> 1.)
> Perhaps the most far-reaching changes needed
> will be to rename the "profile" claim, since that has already been
> allocated to OIDC Core for a very different usage.

[LS] FIXED. I renamed the "profile" claim and parameters to "ace_profile"
Note that this will require changes in all of the profile drafts as well.

> 2.)
> I also made a pull
> request with a bunch of trivial editorial stuff that's easier to fix
> than describe how to fix, up at
> https://github.com/ace-wg/ace-oauth/pull/175 .

[LS] Merged.

> 3.)
> One general note before getting into the section-by-section comments:
> 
> There's a bunch of places where we talk about parameters that "MUST be
> mapped to CBOR types as specified in Figure N"; we may want to point to
> the IANA registry as authoritative and just give the figure reference
> for description.

[LS] FIXED.

> 
> 4.)
> Abstract
> 
>    OAuth.  The framework is based on a set of building blocks including
>    OAuth 2.0 and CoAP, thus making a well-known and widely used
>    authorization solution suitable for IoT devices.  Existing
> 
> nit: It's easy to (mis)read this text as saying that just amalgamating
> OAuth 2.0 and CoAP magically makes for a solution that is well-known and
> widely used.  I'd suggest rewording slightly, perhaps "thus transforming
> a well-known and widely used authorization solution into a form suitable
> for IoT devices".

[LS] FIXED.

> 5.)
>    specifications are used where possible, but where the constraints of
>    IoT devices require it, extensions are added and profiles are
>    defined.
> 
> nit: from a rhetorical point of view, it's probably better to reiterate
> why the extensions are added and profiles defined ("to better serve the
> IoT case") than to just stop with a flat declaration of what is done.

[LS] FIXED.

> 6.)
> Section 1
> 
>    Authorization is the process for granting approval to an entity to
>    access a resource [RFC4949].  The authorization task itself can best
>    be described as granting access to a requesting client, for a
>    resource hosted on a device, the resource server (RS).  This exchange
> 
> I had to pause for a while after reading this and ponder whether I
> agreed with it.  I think that my reticence stems from some friction
> between the most generic abstract definition of "resource" and a more
> specific one used in the web/HTTP world and, to a lesser extent, the
> world of URIs and URNs in general.  The resources we are discussing here
> are not always specific named resources, but can also refer to
> attributes or capabilities mediated by a RS; similarly, we may be
> creating/modifying named resources as part of the resource access
> performed by a client in the OAuth model.  I don't think it's wise to
> diverge from the RFC 4949 definition just yet, but am still considering
> whether adding some additional clarifying text here is worthwhile.

[LS] I would argue that this specification is applicable even to the wider
definition of "resource" that you are thinking of. Since OAuth 2.0 leaves
the definition of "scope" up to the specific applications, and the ACE
framework does not change this, we can deal with both web/HTTP/CoAP 
resources
(named by URIs or URNs) and any other type of resources where the 
application
can map the resource in question to a set of scopes.
I am therefore inclined to say that this section is fine, but I'd be glad to
hear the result of your considerations on that matter.

> 7.)
> Section 3
> 
> It's probably worth adding (informative) references for HTTP/2, MQTT,
> BLE, and QUIC.

[LS] FIXED.

> 
> 8.)
>    A fourth building block is the compact CBOR-based secure message
> 
> "compact CBOR" has the "PIN number" nature and is probably redundant.

[LS] FIXED.

> 9.)
>    format COSE [RFC8152], which enables application layer security as an
>    alternative or complement to transport layer security (DTLS [RFC6347]
> 
> I'm undecided whether it's better to think of this as "application layer
> security" or "object-level security".

[LS] FIXED. Now that you mention it, I find that "object security" describes
COSE better than "application layer security". I have made the 
appropriate change.

> 
> 10.)
>    or TLS [RFC8446]).  COSE is used to secure self-contained tokens such
>    as proof-of-possession (PoP) tokens, which is an extension to the
>    OAuth tokens.  The default token format is defined in CBOR web token
> 
> nit: I think there's a singular/plural mismatch in "is an extension",
> which currently seems to be talking about the PoP tokens themselves.

[LS] FIXED.

> 11.)
> Section 3.1
> 
>    Refresh Tokens:
>       Refresh tokens are credentials used to obtain access tokens.
>       Refresh tokens are issued to the client by the authorization
>       server and are used to obtain a new access token when the current
>       access token becomes invalid or expires, or to obtain additional
>       access tokens with identical or narrower scope (access tokens may
>       have a shorter lifetime and fewer permissions than authorized by
>       the resource owner).  Issuing a refresh token is optional at the
> 
> nit: I'd suggest "such access tokens" in the parenthetical.

[LS] FIXED.

> 12.)
>       A refresh token in OAuth 2.0 is a string representing the
>       authorization granted to the client by the resource owner.  The
>       string is usually opaque to the client.  The token denotes an
> 
> Apparently I'd forgotten that this couldn't be binary.

[LS] From RFC 6749, section 1.5: "A refresh token is a string ..."
No qualifiers such as "binary" so I interpret this to mean a text-string.
Do you want us to add some clarification?

> 13.)
>    Proof of Possession Tokens:
>       An access token may be bound to a cryptographic key, which is then
> 
> Can refresh tokens not be PoP?

[LS] FIXED. I see no problem with refresh tokens using PoP as well. However
note that we have not focused on refresh tokens very much, as we have mostly
been considering M2M communication, leading to the client credentials grant
flow, which in turn makes refresh tokens seem rather superfluous.
> 
> 14.)
>       The proof-of-possession (PoP) security concept assumes that the AS
>       acts as a trusted third party that binds keys to access tokens.
> 
> nit: I think we're describing the PoP model used by ace, not the generic
> PoP concept.

[LS] FIXED: I expanded that paragraph to: "... concept used here ...".

> 15.)
>    Scopes and Permissions:
>       In OAuth 2.0, the client specifies the type of permissions it is
>       seeking to obtain (via the scope parameter) in the access token
>       request.  In turn, the AS may use the scope response parameter to
>       inform the client of the scope of the access token issued.  As the
>       client could be a constrained device as well, this specification
>       defines the use of CBOR encoding as data format, see Section 5, to
>       request scopes and to be informed what scopes the access token
>       actually authorizes.
> 
> nit: I think there's a word missing in "the use of CBOR encoding as data
> format".

[LS] FIXED: "... the use of CBOR encoding, see Section 5, for such requests
and responses."

> 16.)
> Section 3.2
> 
>    One application of COSE is OSCORE [I-D.ietf-core-object-security],
>    which provides end-to-end confidentiality, integrity and replay
>    protection, and a secure binding between CoAP request and response
>    messages.  In OSCORE, the CoAP messages are wrapped in COSE objects
>    and sent using CoAP.
> 
>    This framework RECOMMENDS the use of CoAP as replacement for HTTP for
>    use in constrained environments.
> 
> Do we have a reason to mention OSCORE if we're not going to make a
> recommendation about its use?

[LS] We also mention DTLS and TLS without making any recommendation about
which to use. I would suggest to either remove all of it or to add a 
sentence
noting that this is an enumeration of some security options, and the choice
depends on the specific application scenario.

> 17.)
> Section 4
> 
> If the description for step (C) is going to refer to the "profile (see
> step B)", then we need to mention the profile as an example of the
> Access Information in step (B).

[LS] FIXED.

> 18.)
>                                                                The RS
>       verifies that the token is integrity protected by the AS and
>       compares the claims contained in the access token with the
>       resource request.  [...]
> 
> I note that this verification of "integrity protected by the AS"
> implicitly also includes verification of the source authentication
> (i.e., signature or keyed MAC in key known only to AS and RS, in the
> non-introspection case); do we want to make that more explicit in the
> text?

[LS] FIXED. Yes.

> 
> 19.)
> Section 5
> 
>    Credential Provisioning
>       For IoT, it cannot be assumed that the client and RS are part of a
>       common key infrastructure, so the AS provisions credentials or
>       associated information to allow mutual authentication.  These
>       credentials need to be provided to the parties before or during
>       the authentication protocol is executed, and may be re-used for
>       subsequent token requests.
> 
> nit: either "before or during the execution of the authentication
> protocol" or "before or during the authentication protocol's execution".
> And just to double-check that we mean the authentication protocol of
> provisioning in the last sentence, not the authorization protocol that
> occurs between the client and RS.

[LS] The whole last sentence was a bit off. I would suggest:
"The resulting security association between client and RS may then be 
re-used
by binding these credentials to additional access tokens." Does that
sound better?

> 20.)
>    OAuth 2.0 requires the use of TLS both to protect the communication
>    between AS and client when requesting an access token; between client
>    and RS when accessing a resource and between AS and RS if
>    introspection is used.  In constrained settings TLS is not always
>    feasible, or desirable.  Nevertheless it is REQUIRED that the data
>    exchanged with the AS is encrypted, integrity protected and protected
>    against message replay.  It is also REQUIRED that the AS and the
>    endpoint communicating with it (client or RS) perform mutual
>    authentication.  [...]
> 
> This seems to leave open the possibility that client/RS interactions are
> not encrypted, integrity protected, and/or replay protected; is that
> intentional?  (We do require later on that all resource access gets the
> "usual protections" other than the authz-info endpoint, so this is
> mostly just an editorial remark.)

[LS] FIXED. No this was not intentional. Except for the authz-info endpoint,
the same requirements should be applied to C-RS communication.

> 
> 21.)
>    In OAuth 2.0 the communication with the Token and the Introspection
>    endpoints at the AS is assumed to be via HTTP and may use Uri-query
>    parameters.  When profiles of this framework use CoAP instead, this
>    framework REQUIRES the use of the following alternative instead of
> 
> nit: REQUIRES is not an RFC 2119 keyword (just REQUIRED).

[LS] FIXED.

> 22.)
>    Uri-query parameters: The sender (client or RS) encodes the
>    parameters of its request as a CBOR map and submits that map as the
>    payload of the POST request.  Profiles that use CBOR encoding of
>    protocol message parameters MUST use the media format 'application/
>    ace+cbor', unless the protocol message is wrapped in another Content-
>    Format (e.g. object security).  [...]
> 
> Is the bit starting at "Profiles that use CBOR" directly tied to the
> CBOR-map submission to the AS, or a more general requirement on all
> protocol flows (that would better be split off in its own paragraph)?

[LS] FIXED. I split the scond part off into its own paragraph.

> 23.)
> Also, some IESG members have not liked the "MUST [...], unless [...]"
> construction, as it makes the MUST "no longer a MUST".  So perhaps we
> could rephrase this as a positive statement, such as "CBOR-encoded
> protocol messages that appeare at the outermost encoding layer MUST use the
> media type "application/ace+cbor or, for CoAP, the corresponding
> Content-Format's short ID of 19".

[LS] FIXED.

> 24.)
> Section 5.1
> 
>    Instead of the initial Unauthorized Resource Request message, other
>    discovery methods may be used, or the client may be pre-provisioned
>    with the address of the AS.
> 
> nit: I suggest "pre-provisioned with an RS-to-AS mapping" so that we
> don't have the "the AS" construct that implies there is only a single
> AS.

[LS] FIXED.

> 25.)
> Section 5.1.1
> 
>    Note: These conditions ensure that RS can handle requests
>    autonomously once access was granted and a secure channel has been
>    established between C and RS.  The authz-info endpoint MUST NOT be
>    protected as specified above, in order to allow clients to upload
>    access tokens to RS (cf.  Section 5.8.1).
> 
> I have some editorial suggestions here that I'll put into a pull requeste.

[LS] merged.

> 26.)
> Section 5.1.2
> 
> It may not be worth having an attempt at a printable version of the
> cnonce in the comments in Figure 4.

[LS] FIXED.

> 
> 27.)
> Section 5.1.2.1
> 
>    o  A RS sending a "cnonce" parameter in an an AS Request Creation
>       Hints message MUST store information to validate that a given
>       cnonce is fresh.  How this is implemented internally is out of
>       scope for this specification.  Expiration of client-nonces should
>       be based roughly on the time it would take a client to obtain an
>       access token after receiving the AS Request Creation Hints
>       message.
> 
> nit: I suggest to add something like "with some allowance for slop".

[LS] FIXED.

> 28.)
> Section 5.2
> 
>    The grant type is selected depending on the use case.  In cases where
>    the client acts on behalf of the resource owner, authorization code
>    grant is recommended.  If the client acts on behalf of the resource
>    owner, but does not have any display or very limited interaction
>    possibilities it is recommended to use the device code grant defined
>    in [I-D.ietf-oauth-device-flow].  In cases where the client does not
>    acts autonomously the client credentials grant is recommended.
> 
> The context seems to suggest that this last sentence was intended as
> "cases where the client acts autonomously"; is that correct?

[LS] FIXED. Indeed that was a typo.

> 29.)
> Section 5.5
> 
>    well.  Non-constrained clients interacting with constrained resource
>    servers can use the specifications in section 3.1 of [RFC6749] and of
>    section 4.2 of [RFC6819].
> 
> nit: Section 4.2 of RFC 6819 isn't exactly a "specification" per se; we
> might want to reword this.

[LS] FIXED.

> 30.)
> Section 5.6
> 
>    For the AS to be able to issue a token, the client MUST be
>    authenticated and present a valid grant for the scopes requested.
>    Profiles of this framework MUST specify how the AS authenticates the
>    client and how the communication between client and AS is protected.
> 
> We should probably list what attributes this protection entails --
> confidentiality+integrity, of course, and we already mentioned both
> sides of the mutual authentiation; do we need anything else like replay
> protection?

[LS] FIXED. The requirements made in the heading of section 5 apply
specifically here (confidentiality, integrity, mutual AuthN,
replay-protection, request/response binding). I changed the text to 
point that out.

> 31.)
>    The default name of this endpoint in an url-path is '/token', however
>    implementations are not required to use this name and can define
>    their own instead.
> 
> We may get questions about the relationship between this "default" and
> BCP 190; unfortunately, I haven't had a time to internalize the changes
> proposed for draft-nottingham-rfc7320bis yet to see whether they would
> affect us.  (To be clear, "leave it as-is for now and see if we get
> complaints from the experts" is the right thing to do.)

[LS] Noted.

> 32.)
> Section 5.6.1
> 
>    The client sends a POST request to the token endpoint at the AS.  The
>    profile MUST specify how the communication is protected.  The content
> 
> In the previous section we said that maybe even other transports than
> coaps or https would be possible; are we limited to those that have POST
> verbs?
> Also, a similar comment as above about what attributes the protection
> entails seems to apply.

[LS] This will need a major rephrasing of the text.
I see two options here:

1.) We rewrite all parts to use a neutral language in general and specify
POST/GET etc. for transports that have these verbs.

2.) We state in the beginning that transports that do not use RESTful verbs
should use the best equivalent.

Option 1. would get a bit cluncky, while option 2. might be a bit confusing
Do you have a specific preference?

> 33.)
>    o  The "scope" parameter MAY be encoded as a byte string instead of
>       the string encoding specified in section 3.3 of [RFC6749], in
>       order allow compact encoding of complex scopes.
> 
> "scope" is a space-separated list (in the string encoding); is 0x20
> still the list separator in the binary encoding?

[LS] FIXED. I added a clarification that the syntax of a binary encoding is
up to profiles or applications and that it does not necessarily use 0x20 as
scope-token delimiter (see e.g. draft-bormann-core-ace-aif).

> 34.)
>    o  A client MUST be able to use the parameters from
>       [I-D.ietf-ace-oauth-params] in an access token request to the
>       token endpoint and the AS MUST be able to process these additional
>       parameters.
> 
> [we might get someone complaining that if support for these is mandatory
> they might as well be in the core document.  I don't mind the split, but
> be forewarned.]

[LS] The split came to pass since some parameters were expected to become
obsolete with some ongoing OAuth work and we wanted to have a document that
was independent from the core that could be overridden.
I'm not sure this still applies, since the main culprit was the "resource"
parameter defined in OAuth-resource-indicators. The remaining "req_cnf"
parameter for requesting specific confirmation claims might become 
obsolete if
the OAuth WG takes up their PoP work again.

> 35.)
>     Figure 5: Example request for an access token bound to a symmetric
>                                    key.
> 
> I wouldn't be surprised is some reviewers asked for more text to clarify
> that the AS-generated symmetric key is the default, so we don't need any
> extra parameters to indicate that's what's being requested.

[LS] Point taken. I will add some text to clarify that this is the default
behaviour.

> 36.)
> I'm a little surprised that OSCORE didn't provide some more scrutable
> diagnostic notation for the OSCORE CoAP option.  To check: h is set, n is 1,
> k is not set, the (1-byte) partial IV is 0x05, s is 5, and the rest of
> the option is the kid context?

[LS] FIXED. I added a decoded version in paratheses for the reader's
convenience and make a note about it. Note that I took the opportunity to
simplify the OSCORE option somewhat since the original example was somewhat
intricate.

> 37.)
> 
>    Figure 7 shows a request for a token where a previously communicated
>    proof-of-possession key is only referenced.  Note that the client
>    performs a password based authentication in this example by
>    submitting its client_secret (see Section 2.3.1 of [RFC6749]).  Note
> 
> RFC 6749 says about the bare "client_secret" that "[i]ncluding the
> client credentials in the request-body using the two parameters is NOT
> RECOMMENDED and SHOULD be limited to clients unable to directly utilize
> the HTTP Basic authentication scheme (or other password-based HTTP
> authentication schemes)."  I know this is "just an example", so our
> obligations are somewhat weak, but I still think we should note that
> this is not recommended by 6749 (but since there is no CoAP analogue of
> HTTP Basic, it's the only plain-password scheme available).

[LS] My take is to modify this example and remove the client_secret part.
It feels wrong to use NOT RECOMMENDED stuff, even if it only is an example.

> 38.)
> 
>    that this example uses the "req_cnf" parameter from
>    [I-D.ietf-ace-oauth-params].
> 
> Er, the "req_cnf" parameter is exactly how the previously communicated
> key is being referenced.  So we should not have a separate "Note" that
> doesn't link back to the point of the example.

[LS] FIXED.

> 39.)
>    Refresh tokens are typically not stored as securely as proof-of-
>    possession keys in requesting clients.  Proof-of-possession based
>    refresh token requests MUST NOT request different proof-of-possession
>    keys or different audiences in token requests.  Refresh token
>    requests can only use to request access tokens bound to the same
>    proof-of-possession key and the same audience as access tokens issued
>    in the initial token request.
> 
> This is perhaps something of a philosophical question, but if a refresh
> token is only usable at the token endpoint, in some sense its audience
> is definitionally the AS.  So there's a little bit of a mismatch in
> treating it as having the audience value that the access tokens issued
> from it will have.  I don't know the background for audience-restriced
> refresh tokens in regular OAuth 2.0, though, so hopefully someone can
> educate me.

[LS] I'm equally confused. I suggest that Hannes or one of the other OAuth
experts give us a hint on that one.

> 40.)
> Section 5.6.2
> 
>    Note that the AS decides which token type and profile to use when
>    issuing a successful response.  It is assumed that the AS has prior
> 
> Would it be useful to note that this can be shaped by the "req_cnf"
> contents?

[LS] I don't think token type would be affected by the requested 
confirmation
key format. The selected profile however may well be. A note to this 
effect has
been added.

> 
> 41.)
>    token_type:
>       This parameter is OPTIONAL, as opposed to 'required' in [RFC6749].
>       By default implementations of this framework SHOULD assume that
>       the token_type is "pop".  If a specific use case requires another
>       token_type (e.g., "Bearer") to be used then this parameter is
>       REQUIRED.
> 
> When we are "weakening" the formal requirements of the parent spec like
> this, we should be careful about what happens when someone is trying to
> use the ACE stuff but ends up needing to go over HTTPS like traditional
> OAuth -- do they have to fall back to the OAuth required behavior in
> that case or are we trying to preempt that as well?

[LS] The use case here is a constrained connection between client and AS
(e.g. due to a constrained client), where we try to save the bytes for
sending a token_type by defining a default. I would guess that a HTTPS based
connection would not have such constraints and would therefore be able 
to send
the token_type = "pop" as required by RFC6749. Should we explain this 
somewhere
and describe the cases where RFC6749 behaviour is warranted?

> 
> 42.)
> Given that we're requiring support for them, I think including the
> parameters from draft-ietf-ace-oauth-params in Figure 8 (and removing
> the note above it) would be helpful to the reader.  We probably also
> want to leave some room for future extensions when we talk about it
> ("summarizes the parameters that may be part of the Access Information"
> could maybe be read as restrictive, though it's clearly not meant to
> be).

[LS] FIXED

> 43.)
>      "cnf" : {
>        "COSE_Key" : {
>          "kty" : "Symmetric",
>          "kid" : b64'39Gqlw',
>          "k" : b64'hJtXhkV8FJG+Onbc6mxCcQh'
>        }
>      }
>    }
> 
>        Figure 9: Example AS response with an access token bound to a
>                               symmetric key.
> 
> In light of the related discussion on cwt-proof-of-possession (resulting
> in the removal of a bunch of text), do we want to say anything about the
> scope of the 'kid' (that is, which participants are expecting to receive
> and understand that value)?

[LS] FIXED

> 44.)
> Section 5.6.3
> 
>    The error responses for CoAP-based interactions with the AS are
>    equivalent to the ones for HTTP-based interactions as defined in
>    Section 5.2 of [RFC6749], with the following differences:
> 
> "equivalent [...] with the following differences" is going to get
> complaints from the IESG.  I suggest "generally equivalent".

[LS] FIXED

> 45.)
>    o  A response code equivalent to the CoAP code 4.00 (Bad Request)
>       MUST be used for all error responses, except for invalid_client
>       where a response code equivalent to the CoAP code 4.01
>       (Unauthorized) MAY be used under the same conditions as specified
>       in Section 5.2 of [RFC6749].
> 
> 6749 has a case where 401 MUST be used, but I think it does not apply to
> us (since WWW-Authenticate would have to have been used).  I'm not sure
> whether it makes sense to note that or not, though.

[LS] The 401 in 6749 is the invalid_client case. For CoAP a 4.01 is also
allowed (MAY) instead of 4.00 if this is warranted. I do not believe further
action is warranted at this point. Do you agree?

> 46.)
> Section 5.6.4.3
> 
>    responses.  Furthermore profiles MUST define proof-of-possession
>    methods, if they support proof-of-possession tokens.
> 
> nit: define the *methods* or the *list* of allowed ones?

[LS] FIXED. The latter.

> 47.)
>    A profile MUST specify an identifier that MUST be used to uniquely
>    identify itself in the "profile" parameter.  The textual
>    representation of the profile identifier is just intended for human
>    readability and MUST NOT be used in parameters and claims.
> 
> This seems like an opportunity for a registry...just like we have in
> Section 8.7.  I'd suggest mentioning that registry here.

[LS] FIXED.

> 48.)
> Section 5.6.5
> 
> Perhaps the title of Figure 12 should include both requests and
> responses?

[LS] FIXED.

> 49.)
> Section 5.7.1
> 
>    map with a "token" entry containing either the access token or a
>    reference to the token (e.g., the cti).  Further optional parameters
>    [...]
>    The same parameters are required and optional as in Section 2.1 of
>    [RFC7662].
> 
> This seems somewhat problematic, in that 7662 is pretty clear that the
> "token" value is, well, the actual token.  So if we want to be able to
> use "cti" instead, then we need to have some text saying that we're
> deviating from 7662 and the specification for how "cti" is used there.

[LS] FIXED. Actually the phrasing here was simply bad, what I tried to 
express
was that there are token types that are simply a reference to claims held by
the AS (i.e. not self-contained). I have changed the text to be correct.

> 50.)
>    {
>      "token" : b64'7gj0dXJQ43U',
>      "token_type_hint" : "pop"
>    }
> 
>                          Figure 14: Decoded token.
> 
> nit: I think this is a decoded payload, not a decoded token.

[LS] FIXED

> 51.)
> Section 5.7.2
> 
>    cnonce  OPTIONAL.  A client-nonce previously provided to the AS by
>       the RS via the client.  See Section 5.6.4.4.
> 
> I think this statement is only true after the token in question has been
> properly validated at the RS.  Before that, all we can say is that it is
> the client-nonce provided to the AS by the client, since there is
> nothing to enforce that the client actually provided the same value that
> was generated by the RS.

[LS] FIXED. I've added some clarification text.

> 52.)
> Section 5.7.4
> 
> "scope" is showing up with the key 9 a lot; I have mixed feelings about
> having the CBOR map key value be aliased across different contexts (but
> it's probably too late to change without disruption anyway).

[LS] Scope is used in 4 places:

I. AS Request Creation Hints
II. /token request and response parameter
III. CWT claim
IV. /introspection response parameter

I claim that in all 4 cases we expect the same content with the same
formatting and semantics:  I. tells the client what to put into II.
which in turn informs the AS about the requested claim value in III.
and IV. is supposed to reflect that claim value. I therefore don't feel
hesitant to use the same abbreviation for all four cases.
Is that acceptable or do you whish us to perform any action on this?

> 53.)
> Section 5.8
> 
>    In order to facilitate offline processing of access tokens, this
>    document uses the "cnf" claim from
>    [I-D.ietf-ace-cwt-proof-of-possession] and specifies the "scope"
>    claim for JWT- and CWT-encoded tokens.
> 
> [looks like draft-ietf-oauth-token-exchange is going to beat us for the
> JWT version of "scope":
> https://www.iana.org/assignments/jwt/jwt.xhtml#claims ]

[LS] Agree, I will change the whole document to refer to this registration
instead. Life punishes those who come too late.

> 54.)
>    The "scope" claim explicitly encodes the scope of a given access
>    token.  This claim follows the same encoding rules as defined in
>    Section 3.3 of [RFC6749], but in addition implementers MAY use byte
>    strings as scope values, to achieve compact encoding of large scope
>    elements.  The meaning of a specific scope value is application
>    specific and expected to be known to the RS running that application.
> 
> I think I commented above about the use of 0x20 as the list separator;
> whatever we do there should be applied here as well (or the text
> otherwise consolidated to only need to say it once).

[LS] FIXED. This got removed, as we no longer specify the scope claim.
I added text though to specify that even the scope claim could be binary.

> 55.)
> Section 5.8.1
> 
>    The access token, containing authorization information and
>    information about the key used by the client, needs to be transported
>    to the RS so that the RS can authenticate and authorize the client
>    request.
> 
> nit(?): do we want to talk about the "confirmation method" rather than
> the "key", out of some sense of remaining generic?

[LS] I agree. FIXED.

> 56.)
>    This specification RECOMMENDS that an RS stores only one token per
>    proof-of-possession key, meaning that an additional token linked to
>    the same key will overwrite any existing token at the RS.
> 
> I might consider mentioning that this greatly simplifies implementation,
> as not only is there not a need for the extra storage, but when a
> request comes in using a given key, there's no need to resolve which
> token to use with that request (and whether there would be different
> access decisions made depending on which token was used), since the key
> can be used directly for dispatch (e.g., with DTLS raw public key).

[LS] FIXED. Comment added

> 57.)
>    If the payload sent to the authz-info endpoint does not parse to a
>    token, the RS MUST respond with a response code equivalent to the
>    CoAP code 4.00 (Bad Request).
> 
> Do we want this to specifically be about "parse to a token" with no need
> to be a valid token?  (What error do we return for a syntactically valid
> but semantically invalid token?)

[LS] This is intentional. Syntactically valid but semantically invalid 
tokens
are dealt with in section 5.8.1.1 including detailed instructions on the 
error
messages to be returned.

> 
> 58.)
>    Profiles MUST specify whether the authz-info endpoint is protected,
>    including whether error responses from this endpoint are protected.
>    Note that since the token contains information that allow the client
>    and the RS to establish a security context in the first place, mutual
>    authentication may not be possible at this point.
> 
> We'll need some careful reasoning about this for the security
> considerations, since the authz-info transaction can impact what profile
> the RS thinks is in use.  E.g., whether a network attacker could
> cause the client to think that a different (vulnerable) profile is in
> use than the one the RS expects to use.

[LS] Noted. Do you think the reasoning in section 6.5 needs to be extended?

> 59.)
>    The RS MAY make an introspection request to validate the token before
>    responding to the POST request to the authz-info endpoint.
>    [...]
>    A RS MAY use introspection on a token received through the authz-info
>    endpoint, e.g. if the token is an opaque reference.  Some transport
> 
> We should probably deduplicate the "MAY use introspection"s.

[LS] FIXED.

> 60.)
> Section 5.8.1.1
> 
> I think we whould make an explicit note at the end of the list of claims
> that we specify processing for, that additional processing is done (in a
> profile-specific manner?) for other claims in the token.

[LS] FIXED.

> 61.)
>    If the access token contains any other claims that the RS cannot
>    process the RS MUST discard the token.  If this was an interaction
> 
> Just to double-check: this is overriding the JWT/CWT default so that all
> token claims are comprehension-mandatory?  It might be worth making this
> deviation from the core [JC]WT more explicitly a deviation.

[LS] FIXED. That was not intentional. I removed that part.

> 62.)
> Section 5.8.2
> 
>    If an RS receives a request from a client, and the target resource
>    requires authorization, the RS MUST first verify that it has an
>    access token that authorizes this request, and that the client has
>    performed the proof-of-possession for that token.
> 
> As written, this sounds like it's okay to only do proof-of-possession
> when posting tokens to the authz-info endpoint and not bind the
> proof/key to the current request.  Is that intended?

[LS] FIXED. No, I've added some clarification

> 
> 63.)
>    The response code MUST be 4.01 (Unauthorized) in case the client has
>    not performed the proof-of-possession, or if RS has no valid access
>    token for the client.  If RS has an access token for the client but
>    not for the resource that was requested, RS MUST reject the request
> 
> nit: I suggest "but the token does not authorize access for".

[LS] FIXED.

> 64.)
> Section 5.8.3
> 
>    o  The RS verifies the validity of the token by performing an
>       introspection request as specified in Section 5.7.  This requires
>       the RS to have a reliable network connection to the AS and to be
>       able to handle two secure sessions in parallel (C to RS and AS to
>       RS).
> 
> nit: isn't the second one RS to AS, not AS to RS?

[LS] FIXED.

> 
> 65.)
>       specification defines the following approach: The claim "exi"
>       ("expires in") can be used, to provide the RS with the lifetime of
>       the token in seconds from the time the RS first receives the
>       token.  This approach is of course vulnerable to malicious clients
>       holding back tokens they do not want to expire.  Such an attack
> 
> It also has suboptimal behavior if the RS loses state (e.g., by
> rebooting), and possibly requires the RS to store indefinitely all
> tokens with an "exi" value.  I have mixed feelings about specifying it
> at all, though I concede it probably does have some value.  Regardless,
> I think a dedicated subsection in the security considerations is in
> order.

[LS] We wanted to provide some solution for expiring tokens for RSes 
that have
no connectivity and no synchronized clocks. Using the "exp" claim in 
such cases
would have pretty unpredictable results.
I have extended section 6.3 in the security considerations to go into 
the detail
of "exi", please have a look if this covers the necessary issues.

> 66.)
> Section 5.8.4
> 
>    The AS provides the client with key material that the RS uses.  This
>    can either be a common symmetric pop-key, or an asymmetric key used
>    by the RS to authenticate towards the client.  Since there is no
> 
> Can you walk me through how a symmetric PoP key would be used for mutual
> authentication (i.e., authentication of RS to client)?  It's easy/common
> to do the PoP the other way, making the client prove it knows the key
> associated with the token, but the other direction is not always
> possible, depending on the protocol.

[LS] The reasoning is that if both client and RS perform a 
proof-of-possession of
the symmetric PoP key to each other, they can be considered to be mutually
authenticated, under the provision that no other parties (except for the 
AS) have
access to said key.  My understanding is that e.g. DTLS-PSK provides 
this kind of
guarantee. Do you want us to clarify this reasoning in the document?

> 67.)
>    o  The client knows a default validity period for all tokens it is
>       using.  This information could be provisioned to the client when
>       it is registered at the AS, or published by the AS in a way that
>       the client can query.
> 
> Just to double-check: this validity period is time since issuance?  I'm
> not sure whether this is sufficiently obvious that it goes without
> saying (but it might be).

[LS] FIXED. Yes time since issuance is what is meant.
I have added some clarification.

> 68.)
>    o  The client performs an introspection of the token.  Although this
>       is not explicitly forbidden, how exactly a client does
>       introspection is not currently specified for OAuth.
> 
> I'm pretty sure this is overtaken by events (sorry for my part in
> that!).  E.g., draft-ietf-oauth-jwt-introspection-response discusses
> clients doing introspection, and even RFC 7662 itself discusses using a
> client secret to authenticate to the introspection endpoint.  I think
> there's another document between those two that's also relevant, but
> can't find it right now

[LS] I am not so sure. When reading the fine print in both RFC 7662 and
draft-ietf-oauth-jwt-introspection-response, I find that when they mention
the term "client", they refer to the protected resource / RS as being a 
client of
the AS introspection endpoint.  A client holding an access token and 
performing introspection is never explicitly mentioned in both 
documents, to my best
knowledge.

> 69.)
> Section 6
> 
> There could perhaps be some security considerations relating to
> discovery of RSes/resources thereon, but since we don't really talk
> about that much to begin with, it's probably okay to skip the security
> considerations discussion as well.

[LS] Noted. I opt to skip this for the time being. If it comes up in the 
IESG
review we can revisit this issue.

> 70.)
> Both the client and the RS need to have their own respective long-term
> credentials with the AS; we should talk about protecting those
> credentials, what is risked/lost if the credentials are compromised,
> how to recover from compromise, and the general key management
> lifecycle (per BCP 107).

[LS] FIXED. I added a subsection and also restructured the body of section
6 into several additional subsections (the current structure had some 
seemingly
arbitrary material in the body of section and other material in 
subsections).

> 71.)
> We should probably mention revocation in some fashion (even if only to
> say that it's unlikely in practice) -- right now we just have one
> passing mention as an optional responsibility of the AS, in Appendix B.

[LS] FIXED. I have added a paragraph in the section on token protection,
with a reference to RFC 7009.Most revocation scenarios are neither 
covered in OAuth and in ACE though.

> 
> 72.)
>    A large range of threats can be mitigated by protecting the contents
>    of the access token by using a digital signature or a keyed message
>    digest (MAC) or an Authenticated Encryption with Associated Data
> 
> I won't be surprised if someone asks for examples of that "large range
> of threats", so be prepared to answer...

[LS] Noted. I take it that no action is required here at this point.

> 73.)
>    (AEAD) algorithm.  Consequently, the token integrity protection MUST
>    be applied to prevent the token from being modified, particularly
>    since it contains a reference to the symmetric key or the asymmetric
>    key.  [...]
> 
> nit: "key used for proof of possession" (though my previous remark about
> limiting to keys may apply as well).

[LS] FIXED.

> 74.)
>    key.  If the access token contains the symmetric key, this symmetric
>    key MUST be encrypted by the authorization server so that only the
>    resource server can decrypt it.  Note that using an AEAD algorithm is
>    preferable over using a MAC unless the message needs to be publicly
>    readable.
> 
> nit(?) is "the message" here still "the token" like it was for the rest
> of the paragraph?

[LS] FIXED. Indeed in this context "the token" is what is meant.

> 75.)
>    If the token is intended for multiple recipients (i.e. an audience
>    that is a group), integrity protection of the token with a symmetric
>    key is not sufficient, since any of the recipients could modify the
>    token undetected by the other recipients.  Therefore a token with a
> 
> I can't decide whether we should clarify that this is "integrity
> protection of the token with a symmetric key shared by the AS and RS"
> (as opposed to a symmetric PoP key).

[LS] FIXED. I added a clarification.

> 76.)
>    It is important for the authorization server to include the identity
>    of the intended recipient (the audience), typically a single resource
>    server (or a list of resource servers), in the token.  Using a single
>    shared secret with multiple resource servers to simplify key
>    management is NOT RECOMMENDED since the benefit from using the proof-
>    of-possession concept is significantly reduced.
> 
> I think we should word this more clearly with respect to "single shared
> secret" -- is this the credential used by the RS to authenticate itself
> to the client?  I'm not even sure if this text is intended to be
> describing a workflow using symmetric or asymmetric keys.

[LS] I added some clarification. Can you double-check that this makes
  more sense?

> 77.)
>               Profiles MUST specify how confidentiality protection is
>    provided, and additional protection can be applied by encrypting the
>    token, for example encryption of CWTs is specified in Section 5.1 of
>    [RFC8392].
> 
> We should probably be a little more precise about what the additional
> protection is -- it's not exactly protection for the PoP key, since
> that's transmitted alongside the token in the authorization info (i.e.,
> outside the token contents), but it does provide protection when the
> token is subsequently used.
> also, nit: comma splice

[LS] FIXED.

> 78.)
> Section 6.1
> 
> I think we should have a little bit more discussion about what attacks
> are possible even when a client hard-codes a list of trustworthy ASes,
> e.g., when a device in one AS's purview is compromised and tries to get
> the client to use a different (possibly also compromised, or maybe just
> buggy) AS than the one that's supposed to be responsible for the device
> in question.  In short, yes, spoofing is only possible within that set
> of trusted ASes, but spoofing can still cause problems.

[LS] I have added some text in section 6.FIXME Please have a look if this
covers what you were aiming at.

> 79.)
> Are there any AS parameters other than URI that might be useful for an
> out-of-band-configured list of valid values?

[LS] One might want to include the public key or certificate of the AS.
Do you want us to expand this section to include such parameters?

> 80.)
> Section 6.2
> 
>       client MUST be able to determine whether an AS has the authority
>       to issue access tokens for a certain RS.  This can be done through
>       pre-configured lists, or through an online lookup mechanism that
>       in turn also must be secured.
> 
> nit: I suggest "for example" to preempt anyone complaining about this
> being needlessly prescriptive.

[LS] FIXED

> 81.)
> Section 6.4
> 
>    There may be use cases were different profiles of this framework are
>    combined.  For example, an MQTT-TLS profile is used between the
>    client and the RS in combination with a CoAP-DTLS profile for
>    interactions between the client and the AS.  Ideally, profiles should
>    be designed in a way that the security of system should not depend on
>    the specific security mechanisms used in individual protocol
>    interactions.
> 
> I think the IESG is going to pounce on this "ideally" -- what can we do
> to ensure this is the case and/or warn when it isn't?  Can we just
> say that profiles MUST NOT make assume that the same profile is used for
> the different types of interactions?

[LS] FIXED. I used this phrasing:
"The security of a profile MUST NOT depend on the assumption that the 
profile
is used for all the different types of interactions in this framework.

> 
> 82.)
> Section 6.6
> 
>    inadvertently communicating with the wrong RS.  The correct values
>    for "audience" can either be provisioned to the client as part of its
>    configuration, or provided by the RS as part of the "AS Request
>    Creation Hints" Section 5.1.2 or dynamically looked up by the client
> 
> I'm not sure that the AS Request Creation Hints are a big help here,
> since they are untrusted input and it seems like bogus hints could cause
> the client to obtain a token for a "wrong" RS.

[LS] FIXED

> 83.)
> Section 6.7
> 
>    First an attacker could perform a denial of service attack against
>    the introspection endpoint at the AS in order to prevent validation
>    of access tokens.  To mitigate this attack, an RS that is configured
>    to use introspection MUST NOT allow access based on a token for which
>    it couldn't reach the introspection endpoint.
> 
> I'm not sure that "mitigate" is quite right, as the attack (denial of
> service) succeeds!  Probably we want something like "To maintain the
> security of the system" instead.
> nit: comma after First (and Second, in the next paragraph).

[LS] FIXED

> 84.)
>    an introspection call.  RS can mitigate such attacks by implementing
>    a rate limit on how many introspection requests they perform in a
>    given time interval and rejecting incoming requests to authz-info for
>    a certain amount of time, when that rate limit has been reached.
> 
> I think the rate limit can be partially per client IP address (but a
> global limit will probably still be necessary, too).

[LS] FIXED

> 85.)
> Section 7
> 
> Introspection could potentially output a lot of information from a given
> token; some of the claims that might be found in a JWT or CWT can have
> privacy considerations.  Though for constrained usage, it is likely that
> the claims set will be pretty stripped down, so that might be more of an
> exceptional case.  (When signed-only CWT/JWT are used, similar
> considerations apply to the same claims, which is mostly covered already
> under the "if access tokens are only integrity protected" part.

[LS] FIXED. I added a section stressing the possible privacy concerns of 
token
claims in general, whether they are obtained by processing the payload of a
self-contained token or by introspection.

> 
> 86.)
>    information as possible in an unencrypted response.  Means of
>    encrypting communication between C and RS already exist, more
>    detailed information may be included with an error response to
>    provide C with sufficient information to react on that particular
>    error.
> 
> nit: I think the sentence is supposed to start with "When" or "If".

[LS] FIXED

> 
> 87.)
> Section 8.X
> 
> I might consider adding a sentence to the toplevel Section 8 that notes
> "this document creates several registries with a registration policy of
> Expert Review; guidelines to the experts are given in Section 8.16" so
> that we have a heads-up that these guidelines exist while we're reading
> through the rest of the sections.  Though, perhaps most people aren't
> reading it straight through like I am...

[LS] FIXED

> 
> 88.)
> Section 8.3
> 
>    Name  The OAuth Error Code name, refers to the name in Section 5.2.
>       of [RFC6749], e.g., "invalid_request".
> 
> We should refer to the OAuth registry as the authority on names, not the
> immutable RFC.  (Similarly for the other mappings registries; I won't
> repeat it each time, though for the later ones we're already doing the
> right thing.)

[LS] This is interesting. The referenced section (5.2 of RFC6749) is not
mirrored in any IANA registry. I have put the question to the OAuth WG.

> 89.)
> Section 8.5
> 
> We should check what capitalization we want for the token type name, as
> it's used all-lowercase elsewhere in the document but appears here as
> "PoP".

[LS] FIXED. I'll align with the use in RFC 7800 which is PoP.

> 90.)
> Section 8.9
> 
>    Note that the mappings of parameters corresponding to claim names
>    intentionally coincide with the CWT claim name mappings from
>    [RFC8392].
> 
> I tried to spot-check this but couldn't find any that match up; am I
> looking in the wrong place?

[LS] FIXED.  This used to be the case for "aud", "scope" and "cnf" but has
since then intentionally been discontinued when the "aud" /token 
parameter was
replaced by the "audience" parameter. "scope" and "cnf" are still aligned.
I have removed this comment altogether instead of some convoluted 
explanation.

> 91.)
> Section 8.11
> 
>    Reference  This contains a pointer to the public specification of the
>       grant type abbreviation, if one exists.
> 
> Do we have some copy/paste issue with the "grant type abbreviation"
> going on here (and elsewhere)?

[LS] FIXED. How embarrassing.

> 92.)
> Section 8.12
> 
> "scope" is already registered by draft-ietf-oauth-token-exchange (for
> the same meaning).
> "profile" is already registered by OIDC Core, for a "profile page URL".
> So we will probably have to change to (e.g.) "ace_profile" everywhere.

[LS] DUPLICATE. Scope was fixed as part of comment 53. and profile as 
part of comment 1.

> 93.)
> Section 8.13
> 
> The entry for "scope" should maybe also point to the oauth draft that
> defined it for JWT.
> 
> (We can't get "profile" here, either.)

[LS] DUPLICATE. Scope was fixed as part of comment 53. Profile as part of 1.

> 94.)
> Section 8.14
> 
> I think for the media type we still want to use the IESG as the contact.
> 

[LS] FIXED. I was just unsure here and used the CoAP RFC as a template.

> 95.)
> Section 8.16
> 
>    o  Specifications are required for the standards track range of point
>       assignment.  Specifications should exist for specification
>       required ranges, but early assignment before a specification is
>       available is considered to be permissible.  Specifications are
> 
> This seems to be attempting to re-explain things already in RFC 8126,
> but introduces some divergence along the way (which is bad).  I do note
> that RFC 7120 covers early allocations, but specifically excludes doing
> so from Expert Review registries.  I think technically that would
> exclude using the early allocation procedure even for the "standards
> action" or "specification required" ranges, though the experts would
> still have leeway to make allocations under essentially the same
> circumstances.

[LS] FIXED. I removed that bullet. (note: RFC 8152 has the same text,
  which is entirely coincidental ahem...)
> 
> 96.)
>       document cannot have points assigned outside of that range.  The
>       length of the encoded value should be weighed against how many
>       code points of that length are left, the size of device it will be
>       used on, and the number of code points left that encode to that
>       size.
> 
> seems to duplicate "number of code points of that length that are left".

[LS] FIXED. Note that this error appears in RFC 8152 as well
(for some reason ;-) ).

> 97.)
> Section 10.1
> 
> We may get some debate about whether IANA registries are properly
> Normative of Informative references, but we can wait for that to happen
> -- no need to do anything now.

[LS] Noted. Does the AD have a position on this?

> 98.)
> Section 10.2
> 
> If we're using RFC 4949 for terminology definitions, I think that makes
> it a normative reference.
> 
> If we REQUIRE CBOR when used with CoAP, that also feels like a normative
> reference.
> 
> I also think 7519 needs to be normative, since we mandate some of its
> processing rules.

[LS] RFC 4949 is informational, so it cannot be normative. I would argue
that we are just using it to clarify the meaning of our terminology.

I agree with the other two and have FIXED that.

> 99.)
> Appendix A
> 
>    Proof-of-Possession:
> 
>       This framework makes use of proof-of-possession tokens, using the
>       "cnf" claim [I-D.ietf-ace-cwt-proof-of-possession].  A
>       semantically and syntactically identical request and response
>       parameter is defined for the token endpoint, to allow requesting
>       and stating confirmation keys.  This aims at making token theft
> 
> nit: the wording here is a little weird in terms of specifying what is
> identical.  I'm not sure I have a great suggestion for improvement,
> though; what I'm coming up with is a lot more words.

[LS] How about:
A request parameter "cnf" and a Response parameter "cnf", both having a
value space semantically and syntactically identical to the "cnf"
claim, are defined for the token endpoint, to allow requesting and
stating confirmation keys.

> 100.)
> Appendix C
> 
> Skimming through the main text, a couple more items we may want to list
> here:
> 
> - optional procedures for client discovery of RS/resources/permissions
>   (Section 4)
> - optionally define new grant types (Section 5.2)
> - optional use of client certificate as client credentials (Section 5.3)

[LS] FIXED

> 101.)
>    o  Specify the security protocol the client and RS must use to
>       protect their communication (e.g., OSCORE or DTLS over CoAP).
> 
> IIRC both DTLS over CoAP and CoAP over DTLS are defined; do we need
> specifically one of them?

[LS] no. FIXED.

> 
> 102.)
> Appendix D
> 
>    o  The symmetric key shared between client or RS and AS (if any).
> 
> I suggest splitting this into separate bullet points for client/AS and
> RS/AS, to avoid chance of confusion.

[LS] FIXED.

> 103.)
> Appendix E.1
> 
>       the Access Information contains the public key of the RS.  For
>       communication security this example uses DTLS RawPublicKey between
>       the client and the RS.  The issued token will have a short
>       validity time, i.e., "exp" close to "iat", to protect the RS from
>       replay attacks.  The token includes the claim such as "scope" with
> 
> With DTLS RPK I don't think replay is a huge concern; the short lifetime
> seems more of a bound on how soon the authorization might be
> invalidated.

[LS] Agree. FIXED.

> 104.)
> In Figure 18 and the other example request payloads, let's use a
> higher-entropy client_secret than "qwerty" (or "ytrewq"), so as to not
> give anyone bad ideas.

[LS] This issue disappeared with an update removing the rather insecure
client authentication method using the client_secret parameter.

> 105.)
> I'm not 100% sure, but the << "access_token" : b64'SlAV32hkKG ...', >>
> snippet doesn't seem to decode to the start of the expanded token
> contents shown in Figure 19.

[LS] FIXED. It is now a COSE_Encrypt0 based CWT.

> 106.)
> Do we want to update the times in the tokens/etc. to be closer to "now"?

[LS] FIXED.

> 107.)
>    Messages C and F are shown in Figure 20 - Figure 21.
> 
>       C: The client then sends the PoP access token to the authz-info
>       endpoint at the RS.  This is a plain CoAP request, i.e., no
>       transport or application layer security is used between client and
>       RS since the token is integrity protected between the AS and RS.
>       The RS verifies that the PoP access token was created by a known
>       and trusted AS, is valid, and has been issued to the client.  The
> 
> How can the RS check that the token "has been issued to the client"?
> (How does it identify the client at this point to do so?)

[LS] FIXED. The last part of the sentence didn't belong here.

> 108.)
>       The client sends the CoAP request GET to /temperature on RS over
>       DTLS.  The RS verifies that the request is authorized, based on
>       previously established security context.
>       F: The RS responds with a resource representation over DTLS.
> 
> nit: we should probably be consistent about whether we mention the CoAP
> layer in the request+response.

[LS] FIXED: Updated all examples to a consistent use of CoAP.

> 109.)
> Appendix E.2
> 
>    Note: In this example the client does not know the exact door it will
>    be used to access since the token request is not send at the time of
>    access.  So the scope and audience parameters are set quite wide to
>    start with and new values different form the original once can be
>    returned from introspection later on.
>    
> Maybe add "and tailored to the RS performing introspection"?

[LS] FIXED: I added a slightly different phrasing to the same effect, 
since the
original one didn't sound very elegant to me anymore.

> 
> 110.)
>       A: The client sends the request using POST to the token endpoint
>       at AS.  The request contains the Audience parameter set to
>       "PACS1337" (PACS, Physical Access System), a value the that the
>       online door in question identifies itself with.  The AS generates
> 
> Didn't we just say that the client doesn't know the exact door it is
> going to be talking to?

[LS] FIXED: Good catch. This is of course meant to restrict the audience to
the underlying physical access control system, controlling multiple 
doors,some of which the client might get access to. I have updated the 
text accordingly.

> 111.)
>       an access token as an opaque string, which it can match to the
>       specific client, a targeted audience and a symmetric key.  The
> 
> So we're handing out a symmetric PoP key for a long-lived token that may
> be used at multiple resources?  Isn't that the example we gave earlier
> that lets RSes impersonate the client to each other?

[LS] Since the doors (RSs) are all part of the same system we assume 
that this is not a issue here.  I added some explanatory text to clarify 
these
assumptions. Do you deem this acceptable?

Side note: If we used asymmetric pop keys instead, we would either have 
to provision all the doors' keys to the client up-front, or define a whole
new protocol (which we initially tried with the "clientToken" concept, 
but later dropped) that provides the client with the RS's public key 
during the
token POST or access attempt.

> 112.)
> In Figure 22, should we show the communications security layer between
> client and AS?

[LS] FIXED

> 113.)
> The introspection response in Figure 25 only has a 'kid' element for the
> confirmation claim (that corresponds to a symmetric key); how is the RS
> supposed to know/learn the corresponding key so as to be able to verify
> the client's proof of possession?

[LS] FIXED

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Ludwig Seitz <[email-masked]>
Date: Tue, 15 Oct 2019 16:14:48 +0200

> covers what you were aiming at.

A little typo snuck in here. "6.FIXME" should be "6.4"

/Ludwig

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Jim Schaad <[email-masked]>
Date: Tue, 15 Oct 2019 18:20:51 -0700

I have trimmed down to the issues that I wanted to respond to.

Jim

-----Original Message-----
From: Ludwig Seitz <[email-masked]> 
Sent: Tuesday, October 15, 2019 7:08 AM
To: Benjamin Kaduk <[email-masked]>; [email-masked]
Cc: [email-masked]
Subject: Re: AD review of draft-ietf-ace-oauth-authz-24

Hello Ben,

Detailed comments below.

/Ludwig

[JLS] I think we may want to change this at some point in the future, but I don't know how much people are going to use refresh tokens so I don't think I would worry about it.

[JLS] I would suggest that this could fall under the punt to the new transport that does not have the same as these verbs in it to explain how they would map.  

[JLS]  My understanding is that the refresh token is completely opaque to anybody but the AS so how it is stored is not as critical.  The client then does a normal request with that as a parameter.  The refresh token allows for the AS to avoid doing some of the authorization steps and potentially re-use the same key for the new token.  As such, the newly requested token needs to headed to the same audience as the original POP token.  This is the reason for needing to keep audience with it on the client.  

[JLS] Given that it would be identified as being application/ace+json, I don't think there is a problem with having the default value.  The fact that this is ACE is identifiable from the media-type of the message sent over HTTPS.

[JLS]  I could have wished you said something sooner.  I kept trying to argue that there was no need to be consistent on the mapping, but that is how it is.  I will note that sections 8.6, 8.9, 8.11 as well as 8.13 are all separate registries for doing the mapping.  This means that while today there is consistency across all of the registries, there is no guarantee that this will be true going forward.

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 28 Oct 2019 14:06:07 -0700

I guess I'll be responding to the thread in backwards order, to try and
catch any topics that have received later updates...

> clarifications. These are:

I don't think we need to do anything right now.

> [JLS] I would suggest that this could fall under the punt to the new transport that does not have the same as these verbs in it to explain how they would map.  

I agree that (1) is more effort than it's likely to be worth.  If we can
finagle a single-sentence disclaimer like "transports that do not use these
verbs will need to specify the requisite behavior" that would be great; if
not, then we can consider whether to just ignore it or do something more
complicated.

> [JLS]  My understanding is that the refresh token is completely opaque to anybody but the AS so how it is stored is not as critical.  The client then does a normal request with that as a parameter.…

I agree with the behavior, yes.  It's just interesting/surprising to have
something associated with the refresh token that uses the term "audience"
but that does not apply to the actual usage of the refresh token.
We probably don't need any text changes here, on second look (I note that
the text is "same audience as *access* tokens issued in the initial token
request).

> [JLS] Given that it would be identified as being application/ace+json, I don't think there is a problem with having the default value.  The fact that this is ACE is identifiable from the media-type…

Okay, you convinced me.

> [JLS]  I could have wished you said something sooner.  I kept trying to argue that there was no need to be consistent on the mapping, but that is how it is.  I will note that sections 8.6, 8.9, 8.1…

Sorry for being slow, and agreed on all the rest.
It seems unlikely that any gain from changing things now will be worth the
pain of churn.

-Ben

---

From: Ludwig Seitz <[email-masked]>
Date: Wed, 30 Oct 2019 13:57:29 +0100

> not, then we can consider whether to just ignore it or do something more

I'll go for this:

" Note that this document specifies
   protocol exchanges in terms of RESTful commands such as GET and POST.
   Future profiles using protocols that do not support these verbs MUST
   specify how the corresponding protocol messages are transmitted instead.
"

In the Overview section where we mention alternate transport protocols.

/Ludwig

-- 
Ludwig Seitz, PhD
Security Lab, RISE
Phone +46(0)70-349 92 51