Re: [Ace] AD review of draft-ietf-ace-oauth-params-05

2019-11-18 Thread Benjamin Kaduk
One more update on this one: I cancelled the last-call request, because my
discussions with Ludwig about the core framework indicated a strong chance
that we'll want to tweak how rs_cnf works, and we should of course reflect
any change to rs_cnf in this document as well.

Further updates (hopefully) in the framework's thread.

-Ben

On Mon, Nov 18, 2019 at 09:42:41PM -0800, Benjamin Kaduk wrote:
> On Sun, Nov 17, 2019 at 04:45:04AM +0100, Ludwig Seitz wrote:
> > On 15/11/2019 13:14, Benjamin Kaduk wrote:
> > > Hi all,
> > > 
> > > I'm mostly just nitpicking in the following comments; the actual content
> > > here is in good shape.  (But some of these are popular things that
> > > directorate reviewers like to complain about, so fixing them preemptively
> > > seems wise.)
> > > 
> > > Section 1
> > > 
> > > access tokens.  These parameters and claims can also be used in other
> > > contexts, and may need to be updated to align them with ongoing OAuth
> > > work.  Therefore they have been split out into this document, which
> > > can be used and updated independently of [I-D.ietf-ace-oauth-authz].
> > > 
> > > I expect that we'll get some reviewers who want to wordsmith this last
> > > sentence.  It's fine to wait and see what suggestions come in (if any),
> > > but if you want to try to forestall those, my suggestion would be:
> > > 
> > > % Therefore, these parameters and claims have been put into a dedicated
> > > % document, to facilitate their use and any potential updates in a manner
> > > % independent of [I-D.ace-oauth-authz].
> > > 
> > Done.
> > 
> > > Section 3.1
> > > 
> > > req_cnf
> > >OPTIONAL.  This field contains information about the key the
> > >client would like to bind to the access token for proof-of-
> > >possession.  It is RECOMMENDED that an AS reject a request
> > >containing a symmetric key value in the 'req_cnf' field, since the
> > >AS is expected to be able to generate better symmetric keys than a
> > >potentially constrained client.  The AS MUST verify that the
> > > 
> > > nit: I'd wordsmith this a bit, since the idea is that the AS-generated
> > > key will be better than one generated by a constrained client, but a
> > > non-constrained client can probably do just fine at keygen.  So,
> > > I'd consider dropping "potentially", as the rest of the sentence does
> > > not have anything to imply that all clients using this claim are
> > > constrained.
> > > 
> > Done.
> > 
> > > Payload:
> > > {
> > >"req_cnf" : {
> > >   "COSE_Key" : {
> > >  "kty" : "EC",
> > >  "kid" : h'11',
> > >  "crv" : "P-256",
> > >  "x" : b64'usWxHK2PmfnHKwXPS54m0kTcGJ90UiglWiGahtagnv8',
> > >  "y" : b64'IBOL+C3BttVivg+lSreASjpkttcsz+1rb7btKLv8EX4'
> > >   }
> > >}
> > >  }
> > > 
> > > Figure 1: Example request for an access token bound to an asymmetric
> > > key.
> > > 
> > > Shouldn't an access token request have an authorization code?
> > > The other parameters from Section 4.1.3 of RFC 6749 have conditions
> > > attached to their REQUIRED status, but not "code".
> > >
> > In draft-ietf-ace-oauth-authz section 5.6.1. (Client-to-AS Request) we 
> > specify that when the "grant_type" parameter is missing, then 
> > "client_credentials" is implied, which in turn means that the client 
> > does not need an authorization code.  Section 4.1.3 of RFC 6749 is about 
> > the authorization code grant, you are looking at the ACE-ified version 
> > of Section 4.4
> 
> Ah, thanks.  I remembered that we had a default "grant_type" but was too
> fast when trying to check the other (sometimes-)required parameters.
> 
> > > Section 3.2
> > > 
> > > cnf
> > >REQUIRED if the token type is "pop" and a symmetric key is used.
> > > 
> > > Just to check, this means that if the client includes a symmetric 'cnf'
> > > key and the AS ignores the prior recommendation to ignore such a
> > > symmetric confirmation key in the request, the AS still has to echo
> > > back the symmetric key that is in use?  (There's also potentially some
> > > interesting interactions if we use a 'kid'.)
> > 
> > Indeed as the drafts are written right now, that would be the expected
> > AS behaviour. This is not entirely intentional, and my only objection to 
> > changing it at this time is that writing specification text handling 
> > interactions we are recommending against (i.e. accepting 
> > client-suggested symmetric keys) seems icky.
> > As for 'kid' I'd just expect the AS to echo back the kid, not the actual 
> > key.
> 
> I don't see any obvious problems from this behavior, so leaving it as-is
> seems like the right thing to do for now.
> 
> > > 
> > >MAY be present for asymmetric proof-of-possession keys.  This
> > >field contains the proof-of-possession key that the AS selected
> > >for the 

