[babel] Benjamin Kaduk's Discuss on draft-ietf-babel-information-model-11: (with DISCUSS and COMMENT)

Benjamin Kaduk via Datatracker <noreply@ietf.org> Tue, 03 November 2020 00:08 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 44EE83A135E; Mon, 2 Nov 2020 16:08:39 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: Benjamin Kaduk via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-babel-information-model@ietf.org, babel-chairs@ietf.org, babel@ietf.org, Donald Eastlake <d3e3e3@gmail.com>, d3e3e3@gmail.com
X-Test-IDTracker: no
X-IETF-IDTracker: 7.21.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Benjamin Kaduk <kaduk@mit.edu>
Message-ID: <160436211925.31069.7931310164710901437@ietfa.amsl.com>
Date: Mon, 02 Nov 2020 16:08:39 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/babel/KNzgGkbfKHbPlSMleUQ3yISbt3s>
Subject: [babel] Benjamin Kaduk's Discuss on draft-ietf-babel-information-model-11: (with DISCUSS and COMMENT)
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: Tue, 03 Nov 2020 00:08:39 -0000

Benjamin Kaduk has entered the following ballot position for
draft-ietf-babel-information-model-11: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-babel-information-model/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Section 2 says that the "DTLS certificate values" are required to return
no value when read, but this property seems to be intended for private
data such as DTLS private key values, not the certificates themselves
(which are public).

While I appreciate that IPv6 is the current version of the internet
protocol, I do see that 6126bis allows for Babel to run over both IPv6
and IPv4, yet this document in multiple places implicitly assumes that
Babel runs over IPv6, to the exclusion of IPv4.  Such a restriction from
the core protocol spec should only be undertaken by an information model
with clear reasoning and loud disclaimer.

Similarly (as Roman notes), we are putting requirements on the key
length for MAC keys (relative to the block size of the underlying hash
function) that have were once present in draft-ietf-babel-hmac but have
been removed as of draft-ietf-babel-hmac-10.  I assume that the intent
is not to impose additional restrictions compared to the protocol spec,
thus we need to catch up to those changes.

The description of the babel-mac-key-test and babel-cert-test operations
need to be tightened up, as the secdir reviewer noted.  (See COMMENT.)

We seem to be using terminology from the Network Management Datastore
Architecture without reference or otherwise introducing the concepts.
This is a Discuss point because the only candidate reference I know of,
RFC 8342, is specific to YANG and data models, so it's applicability for
use in an information model may be subject to discussion.  (Hopefully
this only reflects my ignorance and not a fundamental lack of datastore
architecture for information models.)


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 1.1

Please use the specific RFC 8174 boilerplate (in particular, it includes
"NOT RECOMMENDED").

Section 2

We have separate MAC/DTLS-enablement nodes at a per-interface
level, so not having them at the global level is perhaps suprising.

I'm happy to see babel-dtls-cert-types, given that the babel/DTLS spec
leaves much open as to how to authenticate peers.  Even more specificity
could be useful.

   Most parameters are read-only.  Following is a descriptive list of
   the parameters that are not required to be read-only:

It's suprising to not see router-id on this list; 6126bis says only that
"every Babel speaker is assigned a router-id" without saying how.  In
the absence of a "how", it seems reasonable to assume "assigned by the
administrator" as a valid option.

How do I configure which prefixes to advertise as originated from this
router?  Do I just add something to the babel-routes list with NULL
received metric?  But if that's how I do it, then the babel-route-obj
can't be 'ro'...

   o  Interface: Metric algorithm

   o  Interface: Split horizon

   o  Interface: enable/disable Babel on this interface
   [...]

It might be useful to list these in the same order as they appear in the
tree diagram.

   o  Interface: MAC algorithm

What node in the tree does this correspond to?

Section 3.1

   babel-enable:  When written, it configures whether the protocol
      should be enabled (true) or disabled (false).  A read from the
      running or intended datastore indicates the configured
      administrative value of whether the protocol is enabled (true) or
      not (false).  A read from the operational datastore indicates

Perhaps it's just me, but running/intended/operational datastores feels
like a YANG/NMDA thing and is surprising to see in a nominally generic
information model, absent further reference.
(Similarly for subsequent usage of the terms.)

   babel-self-router-id:  The router-id used by this instance of the
      Babel protocol to identify itself.  [I-D.ietf-babel-rfc6126bis]
      describes this as an arbitrary string of 8 octets.  The router-id
      value MUST NOT consist of all zeroes or all ones.

