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

Julien ÉLIE <julien@trigofacile.com> Wed, 21 September 2016 15:29 UTC

Return-Path: <julien@trigofacile.com>
X-Original-To: secdir@ietfa.amsl.com
Delivered-To: secdir@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 803C712B4DF for <secdir@ietfa.amsl.com>; Wed, 21 Sep 2016 08:29:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.92
X-Spam-Level:
X-Spam-Status: No, score=-1.92 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=-0.01, RCVD_IN_MSPIKE_WL=-0.01] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aQfcWqw5pW0T for <secdir@ietfa.amsl.com>; Wed, 21 Sep 2016 08:29:17 -0700 (PDT)
Received: from 4.mo177.mail-out.ovh.net (4.mo177.mail-out.ovh.net [46.105.37.72]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 4AAA412B4DA for <secdir@ietf.org>; Wed, 21 Sep 2016 08:29:11 -0700 (PDT)
Received: from player699.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo177.mail-out.ovh.net (Postfix) with ESMTP id 0F824FFC2C0 for <secdir@ietf.org>; Wed, 21 Sep 2016 17:29:04 +0200 (CEST)
Received: from RCM-mail305.ha.ovh.net (unknown [193.104.162.7]) (Authenticated sender: julien@trigofacile.com) by player699.ha.ovh.net (Postfix) with ESMTPSA id 6D54B24007D; Wed, 21 Sep 2016 17:28:55 +0200 (CEST)
Received: from [193.104.162.7] by ssl0.ovh.net with HTTP (HTTP/1.1 POST); Wed, 21 Sep 2016 17:28:55 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Date: Wed, 21 Sep 2016 17:28:55 +0200
From: =?UTF-8?Q?Julien_=C3=89LIE?= <julien@trigofacile.com>
To: Barry Leiba <barryleiba@computer.org>
Organization: TrigoFACILE
In-Reply-To: <CALaySJ+mJdorTkygsZ==Bja+0ZmavTkq2kC33QJ67LeM34K=Ng@mail.gmail.com>
References: <CALaySJ+mJdorTkygsZ==Bja+0ZmavTkq2kC33QJ67LeM34K=Ng@mail.gmail.com>
Message-ID: <20981db3190142193043f1445abadaa3@trigofacile.com>
X-Sender: julien@trigofacile.com
User-Agent: Roundcube Webmail/1.1.3
X-Originating-IP: 193.104.162.7
X-Webmail-UserID: julien@trigofacile.com
X-Ovh-Tracer-Id: 12051632606819450146
X-VR-SPAMSTATE: OK
X-VR-SPAMSCORE: -100
X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelvddrtddvgdekiecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd
Archived-At: <https://mailarchive.ietf.org/arch/msg/secdir/h_OBxf23vEp3zU3bEN9wkJ9Ozo8>
Cc: draft-murchison-nntp-compress.all@ietf.org, IESG <iesg@ietf.org>, barryleiba@gmail.com, secdir@ietf.org
Subject: Re: [secdir] Secdir review of draft-murchison-nntp-compress-05
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Security Area Directorate <secdir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/secdir>, <mailto:secdir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/secdir/>
List-Post: <mailto:secdir@ietf.org>
List-Help: <mailto:secdir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/secdir>, <mailto:secdir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 21 Sep 2016 15:29:20 -0000

Hi Barry,

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

Thanks a lot for your review and your comments.


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

OK.


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

OK.


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

I agree that we could change most of SHOULD to MUST here.
The remaining SHOULD would be:

     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

The rationale is because of the wording of RFC 4346, Section 2.1:

    The server MAY list the AUTHINFO capability with no arguments, which
    indicates that it complies with this specification and does not
    permit any authentication commands in its current state.

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.

What wording is preferrable?


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

We used SHOULD because RFC 4642 uses SHOULD in a in Section 2.2.2:

    If, after having
    issued the STARTTLS command, the client finds out that some failure
    prevents it from actually starting a TLS handshake, then it SHOULD
    immediately close the connection.
[...]
    If the TLS negotiation fails, both client and server SHOULD
    immediately close the connection.  Note that while continuing the
    NNTP session is theoretically possible, in practice a TLS negotiation
    failure often leaves the session in an indeterminate state;
    therefore, interoperability can not be guaranteed.

I therefore also suggest to keep SHOULD here.
Do you believe we should switch to MUST anyway?  (Wouldn't it be a
too strong constraint?)


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

Good point.  Besides, I believe I should add it anyway because of
Section 2.1:

    As the COMPRESS command is related to security because it can weaken
    encryption, cached results of CAPABILITIES from a previous session
    MUST NOT be relied on, as per Section 12.6 of [RFC3977].

CAPABILITIES is therefore required to be sent after a successful use
of COMPRESS.


> -- 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:".

Agreed.  It was just a lame copy/paste from previous RFCs defining
NNTP extensions.

FYI, the core documentation (RFC 3977) uses:

    Response code 100 (multi-line)
       Generated by: HELP
       Meaning: help text follows.

    Response code 111
       Generated by: DATE
       1 argument: yyyymmddhhmmss
       Meaning: server date and time.


Wording suggested for our document:

   This document defines the following new response code.  It is not
   multi-line and has no arguments.

    Response code 206
       Generated by: COMPRESS
       Meaning: compression layer activated


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

Je ne suis effectivement ni Shakespeare ni Molière :)


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

