Re: [Ace] Review for draft-ietf-ace-mqtt-tls-profile-01

Hello Jim,
Thanks for this review. I have responded inline.

>
>
> 1.  Are there any specifics about using ACE over HTTP that need to be
> explicitly stated some place.  Some of these things might include:
>         a) Must be HTTPS even if encrypted requests/responses are
> provided.
>         b) What types of validation are permitted/required: anon-anon,
> server validation, server & client validation. The latter corresponds to
> current DTLS statements.
>         c) Are we looking for using the normal HTTP/HTTPS ports or should
> we
> be using different ports?
>
>
>> Agreed. Need to specify that it is HTTPS.
Mutual authentication and server validation Required.
Was thinking it would be normal HTTPS ports. Should we consider different?
What scenarios should permit anon-anon; anon clients?

> 2.  Section 2.1.2: Somehow I missed on the first reading that you were
> suggesting the use of a topic /authz-info inside of the MQTT server for
> posting.   I think that this needs to have a more detailed set of
> instructions.  I assume but don't know that initially this would mean an
> anonymous connect, publish, disconnect, validated connect.  This however is
> not clear from the text.  It is also not clear that this should be over a
> TLS wrapped session for the publish.
>

>> OK. this was described in the Appendix. I will bring this forward in the
document, as an optional flow.

3.  Section 2.1.2: The correct reference is I-D.ietf-ace-dtls-authorize and
> not I-D.gerdes-ace-dtls-authorize.
>

>>  Will fix.

>
> 4.  Section 2.1.2: It is not clear from the document is a Broker is going
> to
> also have the possibility of doing a post of the token via HTTPS.
> Currently
> I would think not, but given that this is documented in OAuth 2.0 as an
> option I am not clear.
>
> >> Do you mean for introspection? Yes, this is a MAY as in the core
document, and so in this one too.
And if there is an RS-AS interaction, we expect that to be on HTTPS.
I will try to see where I should clarify in the text.

5.  It might make sense to break up section 2.1.2 more and cover the
> different methods either in continuous paragraphs or in subsections.
>

>> Sure. That would increase readability. It became rather long with the
new additions.

>
> 6.  As noted before, you need to have a section on TLS requirements.  This
> should cover what levels/types of authentication are permitted.  Some type
> of statement on TLS 1.3 vs prior versions.  Part of what needs to be placed
> here is the fact that while there is client authentication in MQTT, there
> is
> no method of doing server authentication other than the surrounding TLS
> protocol.  Also should cover how the certificate(?) or RPK would be
> validated.  Use the existing rs_cnf?
>

>> OK. Will add that.

>
> 7.   In Figure 2, I think the use of Client Identifier is somewhat
> confusing.  I think this is supposed to be the token as the Client
> Identifier is a different thing in MQTT.  Either that or you need to do a
> better job of describing the property block.
>

OK. I think I should explain MQTT packets better.
A connect packet has three parts: 1) Fixed header 2) Variable Header and 3)
Payload.
The variable header may have several properties, and Authentication Method
and Authentication Data are two of these properties.
According to MQTT Specification:
The ClientID MUST be present and is the first field in the CONNECT packet
Payload.
The ClientID MUST be a UTF-8 Encoded String
A Server MAY allow a Client to supply a ClientID that has a length of zero
bytes, however, if it does so the Server MUST treat this as a special case.

And this is what is done in the case of a collision:
If the ClientID represents a Client already connected to the Server, the
Server sends a DISCONNECT packet to the existing Client with Reason Code of
0x8E (Session taken over)

Is this more clear? If yes, I will try to clarify in text.

> 8.  If the value of Clean Start or Expiry Interval are not set correct is
> the server to fail the connect?
>

>> Agreed. This should be specified.
Two options: 1) Do not accept connection or 2) accept the connection but
then never create sessions, and then server  MUST set Session Present to 0
in the CONNACK packet, indicating
no session is present.

This basically makes Clean Start flag meaningless.
If this is better than client MUST be setting those parameters correctly,
then we can go this way.
Any thoughts?

> 9.  Should do a pass through the document and make sure that terms like
> "AUTH (Authentication Exchange) method" are not used.  I think you meant to
> use 'packet' rather than method in this location.  You need to be clear on
> the difference between methods and packets in the document.
>

>>I think I should clarify this piece as well as it is MQTT terminology. I
will add these to the terminology in the beginning.

AUTH (Authentication Exchange): AUTH packet sent from Client to Server or
Server to Client as part of an extended authentication exchange.
Authentication method: String containing the name of the authentication
method
Authentication data: Binary Data containing authentication data.

This is how these terms are defined in MQTT. Will check where the
terminology is mixed up and will try to assure consistency. Could you flag
particular instances where this was particularly confusing?

> 10. Section 2.1.2.1 - I am having a problem computing the MAC over the
> payload of the CONNECT message.  The password where the value is placed is
> included in the payload and I am not clear if it should be computed up to
> that value or if a fixed value should be used for the computation.
>
>
>>OK. I think it got muddled because I was trying to find a solution for a
scenario where the ClientId is used across sessions, and hence, not random.

Two cases.
1) Client ID uniquely random across each Client connection attempt to
Broker: MAC over Payload - Payload begins with ClientID and MAY include
Will Properties if Client indicated that it will have a will.
2) Client ID NOT uniquely random: MAC over Authentication Data.

It may be best to choose one method and just say MUST always have a nonce
in Authentication Data, and MAC is computed over that?