Why is the MUST NOT a requirement of the information model rather than
the protocol?

   babel-metric-comp-algorithms:  List of supported cost computation
      algorithms.  Possible values include "2-out-of-3", and "ETX". "2-
      out-of-3" is described in [I-D.ietf-babel-rfc6126bis], section
      A.2.1.  "ETX" is described in [I-D.ietf-babel-rfc6126bis], section
      A.2.2.

Perhaps this is just me, but the way this is written implies that the
specific string values are to be used, which may be overly prescriptive
for an information model.  Also, is there a registry for these
algorithms that could be referenced?

   babel-security-supported:  List of supported security mechanisms.
      Possible values include "MAC" and "DTLS".

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

   babel-dtls-cert-types:  List of supported DTLS certificate types.
      Possible values include "X.509" and "RawPublicKey".

Likewise, are there registries for these?  (D)TLS does have an existing
certificate types registry
(https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#tls-extensiontype-values-3
is the one to use), but for the MAC algorithms that's pretty inherently
a very flexible extension point so it's easy to add a new algorithm with
no or a very minimal written spec.

Section 3.2

   babel-mcast-group:  Multicast group for sending and listening to
      multicast announcements on IPv6.  Default is ff02::1:6.  An

IIRC the core protocol only has it as RECOMMENDED for control traffic to
be over IPv6; the "on IPv6" here seems unnecessarily limiting.

Section 3.3

   babel-interface-reference:  Reference to an interface object that can
      be used to send and receive IPv6 packets, as defined by the data

[again the implicit IPv6 requirement]

   babel-mcast-hello-interval:  The current interval in use for
      multicast Hellos sent on this interface.  Units are centiseconds.
      This is a 16-bit unsigned integer.

Perhaps it is better to discuss that the units need to have sufficient
precision to represent centisecond granularity rather than to enforce a
specific unit and constrain data models/implementations.
(Similarly for other mentions of units.)

   babel-dtls-cached-info:  Indicates whether the cached_info extension
      is included in ClientHello and ServerHello packets.  The extension

Please reference RFC 7924 here.

      is included if the value is "true".  An implementation MAY choose
      to expose this parameter as read-only ("ro").

I wonder if we can/should give a bit more guidance on what to include in
the extension, as currently it would be compliant with this spec (but
not very useful) to emit a cached_information extension that is highly
unlikely to result in any packet size savings.

   babel-dtls-cert-prefer:  List of supported certificate types, in
      order of preference.  The values MUST be among those listed in the
      babel-dtls-cert-types parameter.  This list is used to populate
      the server_certificate_type extension in a Client Hello.  Values

An RFC 7250 reference is probably in order.

   babel-packet-log:  A reference or url link to a file that contains a
      timestamped log of packets received and sent on babel-udp-port on
      this interface.  The [libpcap] file format with .pcap file
      extension SHOULD be supported for packet log files.  Logging is

Does there need to be a mechanism for content-type
negotiation/indication?

Section 3.4

Shouldn't these all be 'counter's, not 'uint's?

Section 3.5

   babel-hello-mcast-history:  The multicast Hello history of whether or
      not the multicast Hello packets prior to babel-exp-mcast-hello-
      seqno were received.  A binary sequence where the most recently
      received Hello is expressed as a "1" placed in the left-most bit,

This seems to indicate that the leftmost bit is always '1', and thus
that we cannot be in a situation where we missed one expected multicast
hello and are already expecting "the one after it".  Is that correct?

Also, should we say anything about truncating the bitstring at some
arbitrary point?

(Similarly for the unicast case, on both counts.)

   babel-exp-ucast-hello-seqno:  Expected unicast Hello sequence number
      of next Hello to be received from this neighbor.  If unicast Hello
      packets are not expected, or processing of unicast packets is not
      enabled, this MUST be NULL.  This is a 16-bit unsigned integer; if

