Last Call Review of draft-murchison-nntp-compress-05
review-murchison-nntp-compress-05-secdir-lc-leiba-2016-09-22-00

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.

There are no major issues here, but there are a number of things I
think need to be addressed before this document is ready.

References:
RFC 1951 (DEFLATE) MUST be implemented with this, so it needs to be a
normative reference.

-- Section 2.1 --

   This document defines one
   compression algorithm: DEFLATE.  This algorithm is mandatory to
   implement and MUST be supported in order to advertise the COMPRESS
   extension.

Just to be clear and specific, I suggest changing the second sentence
to say "and MUST be supported and listed in the advertisement of the
COMPRESS extension."

-- Section 2.2.2 --

   In order to help mitigate leaking authentication credentials via for
   instance a CRIME attack [CRIME], authentication SHOULD NOT be
   attempted when a compression layer is active.  Consequently, a server
   SHOULD NOT return any arguments with the AUTHINFO capability label
   (or SHOULD NOT advertise it at all) in response to a CAPABILITIES
   command received from an unauthenticated client after a compression
   layer is active, and such a client SHOULD NOT attempt to utilize any
   AUTHINFO [RFC4643] commands.  It implies that a server SHOULD reply
   with a 502 response code if a syntactically valid AUTHINFO command is
   received while a compression layer is already active.

Why are these SHOULD, and not MUST?  Under what conditions would it be
necessary or reasonable for an implementation not to abide by these,
and what considerations need to be considered when making that
determination?  (And this is also directly referred to in Section 6.)

   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.)

Same question.

-- Section 2.3 --

At the end of the first example, it would be useful to add another
CAPABILITIES command to show that COMPRESS DEFLATE is no longer
listed.

-- Section 5 --

   This section contains a list of each new response code defined in
   this document and indicates whether it is multi-line, which commands
   can generate it, what arguments it has, and what its meaning is.

This is a total nit, but this sentence seems odd when there's only one
new code (there's also nothing that says whether it is or isn't
multi-line).  I suggest, instead, "The following new response code is
defined in this document:".

-- Section 6 --

   NNTP commands other than AUTHINFO are not believed to divulgate
   confidential information

Qu'est que c'est "divulgate"?  Perhaps you mean "divulge", unless
you're Shakespeare.

   In case confidential articles are accessed in private
   newsgroups, special care is needed: implementations SHOULD NOT
   compress confidential data together with public data when a security
   layer is active, for the same reasons as mentioned above in this
   Section.

Sorry: it is not clear to me what reasons those are; can you be
specific here?  And why SHOULD, and not MUST?

   Additionally, implementations MAY ensure that the contents of two
   distinct confidential articles are not compressed together.

Of course they MAY, but what's the point of saying that.  I think
you'd be much better off skipping the 2119 key word here, and instead
say something about why it might be good to do this: what's the
advantage?  Something like, "Additionally, implementations would do
well to ensure that the contents of two distinct confidential articles
are not compressed together, because [of this advantage]."

   Future extensions to NNTP that define commands conveying confidential
   data SHOULD ensure to state that these confidential data SHOULD NOT
   be compressed together with public data when a security layer is
   active.

I guess this is related the the paragraph I quoted immediately above,
so the question is the same.

-- Section 7.1 --

   Any name that conforms to the syntax of an NNTP compression algorithm
   name (Section 4.3) can be used.

It would be useful here, for IANA's benefit, to specifically highlight
that letters in algorithm names are always upper case.

   However, the name of a registered
   NNTP compression algorithm MUST NOT begin with "X".

   To avoid the risk of a clash with a future registered NNTP
   compression algorithm, the names of private compression algorithms
   SHOULD begin with "X-".

This was discussed in response to another review, and I understand the
"X-" stuff will be removed.  Thanks.

   If it does not implement the compression
   algorithm as specified, it MUST NOT list its registered name in the
   arguments list of the COMPRESS capability label.  In that case, it
   MAY, but SHOULD NOT, provide a private algorithm (not listed, or
   listed with a different name) that implements the compression
   algorithm differently.

Oy.  This really sounds convoluted to me, and the "MAY, but SHOULD
NOT" is a direct contradiction.

I think what you're trying to say here is that if you list a
registered algorithm, it MUST be properly implemented according to the
relevant spec.  Got that.  Now, what about including unregistered
algorithms?  Do you want to say that you MAY include unregistered
algorithms?  Do you want to say that you SHOULD NOT include any?  Why
not, and why might you decide to do that anyway?  What are the
interoperability issues with respect to unregistered algorithms?

   A server MUST NOT send different response codes to COMPRESS in
   response to the availability or use of a private compression
   algorithm.

I don't understand what this is saying at all.  Different from what?
Can you clarify this?

-- 
Barry