> 11. I am worried about the text on the Client Identifier in this section.
> It is not clear to me that having the client generate a random value for
> this field is good just in terms of the possibility of collisions occurring
> on the server.  The text in the MQTT 5.0 document appears to say that this
> is more interesting for the situation of getting back state that was left
> on
> the server after a disconnect and not so much for a clean session.
>

>> Yes, but as I explained for comment 7, in MQTT, the typical case is the
client provides its id.
However, to cater to all requirements and all implementation possibilities,
maybe it is best to just use a nonce as explained for comment 10 and not
tie anything to client id.

Previously, when we were writing the draft for MQTT v3.1.1 we wanted to
provide a solution that uses the available fields in the CONNECT packet as
much as possible; without
introducing any new fields. This was because of the constraints of the MQTT
v3.1.1. MQTT v5 gives us more options to implement this better.

For backward compatibility, we would need to think if we create a special
data structure to be used in the password so that it holds both the
MAC/signature and the nonce.

As the use of ClientId seems to create confusion, I am leaning towards this
solution. Any thoughts?

>
> 12.  I don't know where the nonce in this section is coming from.
> Discussion on where it comes from and where it is passed in the protocol is
> needed.
>

 >> As explained in 10. Will clarify in text.

> 13. Is there someplace that authentication methods need to be registered?
> I
> was unable to see any such registry setup with OASIS from the v5
> specification.  Should we suggest to the MQTT group that this is something
> that should be done?
>
>
>> This is an interesting thought. The draft was indeed discussed in the
OASIS MQTT WG.

But, the WG may consider this out of scope, as they have on their charter
page:
 Transport level security is assumed and no mandatory security features
will be added.
The MQTT v5 draft says:
The Authentication Method is commonly a SASL mechanism, and using such a
registered name aids interchange. However, the Authentication Method is not
constrained to using registered SASL mechanisms.

14.  It would be useful to re-iterate that the authentication method for
> 2.1.2.3 is still set to 'ace'.
>

>> Will do.

>
> 15. Section 2.1.3 - Please include a sentence that the signature algorithm
> of "none" is explicitly not permitted for tokens.
>
>
>> Will do.

> 16. You might want to point to the updated text in the ACE Framework
> document for token validation procedures as they were rewritten and contain
> more or less the same content?
>

>> Will do and remove the repetitious text.

> 17.  It is not clear to me, how in the scope string is the ability to have
> a
> "will" indicated?
>
> >> If the Will Flag is set to 1, the Will Topic follows the Client ID and
Will Properties in the Payload.
This is where the broker learns the Will Topic.
Then the scope would be publish_<will topic>.
And subscribes would need to subscribe_<will topic> to receive this
message.
Will clarify in text.

18.  Section 2.1.4 - I am unsure what the implications of discarding tokens
> on the end of a connection are going to be.  How do you publish the token
> before doing the connection or is that only for an updated token?
>
>
>> Tokens are discarded at the end of disconnection because no previous
session state is kept.
And a token is expected in the new CONNECT request (otherwise, it is AS
discovery)

Before the connection, a token may be transported via  "authz-info" topic
described in Appendix B.
During the connection, a new token may be transported via the AUTH packet -
which enables reauthentication.

> 19. Section 2.4  In the second paragraph, it appears you are saying that a
> client could send an AUTH packet after receiving a DISCONNECT message.  I
> don't believe that is true.
>

>>That's a mistake indeed. I should remove DISCONNECT from that list.
DISCONNECT is the last message of the session indeed.

>
> 20.  I am not sure what the benefits of doing the check on a PINGREQUEST is
> going to be.  Like the relay of a PUBLISH message this is just going to
> produce a DISCONNECT and a closed channel.  If this is really allowed, then
> you should potentially also say that the Broker could do a check based on
> clock time elapsing.  (I.e. do the check every hour.)
>

>> This was suggested by one of the reviewers that provide MQTT services
(his e-mail regarding this was sent to the ace list).
It was suggested as an optimization for detecting token expirations and
disconnecting early.
(i.e., if you are already receiving PINGREQUESTs, use these as a trigger to
check the token).
I think time-based checks may also help and be added as an option.

>
> 21.  Section 2.5 - The last sentence in the first paragraph could be state
> more clearly.  Specifically there would appear to be some ambiguity on who
> the associated tokens are attached to.  "messages as long as the publisher
> token for that message remains valid." Might be better.
>

>> Will change.

>
> 22. Not sure if this goes here or in the connect documentation.  There
> needs
> to be a check that the will topic is a topic that the client is going to be
> permitted to publish on.   Corner case, if the ability to publish a will in
> the event that this permission is lost.  Does this kill the session or the
> will message?  I.e. should the permission for the will message be rechecked
> before sending it allowing for the time based expiration of the token.
>

>> We have been thinking about this.
If the disconnect is due to a token expiration, and hence, also includes
the will topic as well, you have to decide between:
1) publish the will
2) do not publish the will.
At the moment, it's written like we publish the will - but on second
consideration,
it seems safer to say "Publish the Will on Server Disconnect or unusual
disconnect if the token (the will permission) has not expired".

Thanks again for these comments, will create the necessary github issues
and start revising the text.

Best,

>
> Jim
>
>
>
>
>
>
>
>
>
> _______________________________________________
> Ace mailing list
> Ace@ietf.org
> https://www.ietf.org/mailman/listinfo/ace
>