[babel] Secdir last call review of draft-ietf-babel-information-model-11

Valery Smyslov via Datatracker <noreply@ietf.org> Mon, 26 October 2020 14:54 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: babel@ietf.org
Delivered-To: babel@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id D5CA63A0C06; Mon, 26 Oct 2020 07:54:39 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Valery Smyslov via Datatracker <noreply@ietf.org>
To: <secdir@ietf.org>
Cc: last-call@ietf.org, babel@ietf.org, draft-ietf-babel-information-model.all@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 7.20.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <160372407981.20077.17795340180313190981@ietfa.amsl.com>
Reply-To: Valery Smyslov <valery@smyslov.net>
Date: Mon, 26 Oct 2020 07:54:39 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/babel/CSSGdKT7SKzx4pNQiw8LblgPC-A>
Subject: [babel] Secdir last call review of draft-ietf-babel-information-model-11
X-BeenThere: babel@ietf.org
X-Mailman-Version: 2.1.29
List-Id: "A list for discussion of the Babel Routing Protocol." <babel.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/babel>, <mailto:babel-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/babel/>
List-Post: <mailto:babel@ietf.org>
List-Help: <mailto:babel-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/babel>, <mailto:babel-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Oct 2020 14:54:40 -0000

Reviewer: Valery Smyslov
Review result: Has Issues

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.

The document defines an informational model for Babel routing protocol. This
informational model can be used as a basis for creating various data models
(e.g. YANG). The document is properly structured and well written, however I
think that it has some issues concerned with security.

Issues.

1. Section 3.1:

   babel-mac-algorithms:  List of supported MAC computation algorithms.
      Possible values include "HMAC-SHA256", "BLAKE2s".

BLAKE2s can produce MACs of different sizes from 1 to 32 bytes and the desired
size of the MAC is a parameter for it. Where the size of MAC is specified? For
HMAC with SHA256 I can at least imagine that full 256 bits output is used as a
MAC...

2. Section 3.9:

   babel-cert-test:  An operation that allows a hash of the provided
      input string to be created using the certificate public key and
      the SHA-256 hash algorithm.  Input to this operation is a binary
      string.  The output of this operation is the resulting hash, as a
      binary string.

I failed to understand what this operation should do. Literally reading it is
intended to produce SHA2-256 hash of public key and some arbitrary string
(concatenated? in what order?). But then I failed to understand the purpose of
this test. I would have understood if this operation provides signing of the
arbitrary string using private key and SHA2-256 as a hash function (similarly
to babel-mac-key-test), but it in not what is written...

3. Section 5 (Security Considerations):

I think that text about keys (their length and properties) needs some
expansion. First, there are no any RFC2119 words discouraging using short and
weak keys (there is some text, but without RFC2119 words and with no references
it's just hand waving). Note, that draft-ietf-babel-hmac-12 has some text about
the properties of the keys, so I believe at least it must be referenced here. I
also suspect that explicitly allowing zero-length and short keys will lead to
situations when some network operators will use them (because they are not
prohibited), thus subverting security properties of MAC...

Nits.

1. Section 1 (Introduction):

   [I-D.ietf-babel-hmac] defines a
   security mechanism that allows Babel packets to be cryptographically
   authenticated, and [I-D.ietf-babel-dtls] defines a security mechanism
   that allows Babel packets to be encrypted.

DTLS provides both confidentiality and authentication of data, so to be
formally correct, please add "both authenticated and" before "encrypted".

2. Section 2 (Overview) at the very end:

   Note that this overview is intended simply to be informative and is
   not normative.  If there is any discrepancy between this overview and
   the detailed information model definitions in subsequent sections,
   the error is in this overview.

This phrase makes me puzzled. The tree-like description of the information
model in the Overview is very useful, however this phrase seems to discourage
using it, because authors are not sure it is correct. I think it would be
better for readers if authors double check that no discrepancy exists between
the overview and the subsequent detailed description and remove this phrase.

3. Section 3.3:

   babel-mac-verify  A Boolean flag indicating whether MAC hashes in
      incoming Babel packets are required to be present and are
      verified.  If this parameter is "true", incoming packets are
      required to have a valid MAC hash.  An implementation MAY choose
      to expose this parameter as read-only ("ro").

"MAC hashes" is strange wording, "MAC values" or just "MACs" are better in my
opinion.

4. Section 3.8:

   babel-mac-key-use-sign:  Indicates whether this key value is used to
      sign sent Babel packets.  Sent packets are signed using this key
      if the value is "true".  If the value is "false", this key is not
      used to sign sent Babel packets.  An implementation MAY choose to
      expose this parameter as read-only ("ro").

"Sign" is not a good word when you describe symmetric key operations (which
computing MAC belongs to). Although it is often used informally, I think that
RFC should be more meticulous in selecting words. I'd rather replace it with
"compute MAC" and rename the entry to babel-mac-key-use-compute or
babel-mac-key-use-mac (if it is possible). Note, that using "verify MAC" is OK.

5. Section 3.8:

   babel-mac-key-value:
       ...
      This value is of a length suitable for the associated babel-mac-key-
      algorithm.  If the algorithm is based on the HMAC construction
      [RFC2104], the length MUST be between 0 and the block size of the
      underlying hash inclusive (where "HMAC-SHA256" block size is 64
      bytes as described in [RFC4868]).  If the algorithm is "BLAKE2s",
      the length MUST be between 0 and 32 bytes inclusive, as described
      in [RFC7693].

I wonder of the rationale for imposing the above restrictions on HMAC key
length. HMAC can use keys of any length, but if the key is greater than block
size of underlying hash function, then it's first hashed (small performance
penalty). So I imagine that the rationale is to avoid this penalty. However, as
RFC2104 states, key sizes greater than output length of the underlying hash
function (32 bytes in case of SHA2-256) would not significantly increase the
function strength, so it's just a waste of space. See also Issue 3 above.

6.    Section 3.8:

   babel-mac-key-test:  An operation that allows the MAC key and hash
      algorithm to be tested to see if they produce an expected outcome.
      Input to this operation is a binary string.  The implementation is
      expected to create a hash of this string using the babel-mac-key-
      value and the babel-mac-key-algorithm.  The output of this
      operation is the resulting hash, as a binary string.

The text mixes up words "hash" and "MAC". MAC is not a hash (even if MAC
algorithm is often based on hash function). As with my previous grunt, I'd like
RFC to be more meticulous in selecting words. Please, avoid using "hash" here.

7. Section 5 (Security Considerations):

   ... This model requires that private keys never be
   exposed.  The Babel security mechanisms that make use of these
   credentials (e.g., [I-D.ietf-babel-dtls], [I-D.ietf-babel-hmac])
   identify what credentials can be used with those mechanisms.

Please add "and MAC keys" after "private keys" since formally private keys are
only defined for public key cryptography.

8. Section 5 (Security Considerations):

   MAC keys are allowed to be as short as zero-length.  This is useful
   for testing.

I wonder what's benefit of allowing zero-length keys for testing purposes. What
is intended to be tested in this case? Implementation of MAC? Is it really
needed outside test lab? Am I missing something?

9. Section 5 (Security Considerations):

    Short (and zero-length) keys and
   keys that make use of only alphanumeric characters are highly
   susceptible to brute force attacks.

Formally, brute force attack with zero-length keys is not defined, since there
is no key to find and all is in clear.

Comments.

1. The document contains an entry in the Informational model defining which
hash functions can be used with HMAC authentication. However, there is no
corresponding entry of which ciphersuites can be used with DTLS. Is it up to
DTLS library to select ciphersuites?