Re: [core] [Ace] FW: WGLC comments on draft-ietf-ace-dtls-authorize

[Jim Schaad <[e-mail]>]

I am responding to the review below in regards to the most recent version -06.


> -----Original Message-----
> From: Göran Selander <[e-mail]>
> Sent: Thursday, December 20, 2018 7:14 AM
> To: [e-mail]; Jim Schaad
> <[e-mail]>; Roman Danyliw <[e-mail]>
> Subject: Re: [Ace] FW: WGLC comments on draft-ietf-ace-dtls-authorize
> 
> Hello co-chairs and co-authors,
> 
> Here is a status update on the ACE DTLS profile.
> 
> I committed us to make an update in December. It seems my co-authors
> have had a hard time to even discuss the continued work. Some of us have
> been busy with other ACE drafts.
> 
> One contribution from my side: I have gone through the latest review
> answer from Olaf to Jim and made some updates to the draft, and some
> annotations in the mail below. This includes three out of four issues
> discussed at the ACE WG meeting in Bangkok. The first issue - which I didn't
> address - the use of reference tokens, could have an impact on different
> parts of the draft and wanted to discuss with my co-authors first.
> 
> The current version on the ACE Github is thus not complete but still possible
> to review and we have hopefully cut down the number of remaining issues. I
> will not be able to work more on the draft before the week starting Jan 7.
> Perhaps my co-authors can also share when they are available pick up the
> work.
> 
> Happy holidays to all!
> 
> Best regards
> Göran
> 
> 
> 
> On 2018-11-05, 16:59, "Ace on behalf of Olaf Bergmann" <ace-
> [e-mail] on behalf of [e-mail]> wrote:
> 
>     Jim,
> 
>     Thank you for this very useful review. Please find our comments inline:
> 
>     Jim Schaad <[e-mail]> writes:
> 
>     > Section 3.3 - Figure 4 - Where is the 'alg' parameter defined at that level?
> 
>     See next comment.
> 
> [GS]  alg parameter included
> 
>     > Section 3.3 - I am always bothered by the fact that PSK should really be
> PSS
>     > at this point.  The secret value is no longer a key and thus does not
>     > necessarily have a length.  There is also a problem of trying to decide
> what
>     > the length of this value would be based on the algorithm.  If the client
>     > offers TLS_PSK_WITH_AES_128_CCM_8 and
> TLS_PSK_WITH_AES_256_CCM_8  (I may
>     > have gotten these wrong but the intent should be understandable) then
> what
>     > length is the PSK supposed to be?
> 
>     I think what you are saying is that for the shared secret (k) in the
>     COSE_Key structure in Fig. 4, the AS needs to tell C what to do with
>     that shared secret? This was the intention of the alg parameter (which
>     has a not-so-useful value in this example).

Some of what is done here makes sense and some of it makes no sense at all.

Happy with the removal of the "alg" parameter in the root map.  

Happy with the addition of the kid parameter in the COSE_Key object since this is required for doing DTLS w/o sending the token as the identifier.

I have no idea what the algorithm is doing here?  This is not currently a COSE algorithm, it is a TLS algorithm and thus would not make a great deal of sense.  The terms of what the PSK length should be would be better covered by a statement along the lines of "When offering and/or accepting a TLS cryptographic suite, the length of the PSK should be at least as long as the symmetric encryption algorithms that are offered."  This may already be pointed to in the TLS documents and thus can be referenced to rather than stated explicitly.

When looking at the KDF option that you are providing.  I don't understand why you don't just make the KDF function and the length of the secret to be derived as pre-configured properties.  There is absolutely nothing that prevents this from being a 141 byte value.  

I am not seeing anything in the security considerations about protecting this secret and what all will be available to an attacker in the event that this secret is leaked.  Specifically, it allows for any previously recorded session to be decrypted if the KID is leaked, such as using the KID as the key identifier in the handshake protocol rather than sending across the CWT (assuming that it is encrypted).  

It will allow for an impersonation attack to occur in the event that the KID for a client in the event that the KID is leaked as the attacker would be able to create the same token that is passed to the client.

> 
>     > Section 3.3 - Figure 5 - Is this defining a new set of entries for cnf or is
>     > there a missing layer someplace?
> 
>     This is indeed a new set of entries. An internal discussion between the
>     draft authors led to the opinion that we cannot use a COSE_Key structure
>     in the cnf structure as this would have different semantics as intended
>     here. We came to the conclusion that for key derivation, listing the
>     required fields directly in the cnf structure is the cleanest solution.
> 
> [GS] new layer added, called ACE_Salt.

This does not seem to be what was implemented in the current draft.

>     > Section 4 - The requirement that the "kid" be in a previously issued token
>     > would have problems with this if you were to use the "kid" of an RPK
> which
>     > can be constant rather than being changed on each newly created token.
> 
>     To solve this we could require that the COSE_Key structure for RS's RPK
>     in the AS-to-Client response must also have a kid. Section 4 does not
>     prevent the kid to be constant.
> 

>     > Section 5 - this seems to be a very strange place to define the new
>     > parameter.
> 
>     True. How about moving this parameter to Section 4?
> 
> [GS] Change already done.

This seems to have just vanished - is that want is desired?


Jim


> 
>     > Section 8 - you need to put your kid in the IANA considerations as well
> 
>     Fixed.
> 
 
>     Grüße
>     Olaf
> 
> 
>