Re: [Ace] AD review of draft-ietf-ace-oauth-params-05

2019-11-18 Thread Benjamin Kaduk
On Sun, Nov 17, 2019 at 04:45:04AM +0100, Ludwig Seitz wrote:
> On 15/11/2019 13:14, Benjamin Kaduk wrote:
> > Hi all,
> > 
> > I'm mostly just nitpicking in the following comments; the actual content
> > here is in good shape.  (But some of these are popular things that
> > directorate reviewers like to complain about, so fixing them preemptively
> > seems wise.)
> > 
> > Section 1
> > 
> > access tokens.  These parameters and claims can also be used in other
> > contexts, and may need to be updated to align them with ongoing OAuth
> > work.  Therefore they have been split out into this document, which
> > can be used and updated independently of [I-D.ietf-ace-oauth-authz].
> > 
> > I expect that we'll get some reviewers who want to wordsmith this last
> > sentence.  It's fine to wait and see what suggestions come in (if any),
> > but if you want to try to forestall those, my suggestion would be:
> > 
> > % Therefore, these parameters and claims have been put into a dedicated
> > % document, to facilitate their use and any potential updates in a manner
> > % independent of [I-D.ace-oauth-authz].
> > 
> Done.
> 
> > Section 3.1
> > 
> > req_cnf
> >OPTIONAL.  This field contains information about the key the
> >client would like to bind to the access token for proof-of-
> >possession.  It is RECOMMENDED that an AS reject a request
> >containing a symmetric key value in the 'req_cnf' field, since the
> >AS is expected to be able to generate better symmetric keys than a
> >potentially constrained client.  The AS MUST verify that the
> > 
> > nit: I'd wordsmith this a bit, since the idea is that the AS-generated
> > key will be better than one generated by a constrained client, but a
> > non-constrained client can probably do just fine at keygen.  So,
> > I'd consider dropping "potentially", as the rest of the sentence does
> > not have anything to imply that all clients using this claim are
> > constrained.
> > 
> Done.
> 
> > Payload:
> > {
> >"req_cnf" : {
> >   "COSE_Key" : {
> >  "kty" : "EC",
> >  "kid" : h'11',
> >  "crv" : "P-256",
> >  "x" : b64'usWxHK2PmfnHKwXPS54m0kTcGJ90UiglWiGahtagnv8',
> >  "y" : b64'IBOL+C3BttVivg+lSreASjpkttcsz+1rb7btKLv8EX4'
> >   }
> >}
> >  }
> > 
> > Figure 1: Example request for an access token bound to an asymmetric
> > key.
> > 
> > Shouldn't an access token request have an authorization code?
> > The other parameters from Section 4.1.3 of RFC 6749 have conditions
> > attached to their REQUIRED status, but not "code".
> >
> In draft-ietf-ace-oauth-authz section 5.6.1. (Client-to-AS Request) we 
> specify that when the "grant_type" parameter is missing, then 
> "client_credentials" is implied, which in turn means that the client 
> does not need an authorization code.  Section 4.1.3 of RFC 6749 is about 
> the authorization code grant, you are looking at the ACE-ified version 
> of Section 4.4

Ah, thanks.  I remembered that we had a default "grant_type" but was too
fast when trying to check the other (sometimes-)required parameters.

> > Section 3.2
> > 
> > cnf
> >REQUIRED if the token type is "pop" and a symmetric key is used.
> > 
> > Just to check, this means that if the client includes a symmetric 'cnf'
> > key and the AS ignores the prior recommendation to ignore such a
> > symmetric confirmation key in the request, the AS still has to echo
> > back the symmetric key that is in use?  (There's also potentially some
> > interesting interactions if we use a 'kid'.)
> 
> Indeed as the drafts are written right now, that would be the expected
> AS behaviour. This is not entirely intentional, and my only objection to 
> changing it at this time is that writing specification text handling 
> interactions we are recommending against (i.e. accepting 
> client-suggested symmetric keys) seems icky.
> As for 'kid' I'd just expect the AS to echo back the kid, not the actual 
> key.

I don't see any obvious problems from this behavior, so leaving it as-is
seems like the right thing to do for now.

> > 
> >MAY be present for asymmetric proof-of-possession keys.  This
> >field contains the proof-of-possession key that the AS selected
> >for the token.  Values of this parameter follow the syntax of the
> > 
> > And to check here again, the omitted 'cnf' in the response means that he
> > AS accepted the client's requested asymmetric key?  (There wouldn't be a
> > case where the AS rejects an offered asymmetric key and replaces it with
> > a new one, right?)
> Indeed that is correct. Note that there could be cases where an AS 
> rejects an offered asymmetric key and replaces it by another one. This 
> would be if the AS knows that the RS doesn't support this key format,
> and also knows that the client holds another public key which is 
> 