(We haven't defined "NULL" semantics yet.)

Section 3.6

   babel-route-neighbor:  Reference to the babel-neighbors entry for the
      neighbor that advertised this route.

Wouldn't that make this a "reference" type rather than "string"?

   babel-route-seqno:  The sequence number with which this route was
      advertised.  This is a 16-bit unsigned integer.

Is this text correct for locally originated routes?

Section 3.7

I don't wish to revisit the decision, but it might have been interesting
to record some of the reasoning for having an additional abstraction for
"key set" and having a list of key-sets, vs just having a list of keys
directly.  (Similarly for the DTLS cert sets.)

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

I agree with the secdir reviewer that the "sign" terminology is
problematic here, and would prefer something like
"babel-mac-key-use-generate" and a similar wording in the prose.

   babel-mac-key-value:  The value of the MAC key.  An implementation
      MUST NOT allow this parameter to be read.  This can be done by
      always providing an empty string when read, or through
      permissions, or other means.  This value MUST be provided when
      this instance is created, and is not subsequently writable.  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].

[Per the Discuss, this key-length guidance is not aligned with
draft-ietf-babel-hmac.]

   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.

s/create a hash of/create a MAC over/
s/resulting hash/resulting MAC value/
Given that the intent is to test the MAC operation, it seems like we
might want to say that the input string is treated as a babel packet,
getting the pseudo-header added per draft-ietf-babel-hmac §4.1, etc.
It would be in keeping with cryptographic best practice to continue to
use the same pseudo-header (and possibly even include a disambiguating
context string) to avoid the risk of being an oracle for generating the
MAC of arbitrary text that could then be used to forge other packets
elsewhere.

As the secdir review noted, the MAC output length is not necessarily
fixed by the algorithm, so some indicatino of the output length is also
in order.

Section 3.10

   babel-cert-name:  A unique name for this DTLS certificate that can be
      used to identify the certificate in this object instance, since
      the value is too long to be useful for identification.  This value

Some guidance on whether or not it is expected to be useful to draw
naming information from the certificate's Subject information, vs an
arbitrary or fingerprint-based naming scheme, might be in order.

Also, it's somewhat unusual to talk about "(D)TLS certificates" as
opposed to X.509 certificate, raw public key, etc..

   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.

This is problematic in several ways, as noted by the secdir reviewer.
For one, if we want to test a certificate, we usually would do that by
producing a signature, not a hash over the public key and some other
input.  Furthermore, not all the signatures produced by X.509 certificates
compatible with DTLS require a hash at all or are allowed to use SHA-256
within the signature operation.  It may be possible to require SHA-256
always by having the input to the signature operation be the SHA-256
output, which would then be hashed again during the process of computing
an (e.g., RSA) signature, but that is also a bit weird.
The purpose of the operation needs to be made more clear (is it to
verify the public key?  The private key?) as well as how the input is
structured; if the certificate private key is used to generate a
signature we must take care to avoid producing a signing oracle that can
be used to produce signatures valid in other contexts.

Section 5

We do expose an operation to get a packet dump, but it's not clear that
there are particularly noteworthy security considerations regarding that
-- the dump would appear to be the ciphertext based on the language
used, so it would not be a way to bypass DTLS encryption, for example.

   This information model defines objects that can allow credentials
   (for this device, for trusted devices, and for trusted certificate
   authorities) to be added and deleted.  Public keys may be exposed
   through this model.  This model requires that private keys never be

It might be worth another sentence indicating the scale of the
consequences of erroneously/maliciously setting such credentials.

   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.

The DTLS one really doesn't, though -- it says only something like
"details of identity management are left to deployment profiles", and
there's a wide variety of DTLS credentials that are possible.
The MAC mechanism has a much clearer picture about what is allowed by
virtue of using the raw crypto primitive (though the allowed MAC
algorithms are negotiated out-of-band there as well).

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

I don't think it's true to say that "keys that make use of only
alphanumeric characters are highly susceptible to brute force attacks".
Even if I stick to a 32-byte key, `dd if=/dev/random bs=1
count=24|openssl base64` is giving me 192 bits of randomness, which is
plenty for a modern security protocol.  I think you mean to say that
short keys are especially susceptible to brute-force when they only use
alphanumeric characters.

Section 8.2

Per https://www.ietf.org/iesg/statement/normative-informative.html even
optional features like DTLS, MAC, RFC 3339 timestasmps, etc., should be
listed as normative references.