Re: [Ace] WGLC draft-ietf-ace-mqtt-tls-profile

[Marco Tiloca <[e-mail]>]

Hello Cigdem,

Thanks, please see my replies inline.

Best,
/Marco

On 2020-09-20 16:02, Cigdem Sengul wrote:
> Hello Marco, 
> Thank you for the review. My responses are inline. 
> Kind regards, 
> --Cigdem
>
>
>
>
>     [General]
>
>     * Refer the AIF adopted draft rather than the individual submission.
>
>     * Some references are included twice side by side, e.g. RFC 4949
>     and RFC 7800. 
>
>      
>
>     [Section 1]
>
>     * Add an inline reference to RFC 8446 for TLS 1.3. I think it's
>     good adding also references to CoAP and CBOR(-bis).
>
>
>  
>   [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 
> <https://github.com/ace-wg/mqtt-tls-profile/issues/65>, will be fixed
> in the next submission]  
>
>  
>
>     [Section 2.2.1]
>
>     * In the paragraph on "TLS:Known(RPK/PSK)-MQTT:none", the last two
>     sentences can clarify that they apply to TLS 1.2. As to the
>     analogous alternative provision of the token in PSK mode for TLS
>     1.3, that can point to "identity" in the "identities" entry of
>     "Pre-Shared Key" ClientHello Extension.
>
>
> [CS: I think it is better to change the wording to match what is
> defined for TLS 1.3, as it is the recommended version.
> Created new issues at https://github.com/ace-wg/mqtt-tls-profile/issues/69
>  Will fix. ]
>  
>
>
>     [Section 2.2.4.1]
>
>     * Section 2.2.4 said that the two-byte integer length indicates
>     the amount of following bytes within Authentication Data. However
>     this section refers to the two-byte length as only the token
>     length, i.e. it does not seem to cover also the MAC/Signature
>     (whose length might be assumed from the used algorithm), even
>     though that's still part of Authentication Data. Could you please
>     confirm or clarify?
>
>
> [CS: This is because the Authentication Data explained under 2.2.4  is
> binary data.  The
>    binary data in MQTT is represented by a two-byte integer length,
> which indicates the number of data bytes, followed by that number of
>    bytes.
> So, we have the total length of the entire Authentication Data, 
> Token length + token  + (total length - token length) of MAC data. 
> I hope this is more clear. 
>
> Do you think I should repeat what binary data is at the subsections
> rather than explaining in 2.2.4?
> ]

==>MT
So the Authentication Data includes some <Length ; Content> binary data,
possibly followed by some extra data (here a MAC/Signature). Correct?

I think it's worth clarifying this when introducing the Authentication
Data, and say what is part of the binary data here in Section 2.2.4.1.
<==

>
>
>     * It's worth making it explicit that the PoP key is used to
>     compute the MAC or the client signature. 
>
>
>     * s/and, the server/and the server
>
>     * Remove the final closed parenthesis.
>
> [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 
> <https://github.com/ace-wg/mqtt-tls-profile/issues/65>, will be fixed
> in the next submission]  
>  
>
>     [Section 2.2.4.2]
>
>     * Shouldn't the Authentication Data in the AUTH message from the
>     server start with a 2-byte server nonce length?
>
> [CS: the client is calculating a MAC over its nonce and server nonce
> and sending it back, with the information of its nonce. 
> I assumed the RS would remember its nonce length]

==>MT
I was referring to the AUTH packet from the server, including its RS
challenge.

The message is still including Authentication Data, and in any other
case this seems to start with a 2-byte length field, as also described
in Section 2.2.4.

If that length field is actually optional, I can see it might be omitted
here, since the client is supposed to get always an 8-byte nonce from
the server.
<==

>  
>
>
>     * Like for the AUTH message from the client, see the comment above
>     for Section 2.2.4.1 about what the 2-byte length covers (i.e.,
>     here too I would have expected it to cover also the MAC/signature,
>     not just the nonce).
>
>
> [CS: Has my previous explanation clarified this?
>
> client_nonce length + nonce
> (the size of AUTH DATA - client_nonce_length)  of MAC  of
> (client_nonce+server nonce)

==>MT
Yes, I think similar clarifications would help here in Section 2.2.4.2.
<==

>
>
>     * Like for the comment above for Section 2.2.4.1, it's worth
>     making it explicit that the PoP key is used to compute the MAC or
>     the client signature.
>
>
>     [Section 2.2.5]
>
>     * s/RS MUST verify/the RS MUST verify
>
>     * Please, add references for HS256 and Ed25519.
>
>
>     [Section 3]
>
>     * s/to all topic3/to all 'topic3' 
>
>        
>
>     [Section 6.1]
>
>     * s/as a UTF-8/is a UTF-8
>
>
>   [CS: Added to https://github.com/ace-wg/mqtt-tls-profile/issues/65 
> <https://github.com/ace-wg/mqtt-tls-profile/issues/65>, will be fixed
> in the next submission]    
>  
>
>
>     ================================
>
>
>     On 2020-09-01 22:54, Daniel Migault wrote:
>>     Hi, 
>>
>>     This email starts a 2 weeks Working Group Last Call
>>     for draft-ietf-ace-mqtt-tls-profile. Please review the document
>>     available here [1] and provide your feed backs by September 15
>>     2020.  
>>
>>     Yours, 
>>     Jim and Daniel
>>
>>     [1] https://datatracker.ietf.org/doc/draft-ietf-ace-mqtt-tls-profile/
>>
>>
>>     -- 
>>     Daniel Migault
>>     Ericsson
>>
>>     _______________________________________________
>>     Ace mailing list
>>     [e-mail] <mailto:[e-mail]>
>>     https://www.ietf.org/mailman/listinfo/ace
>
>     -- 
>     Marco Tiloca
>     Ph.D., Senior Researcher
>
>     RISE Research Institutes of Sweden
>     Division ICT
>     Isafjordsgatan 22 / Kistagången 16
>     SE-164 40 Kista (Sweden)
>
>     Phone: [phone]
>     https://www.ri.se
>
>     _______________________________________________
>     Ace mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/ace
>

-- 
Marco Tiloca
Ph.D., Senior Researcher

RISE Research Institutes of Sweden
Division ICT
Isafjordsgatan 22 / Kistagången 16
SE-164 40 Kista (Sweden)

Phone: [phone]
https://www.ri.se