Re: [Ace] Secdir last call review of draft-ietf-ace-coap-est-15

2019-11-18 Thread Panos Kampanakis (pkampana)
Hi Ben,

All addressed, except for the last one. The diff is here 
https://github.com/SanKumar2015/EST-coaps/commit/53933bb9f9365795f2302baef2e39709ae05
 

My responses below in Pk> 

I will wait for your confirmation before I upload version 17.

Panos


-Original Message-
From: Benjamin Kaduk  
Sent: Tuesday, November 12, 2019 6:06 PM
To: Panos Kampanakis (pkampana) 
Cc: Yaron Sheffer ; 
draft-ietf-ace-coap-est@ietf.org; ace@ietf.org
Subject: Re: [Ace] Secdir last call review of draft-ietf-ace-coap-est-15

Hi Panos,

On Wed, Oct 16, 2019 at 03:06:01PM +, Panos Kampanakis (pkampana) wrote:
> Hi Yaron,
> 
> Thank you for the thorough review and feedback. 
> 
> To make sure I don't miss any of your comments over an email thread, I track 
> all your feedback in git issue 152 
> https://github.com/SanKumar2015/EST-coaps/issues/152 There I tried to address 
> all your comments and question and clarify some points. 
> 
> I also pushed minor changes to fix a few of the issues you brought up. 
> The diff of the changes pushed is here 
> https://github.com/SanKumar2015/EST-coaps/commit/86d785f2122596f28674f
> e8e403d30467c98abfb
> 
> Please review the git issue and let us know if there are pending concerns. 
> Otherwise I am planning to reupload a new iteration next week. 

Thanks for uploading the -16, and thanks again to Yaron for the very thoughtful 
review.

Sadly, I seem to have not looked at the github diff closely enough/in a timely 
fashion, so I think we'll need to publish a -17 before I can move this into 
IESG evaluation.  Here's my remaining notes:

Section 4

   Private keys can be transported as responses to a server-side key
   generation request as described in Section 4.4 of [RFC7030] and
   discussed in Section 5.8 of this document.

I think that, since Yaron brought it up, we should say "Section 4.4 of 
[RFC7030] (and subsections)".

PK> Fixed. 

   curve.  After the standardization of [RFC7748], support for
   Curve25519 will likely be required in the future by (D)TLS Profiles
   for the Internet of Things [RFC7925].

Sorry I missed this before -- "standardization" is a bit of a bugbear (7748 is 
Informational, not even Standards-Track, let alone full Internet Standard).

Pk> Replaced "standardization" with "publication". 

   In a constrained CoAP environment, endpoints can't always afford to
   establish a DTLS connection for every EST transaction.
   Authenticating and negotiating DTLS keys requires resources on low-
   end endpoints and consumes valuable bandwidth.  To alleviate this
   situation, an EST-coaps DTLS connection MAY remain open for
   sequential EST transactions.  For example, an EST csrattrs request
   that is followed by a simpleenroll request can use the same
   authenticated DTLS connection.  However, when a cacerts request is

I think we can also clarify that this "MAY remain open" is changing 
expectations from RFC 7030, where the expectation was that the TLS connection 
did not remain open.

Pk> I added " which was not the case in [RFC7030]"

Section 10.1

   the first DTLS exchange.  Alternatively, in a case where a /sen
   request immediately follows a /crts, a client MAY choose to keep the
   connection authenticated by the Implicit TA open for efficiency
   reasons (Section 4).  A client that interleaves EST-coaps /crts
   request with other requests in the same DTLS connection SHOULD
   revalidate the server certificate chain against the updated Explicit
   TA from the /crts response before proceeding with the subsequent
   requests.  If the server certificate chain does not authenticate
   against the database, the client SHOULD close the connection without
   completing the rest of the requests.  The updated Explicit TA MUST
   continue to be used in new DTLS connections.

I think Yaron was saying that "even if the optimization of keeping the DTLS 
connection open is useful in the general case, it is very risky and prone to 
misimplementation when combined with /crts".  So, if I can rephrase the propsal 
to be that we say something more like "even though in general the DTLS 
connection can remain open for efficiency (Section 4), after a /crts response, 
the client MUST close the DTLS connection and establish a new DTLS connection 
for subsequent EST exchanges", are there reasons that we shouldn't use that 
behavior?

Pk> I am not sure it is worth to add more complexity specific to /crts. We 
would be adding more complexity for the client to track if this TLS connection 
is a cacerts connection then I need to close it, if it is any other connection 
then I can interleave requests. I am not convinced that the risk is that big 
either. A malicious user that was authenticated by the Implicit DB could easily 
craft a cacerts response that allows him to be authenticated by the Explict DB. 
In other words an active attacker would have no problems of fooling the 
Explicit DB if he was allowed in by the Implicit DB. I think the whole issue 
here is 

