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

["STARK, BARBARA H" <[e-mail]>]

Hi Valery,
Thanks for your review. Taking into account Juliusz' response to your email, I'm proposing the following resolutions. For the first issue raised, I'm disagreeing slightly with Juliusz, so that may need further discussion. Please let me know if you disagree. I've copied Juliusz' responses here, so I can include my responses to your complete feedback and his in one email.
Roman specifically mentioned he agreed with your "recommendations about precise language around the names of the parameters". I think perhaps this was in reference to item 4 under nits? I didn't completely agree with your recommendation, so I'd appreciate some additional discussion on that.
Thx,
Barbara

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

Juliusz said:
> Right.  The intent is that Blake2s is used with 32-octet keys and 16-octet
> hashes (collision-resistance is not a concern for Babel-MAC while
> dictionary attacks are).  Barbara, I think that you should explicitly
> state that Blake2s implies 128-bit hashes.  (You may also consider
> renaming BLAKE2s to BLAKE2s-128.)

The defined values for babel-mac-algorithms come directly from draft-ietf-babel-hmac. The defined value names should map closely to the names used for the algorithms in in that draft -- which they currently do. 

If it needs to be explicitly stated somewhere that an implementation of draft-ietf-babel-hmac with BLAKE2s outputs 128-bit MACs, then draft-ietf-babel-hmac (which was already submitted for publication) would be the correct place to say that. The information model is not the right place, unless there's some expectation for the size to be configurable or reportable. I'm not seeing any request for the MAC size to be configured or reported via the information model. 

I'm proposing no change to the defined values of babel-mac-algorithms in order to maintain complete consistency with the names used in draft-ietf-babel-hmac-12.

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

One of the most common problems in configuring security mechanisms is in the format of the input key (hex, ASCII, base64, hashing that occurs to create "actual key", etc.). When a security mechanism fails to work, it is important for users or device managers to be able to trouble-shoot this specific point of failure. This test allows the user/manager to see if what this device thinks the MAC should be is the same as what another device thinks the MAC should be or is the same as the MAC being sent on the wire. Many ISPs have built a test like this into their ISP-supplied CE routers (invoked using the TR-069 protocol and TR-181 data model) to test various stored key values. It has proven useful.

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

Thanks. I'll add a reference to draft-ietf-babel-hmac Security Considerations.

Zero length and short keys were discussed on the mailing list. The group considered it appropriate to
allow configuration of zero-length keys for testing but to advise people to follow best
current practices. I find the use of normative language to attempt to control the behavior of 
a home network owner (for example) or someone setting up an informal ad hoc mesh
network (for example) to be odd. IMO, the IETF should not seek to control
the choices of people putting together such relatively small-scale networks through 
the use of strong normative language in an information model specification. It's 
impossible to enforce and such people pretty much never read RFCs.

If there is a strong desire for some sort of normative language, then I could suggest
OLD
   MAC keys are allowed to be as short as zero-length.  This is useful
   for testing.  Network operators are advised to follow current best
   practices for key length and generation of keys related to the MAC
   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.
NEW
   MAC keys are allowed to be as short as zero-length.  This is useful
   for testing.  Network operators are RECOMMENDED to follow current best
   practices for key length and generation of keys related to the MAC
   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. See the Security Considerations
  section of [ID.draft-ietf-babel-hmac] for additional considerations 
  related to MAC keys.

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

Thank you. I will make this change.
 
> 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.

The authors have double- and triple-checked, but human error is still possible, since these were not auto-generated (like YANG does). Whenever redundant information is presented, it is critical that it be normative in only one place. This language makes it clear which is normative and what to do in the case of discrepancies. I propose no change. If there is a strong push to remove this language, then I believe the correct approach would be to remove the entire section (remove the redundancy) rather than to allow confusion if there is an error. But people have found this section useful, so I would prefer to leave it as is.
 
> 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.

Thank you. I'll change it to MAC.

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

I've been thinking through this. I can't speak to the informal nature of "sign", but I can say that simply replacing "sign" with "compute" or "mac" wouldn't convey correctly what this parameter is about. This parameter is primarily concerned with whether or not a MAC is included in the sent packet. The sending is the critical piece, and not the computing (it's possible to compute the MAC without sending it; a MAC in a sent packet is assumed to have been computed). I could change the description to:
       Indicates whether this key value is used to compute a MAC and include that MAC in the
       sent Babel packet.  A MAC for sent packets is computed using this key
       if the value is "true".  If the value is "false", this key is not
       used to compute a MAC to include in sent Babel packets.  An implementation MAY choose to
       expose this parameter as read-only ("ro")

But I struggle with the proposed parameter renaming. I strongly believe the name should concisely describe that the Boolean value indicates whether or not to include a MAC in the sent packet. The term "sign" is one I've commonly seen to indicate that a MAC is included in the sent packet. I'm not aware of a different, similarly short word. "Compute" and "mac" do not convey the sending aspect. And sending is very asymmetric.
 
> 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.

Juliusz said:
> This was discussed at length on the mailing list.  It's not about
> performance, it's about making it more difficult to use an unsafe
> procedure for generating keys.
> 
> Since Babel-MAC is vulnerable to dictionary attacks, the key must either
> be drawn randomly or generated using a procedure that is hardened against
> such attacks (scrypt, etc.).  Applying the procedure described in RFC 2104
> to a user-provided passphrase is not safe, and therefore we try to make it
> difficult for a naive user to do so.
> 
> I am opposed to putting the RFC 2104 hashing procedure in the information
> model.  Doing so would be a disservice to our users.

In addition to the rationale Juliusz mentioned, we (babel WG) also noted that implementers
of the babel MAC function were using existing libraries for the HMAC-SHA256 algorithm.
The user interface (UI) that accepted manual key entry was also from an existing library. When
the same longer strings were entered into different UIs of the different implementations, these
strings were treated differently and resulted in non-interoperability. The "actual key" (using
RFC 2104 words) ended up different. Requiring entered keys to be directly usable as "actual
keys" solved this problem. BTW, I have considered UIs for direct management and configuration
to effectively be implementations of the information model. 

I recommend no changes to this text.
 
> 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.

If I understand, the suggestion is to reword as:
   babel-mac-key-test:  An operation that allows the MAC key and MAC
      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 MAC of this string using the babel-mac-key-
      value and the babel-mac-key-algorithm.  The output of this
      operation is the resulting MAC, as a binary string.

If this is right, I'll make that change.

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

Thank you. I'll make this change.
 
> 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?

As with the -test actions, this allows someone to diagnose whether a problem they are having is with the formatting 
of the input key (hex, padded, ASCII, base64, etc.). This is by far one of the most common problems when attempting to
get different implementations to interoperate.

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

Juliusz said:
> The key length is not carried in the clear by the protocol.  Guessing the
> key length requires a brute-force attack, even when it is zero.

I propose no change. 

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

Juliusz said:
> Yes.  Babel-DTLS is intended to inherit the security properties of the
> system's DTLS library.  If the DTLS library is unsafe, then Babel-DTLS
> must not be used until the library is fixed.