... because the reasons above do not strictly apply to that case.

Suggestion:

     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.  As a matter of fact, adversaries that observe
     the length of the compressed data might be able to derive
     information about it, when public data (that adversaries know is
     read) and confidential data are compressed in the same compress 
session.

Of course, if you have a more fluent suggestion of wording, you're 
welcome :)


I suggested SHOULD because I thought MUST was too strong.
It would imply that clients and servers have a way to specify
the confidentialness of newsgroups.  It currently does not exist,
and I doubt a user would maintain it in its news client.
That's why I had not used MUST, so as not to declare non-compliant
implementations that do not flag newsgroups as confidential or not.

Am I wrong with that consideration and should we use MUST anyway?


>    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]."

We had in mind the following scenario:

"[...], because adversaries might be able to derive information about
confidential articles as they may have identical header fields (like
Subject, Newsgroups or From) or header fields whose contents may be
predictable (like Xref or Injection-Info)."

Maybe "predictable" is not the right word.
"have a structured pattern"?  "are formatted in a known syntax?"

Examples:
   Injection-Info: news.trigofacile.com; posting-account="eliej";
         posting-host="news.trigofacile.com:2001:41d0:1:6d44::1";
         logging-data="29828"; mail-complaints-to="abuse@trigofacile.com"
   Xref: news.trigofacile.com trigofacile.test:566


Do you have a preferred wording for that paragraph?
(Should a reference to RFC 5536 be also mentioned?  Like "These header
fields are described in [RFC5536].")


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

This time, it correctly refers to Section 6 of [RFC3749].
See the beginning of Section 6 of the document:

    Implementers should be aware that combining compression with
    encryption like TLS can sometimes reveal information that would not
    have been revealed without compression, as explained in Section 6 of
    [RFC3749].


If we decide to use MUST instead of SHOULD before in the document, of
course it will need to be homogenized here.


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

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

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


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

Yep.


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

Just saying that an implementation has the possibility to do that (MAY)
but is not encouraged to (SHOULD NOT).


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

To respond your questions:  yes, unregistered algorithms MAY be
included if the implementation wishes to.  Yes, unregistered
algorithms SHOULD NOT be included (bad practice).
Why that?  because out of consistency between RFCs, I'm just declining
in this NNTP extension what Section 3.3.3 of RFC 3977
(core NNTP specification) mentions:

    If the server advertises a capability defined by a registered
    extension, it MUST implement the extension so as to fully conform
    with the specification (for example, it MUST implement all the
    commands that the extension describes as mandatory).  If it does not
    implement the extension as specified, it MUST NOT list the extension
    in the capabilities list under its registered name.  In that case, it
    MAY, but SHOULD NOT, provide a private extension (not listed, or
    listed with a different name) that implements part of the extension
    or implements the commands of the extension with a different meaning.

And I would just say that interoperability issues with that SHOULD NOT
is out of scope of this document.  Implementations SHOULD NOT do that.
A case I see is "private news servers and news clients" (for instance
Giganews [server] and Mimo [client developed specifically for the
Giganews Usenet service]).  They could implement their own unregistered
algorithm, and use it if they wish.  I believe they are clever enough
not to name it LZMA or BZIP2 but XGIGAFEATURE, GIGACOMPRESS...


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

Suggestion:

     The 206, 403, and 502 response codes a news server answers
     to COMPRESS using a private compression algorithm MUST have the
     same meaning as the one documented in Section 2.2 of this document.


Would it be OK for you with that rewording?

Thanks again for your valuable comments to help improve the quality
of the document.

-- 
Julien ÉLIE