Re: [secdir] Secdir review of draft-murchison-nntp-compress-05

[Julien ÉLIE <[e-mail]>]

Hi Barry,

I've just taken into account your review in a new revision of the draft. 
  Thanks again for it!

FYI, the related changes are:
 
https://github.com/ksmurchison/drafts/commit/ea2ad20efd248fd83ec8e859cf2ab0064b3c3b64

Do not hesitate to tell me in case something is still wrong.



>> Maybe you would prefer for our document:
>>
>>     Consequently, a server MAY list the AUTHINFO capability with
>>     no arguments, or not advertise it at all, in response to a CAPABILITIES
>>     command received from an unauthenticated client after a compression
>>     layer is active
>>
>> This way, we would no longer use SHOULD, but MUST and one MAY.
>
> It's a small point, but I don't like "MAY" to introduce two
> alternatives, because it's not clear whether there might be other
> alternatives as well... or not.  How does it work for you to make it
> "MUST" instead of "MAY", saying, therefore, that the server MUST
> either list AUTHINFO with no arguments or not advertise it at all?
> Then it's clear that there are two alternatives, and either is OK.
> No?

OK.  I've adopted your suggestion.

    In order to help mitigate leaking authentication credentials via for
    instance a CRIME attack, authentication MUST NOT be attempted when a
    compression layer is active.  Consequently, a server MUST either list
    the AUTHINFO capability with no arguments or not advertise it at
    all, in response to a CAPABILITIES command received from an
    unauthenticated client after a compression layer is active [...]


Hmm...  I think I should now do a pass on the document and explicitly 
say when "compression layer" only means the one negotiated with COMPRESS.
As a matter of fact, I do not think it's a good idea to say in this 
draft that authentication MUST NOT be attempted when TLS-level 
compression is active!  It would otherwise be a change in how 
authentication works (RFC 4643 heavily mentions the preferred use of 
AUTHINFO along with TLS, and RFC 4642 allows TLS-level compression).
This document would otherwise be an update to RFC 4643, by no longer 
allowing AUTHINFO when TLS-level compression is active.



>>    When compression is active and either the client or the server
>>    receives invalid or corrupted compressed data, the receiving end
>>    SHOULD immediately close the connection.  (Then the sending end will
>>    in turn do the same.)
>
> Well (and we're now down to nit level, so do as you think best here),
> as I see it, it's not a question of level of constraint -- if it were,
> you could say it without 2119 key words, as "the receiving end
> immediately closes the connection, in response to which the sending
> end will do the same."  (Now that I think about it more, I think
> that's the best answer, but if you disagree that's fine.)

You're right.  Your suggestion is also adopted.

    When compression is active and either the client or the server
    receives invalid or corrupted compressed data, the receiving end
    immediately closes the connection, in response to which the sending
    end will do the same.



>>    Additionally, implementations MAY ensure that the contents of two
>>    distinct confidential articles are not compressed together.
>
> As I say, maybe just drop the key word and say it without capitals.
> If it's not easy to explain or determine, maybe just say that it's
> good to do it if you can:
>
> NEW
>    Additionally, it is preferable not to compress the contents of two
>    distinct confidential articles together if it can be avoided.
> END
>
> But, again, it's a small point that probably doesn't need further discussion.

Thanks for your suggestion of wording.  Also adopted!

    Additionally, it is preferable not to compress the contents of two
    distinct confidential articles together if it can be avoided, as
    adversaries might be able to derive information about them (for
    instance if they have a few header fields or body lines in common).
    This can be achieved for instance with DEFLATE by clearing the
    compression dictionary each time a confidential article is sent.
    More complex implementations are of course possible, and encouraged.



>> In the registry, it is indeed best to use upper case.  I can mention it.

Now done, for IANA's benefit:

    Any name that conforms to the syntax of an NNTP compression algorithm
    name (Section 5.3) can be used.  Especially, NNTP compression
    algorithms are named by strings, from 1 to 20 characters in length,
    consisting of upper-case letters, digits, hyphens, and/or
    underscores.



>> Just to be certain of the meaning of your remark:  the algorithm name
>> in the NNTP command is not required to be upper case.
>>
>>      algorithm = "DEFLATE" / 1*20alg-char
>>      alg-char = UPPER / DIGIT / "-" / "_"
>>
>> ABNF syntax is not case-sensitive, so "COmprESs dEFlaTe" is a valid
>> NNTP command, that has the same effect as "COMPRESS DEFLATE".
>
> Actually, that's an ABNF error that I missed: "DEFLATE" doesn't have
> to be in upper case, but any *other* algorithm does (because
> "alg-char" can only contain UPPER, not ALPHA or LOWER).  To fix that,
> I suggest using this:
>
>    algorithm = %s"DEFLATE" / 1*20alg-char
>
> ...and adding a normative reference to RFC 7405... that forces DEFLATE
> to be upper case.

Good catch.  Also done.

    This section describes the formal syntax of the COMPRESS extension
    using ABNF [RFC7405] [RFC5234].

    [...]

      algorithm = %s"DEFLATE" / 1*20alg-char  ; case-sensitive
      alg-char = UPPER / DIGIT / "-" / "_"



> Unless, of course, you really want to allow case-insensitivity here.

Case-sensitivity is better, and we'll keep that.
It is also coherent with SASL mechanisms (case-sensitive), and 
consistent with RFC 3977:

Section 3.1

    Commands in NNTP MUST consist of a keyword, which MAY be followed
    by one or more arguments.

[...]

    Commands may have variants; if so, they use a second keyword
    immediately after the first to indicate which variant is required.
    The only such commands in this specification are LIST and MODE.

[...]

    Keywords are case insensitive; the case of keywords for commands MUST
    be ignored by the server.  Command and response arguments are case or
    language specific only when stated, either in this document or in
    other relevant specifications.

In the case of COMPRESS:
- the command (COMPRESS) is case insensitive (like any keyword),
- the command has an argument (e.g., DEFLATE), that is not a keyword, 
and therefore can be said to be case-sensitive.



> Considering what you say in your response after that, let me suggest
> an alternative:
>
> NEW
>    If the server advertises a registered NNTP compression algorithm, it
>    MUST implement the compression algorithm so as to fully conform with
>    its related specification.  If it does not implement the compression
>    algorithm as specified, the implementation is considered a private
>    algorithm and MUST NOT be listed with the registered name in the
>    arguments list of the COMPRESS capability label.
>
>    Private algorithms with unregistered names are allowed, but SHOULD
>    NOT be used because it is difficult to achieve interoperability with them.
> END
>
> How's that?

All right.  I kept your second paragraph.
I removed the first one because on second thoughts, we don't really need 
saying it.  As Alexey commented in his review, "this sounds like 
motherhood and apple pie".

Thanks again, and have a nice week,

-- 
Julien ÉLIE

« – Tu parles ?
   – Tu parles ! » (Astérix)