Re: [Ace] Discuss on draft-ietf-ace-mqtt-tls-profile-15: (with DISCUSS and COMMENT)

[Cigdem Sengul <[e-mail]>]

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:

----------------------------------------------------------------------
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?

[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:
----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

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 <[e-mail]> wrote:

> 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
>
> On Thu, 10 Mar 2022 at 15:23, Murray S. Kucherawy <[e-mail]>
> wrote:
>
>> Hi Cigdem,
>>
>> On Thu, Mar 10, 2022 at 12:54 AM Cigdem Sengul <[e-mail]>
>> wrote:
>>
>>> 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.
>>>
>>
>> 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:
>>> "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)'. "
>>>
>>
>> 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
>>
>