Subject: [Ace] Murray Kucherawy's Discuss on draft-ietf-ace-mqtt-tls-profile-15: (with DISCUSS and COMMENT)

From: Murray Kucherawy via Datatracker <[email-masked]>
Date: Wed, 09 Mar 2022 22:35:01 -0800

Murray Kucherawy has entered the following ballot position for
draft-ietf-ace-mqtt-tls-profile-15: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ace-mqtt-tls-profile/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

This should be quick to resolve.  In Section 3.2:

   The Broker MUST NOT forward messages to unauthorized subscribers.
   There is no way to inform the Clients with invalid tokens that an
   authorization error has occurred other than sending a DISCONNECT
   packet.  Therefore, the Broker SHOULD send a DISCONNECT packet with
   the reason code '0x87 (Not authorized)'.

This seems like a contradiction.  How is that SHOULD not a MUST?

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks to Jean Mahoney for her ARTART review.  I encourage the authors to
respond at least to what she identified as minor issues.

The final SHOULD in Section 2.4.2 seems weak to me.  I recommend using a
non-BCP 14 "should" instead.

In Section 7.2, the "Required Parameters" and "Optional Parameters" should be
"N/A", not "none".  See Section 5.6 of RFC 6838.

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 9 Mar 2022 22:37:15 -0800

> https://datatracker.ietf.org/doc/draft-ietf-ace-mqtt-tls-profile/

I suppose you could also be less informative and just drop the transport
connection with no DISCONNECT (or simply hold it open while passing no traffic).
I think that's what I had in mind when I reviewed this with the SHOULD.

-Ben

---

From: Cigdem Sengul <[email-masked]>
Date: Thu, 10 Mar 2022 08:54:34 +0000

Hello Murray,

Thank you for your review. Our thinking was as Ben explained.
In the draft, we used MUST/MUST NOT for the behaviour that affected
security, and SHOULD for desired behaviour.

Would the following revision make it more clear:
"The Broker MUST NOT forward messages to unauthorized subscribers and
SHOULD inform them of authorisation failure.
The only way to inform the client, in this case, would be sending a
DISCONNECT packet.
Therefore, the Broker SHOULD send a DISCONNECT packet with the reason code
'0x87 (Not authorized)'. "

Jean Mahoney's comments have been addressed in the following, and there is
one clarification left that we are working on.
https://github.com/ace-wg/mqtt-tls-profile/pull/102
https://github.com/ace-wg/mqtt-tls-profile/pull/102/commits/75ac0c0a86812f359471a63f6b481b0b80482b97

I thought I should respond to these comments fast and will fix your other
two comments asap.

Kind regards,
--Cigdem

On Thu, 10 Mar 2022 at 06:37, Benjamin Kaduk <[email-masked]> wrote:

> > Please refer to
> https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Thu, 10 Mar 2022 07:23:26 -0800

Hi Cigdem,

> security, and SHOULD for desired behaviour.

This doesn't match my understanding of how BCP 14 is supposed to be used.
MUST describes something obligatory for either interoperability, security,
or operational reasons, and SHOULD is something where the implementer has
limited discretion; when using SHOULD (or SHOULD NOT), it's desirable to
include text describing when one might legitimately do something other than
what's being stated.

In this particular case, what I'm reading is paraphrased as: "In this
situation, there's exactly one thing you can legitimately do within this
protocol, and you SHOULD do that."  This doesn't make sense to me.  What
Ben is saying is that there is a second option, which is to simply close
the connection.  If you want to leave this as a SHOULD, I suggest adding
text saying exactly that.

Would the following revision make it more clear:
> Therefore, the Broker SHOULD send a DISCONNECT packet with the reason
> code '0x87 (Not authorized)'. "
>

That's also less dissonant, yes.  This was just discussed with Ben on the
IESG call and I'll let him guide you on which change is more aligned with
what you're trying to do.

-MSK

---

From: Cigdem Sengul <[email-masked]>
Date: Thu, 10 Mar 2022 16:08:14 +0000

Dear Murray,

Got it - I realise I wasn't clear with SHOULD  -  indeed the implementer
has limited discretion on what can happen: send the DISCONNECT, close
connection or drop the connection. I will discuss this with Ben but am
happy to add the additional text about dropping the connection.

Kind regards,
--Cigdem

---

From: Cigdem Sengul <[email-masked]>
Date: Fri, 18 Mar 2022 12:27:14 +0000

Dear all,

Thank you for your reviews.
I have compiled all the remaining fixes based on the received
DISCUSS/COMMENT inputs in this pull request
<https://github.com/ace-wg/mqtt-tls-profile/pull/106>.
All comments and suggestions were acted on. I have put the fixes proposed
for two DISCUSS below as well.

Murray:

[CS]: The text is now revised to say:
The Broker MUST NOT forward messages to unauthorized subscribers.  To
   avoid silently dropping messages, the Broker MUST close the network
   connection and SHOULD inform the affected subscribers.  The only way
   to inform a client, in this case, would be sending a DISCONNECT
   packet.  Therefore, the Broker SHOULD send a DISCONNECT packet with
the reason code 0x87 (Not authorized) before closing the network
   connection to these clients.

>From Francesca:

Updating my ballot after reviewing draft-ietf-ace-aif-06. Just want to make
sure we don't miss anything, please feel free to correct me if I missed the
mark here.

FP: https://datatracker.ietf.org/doc/html/draft-ietf-ace-aif-06#section-4
states:

default values are the values "URI-local-
   part" for Toid and "REST-method-set" for Tperm, as per Section 3 of
   the present specification.

   A specification that wants to use Generic AIF with different Toid
   and/or Tperm is expected to request these as media type parameters
   (Section 5.2) and register a corresponding Content-Format
   (Section 5.3).

FP: I wonder if this document should define a new media type parameter for
Tperm (as REST-method-set is not appropriate for "pub"/"sub" value) and
register a corresponding Content-Format as indicated in the paragraph above.
CC'ing Carsten for his opinion.

[CS]: I have added a new AIF section as discussed registering Toid, and
Tperm with "pub" and "sub" values.
The specific commit here
<https://github.com/ace-wg/mqtt-tls-profile/pull/106/commits/7c17a3f017f42ad795fab277c7983b147eeb40fd>

Let me know if this pull request addresses your DISCUSS, and I will publish
a new ID.

Kind regards,
--Cigdem

On Thu, 10 Mar 2022 at 16:08, Cigdem Sengul <[email-masked]> wrote:

>> This doesn't match my understanding of how BCP 14 is supposed to be
>> used.  MUST describes something obligatory for either interoperability,
>> security, or operational reasons, and SHOULD is something where the
>> implementer has limited discretion; when using SHOULD (or SHOULD NOT), it's
>> desirable to include text describing when one might legitimately do
>> something other than what's being stated.
>>