[Ace] ace-key-groupcomm-03

2019-11-18 Thread Peter van der Stok
Hi authors, 

Many thanks for this document 


Below my review of the latest version of this document, based on my
implementation experience. 


General point: I find it difficult to understand which secure
communication channel is discussed at some points in the document. My
suggestion is to name the two secure channels in section 1.1. 

For example: 


Management channel: channel secured with OSCORE or DTLS between client
and KDC, according to specified transport profile. 


Group channel: channel secured with OSCORE between group members,
according to specified transport profile. 

Mode detailed text suggestions follow below: 

Page 3 par 2: s/is in Appendix/is shown in Appendix/ 

Figure 1: 


According to text both Dispatcher and KDC can be RS, but only Dispatcher
has (RS) added in the figure 1. 

Page 4, bullet 2: s/join the group/join a given group/ 

Page 4, bullet 3: s/join the group/join a given group/ 


Page 4, bullet 3: remove last phrase "If required…. Changes.". It does
not add anything, does it? 


Page 4, bullet 4: what is an "implicit entity"? The phrase provokes more
questions than it clarifies. 


Page 5, bullet 1: remove: "to communicate with group members" (section
4) is sufficient explanation.) 


Page 5, bullet 2: Shorten to: "Leaving of a node or removal of a node by
the KDC (section 5)" 


Page 5, bullet 3: Do you mean? " retrieval of keying material by a group
member" 


Page 5, line 4 from below: s/between client and KDC/between client and
AS/ 

Page 5, end: suggestion is 


s/The exchange  …and KDC/The joining Request and Joining Response and
all further messages between client and KDC are exchanged over the
Management channel/ 

page 6, top: Remove : "All further … exchange." 


Page 6: s/All communications  … section 4/The group members communicate
via the group channel using keying material of section 4/ 

Section 3, par 2, s/join the group through the KDC/join a given group/ 

Section 3, par 3; reduce to: 


The first two messages exchanged over the management channel use
content-format application/ace+cbor and the third message uses content
format application/cwt. 

Section 3.1, par1: /is as defined/is defined/ 


Section 3, page 7: The appearance of (REQx) is very surprising; Would be
good to announce the function of "(REQx)" at the start of section 3 or
end of section 2. 


Page 6, bullet 4: Not clear what you mean here. Are they optional,
superfluous, or a MUST? 

Section 3.2, par 1: s/is as defined/is defined/ 


Page 8 The phrase "The access token MUST…. ACE requires" is difficult to
parse. 


In my understanding, the request contains a scope parameter, the token
contains a scope parameter and the response contains a scope parameter. 
Which scope must be a subset of the other? That is not clear. And how
can they be a subset; we're talking array. 


Last par of section 3.2: I understand that the AS always replies with a
new access token. 


Section 3.3: May be it is good to write that the POST exchange is not
secured. 

Page 9: N_S is the value of the {"rsnonce" :value} pair? 


Page 10: The phrase: "It is required of the application profiles….". Is
not clear. Does it mean that all application profiles must be changed to
include this parameter? And what if this has not happened? 


Page 11, section 4: It is good to specify that all exchange between
joining node and KDC passes over the management channel. 

Page 12, line 2: Add:  securely communicate "over the group channel"….. 

Page 12: s/assiciated/associated/ 


Sec 4.1: please, make clear from the very start that the path names of
the resources are example names. 


Page 13, section 4.1.2.1: Scope does not specify the resource path, but
the value of gid 

Page 14: N_C is the value of the ("cnonce":value} pair? 


Page 14: I don't understand the paragraph: "The handler verifies … error
message." How can the gid be a subset of the "scope"? This occurs at
several other occasions in the text. 

Page 15: should num be monotonically increasing with each new version? 

Page 16, bullet 1: what are the member identifiers? 

Page 20, section 4.1.6: It is not clear to me how node is defined. 


Page 21, section 4.2: Instead of pairwise secure communication channel
use the term Management channel. 


Page 22, paragraph 3: group identifier cannot be equal to the value of
the scope; At most it can be equal to the first array element of the
scope value. 


Page 22, section 4.3, paragraph 2: "that the client was not able to
receive". The process in which clients receive new keying material
without asking for it is not clear to me. Suggest to remove the phrase. 

Page 23, Text after figure 7: 


The rekeying process is not clear to me. Is it possible to discuss at
least one approach, where the ContextId changes with the Master secret
and the salt, every time the group changes composition or the keying
material is refreshed. Receiving an unknown contextId means that
rekeying took place and new material needs to be fetched. However,