Subject: Re: [babel] [Babel-users] MAC auth. for Babel in babeld

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Wed, 23 Sep 2020 00:27:14 +0200

Hi Antonin

Great to see! A few comments:

> The code has not undergone review. No interoperability testing has
> been done.

I've done basic interoperability testing with the latest version of my
MAC implementation for Babel in Bird. They can successfully exchange
messages with both hmac-sha256 and blake2s hash algorithms configured.

A few comments/suggestions on the babeld implementation:

- You don't enforce a minimum key length. For blake2s this means the key
  will effectively be zero-padded up to the block size of 32 bytes (not
  sure what hmac-sha256 does). For Bird I'm enforcing that the key size
  must match the hash output size (32 bytes for both blake2s and
  hmac-sha256). This is based on the security considerations section in
  the draft; should babeld do the same? (this also tripped me up when
  testing, because I accidentally pasted a truncated key into the babeld
  config without noticing).

- I think the configuration is a bit verbose. This was the minimal
  config I needed to enable MAC in babeld:

key name test algorithm hmac-sha256 value 7465737474657374746573747465737474657374746573747465737474657374 use both
keyset test
keyset-add-key test test
interface veth0 mac true add-keyset test

The two middle lines feel like they are a bit redundant; could we go
without them for simple configs?

-Toke

---

From: Antonin Décimo <[email-masked]>
Date: Wed, 23 Sep 2020 16:59:51 +0200

Hello Toke,

> messages with both hmac-sha256 and blake2s hash algorithms configured.

Yay! Thank you very much! I’m glad it worked (on the first try) :-)

> - You don't enforce a minimum key length.

This was on purpose, but maybe it was a bad idea.

> For blake2s this means the key will effectively be zero-padded up to
> the block size of 32 bytes (not sure what hmac-sha256 does).

hmac-sha256 is the same. Under the block size, the key is padded with
zeroes, above, the key is pre-hashed.

> For Bird I'm enforcing that the key size must match the hash output
> size (32 bytes for both blake2s and hmac-sha256). This is based on
> the security considerations section in the draft; should babeld do
> the same?

Now babeld requires keys of 32 bytes for both algorithms.

> key name test algorithm hmac-sha256 value
> 7465737474657374746573747465737474657374746573747465737474657374 use
> both
> keyset test
> keyset-add-key test test

You’re right; at least the line `keyset test` can go, keysets are now
implicitly created. I don’t see how I can further simplify without
losing the idea that interfaces may share keysets and keysets may
share keys.

One thing that is still buggy in my implementation is to use the
"default" pseudo-interface with keysets and keys, this may simplify
the configuration for common usecases.

-- Antonin

---

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Wed, 23 Sep 2020 17:47:41 +0200

> Yay! Thank you very much! I’m glad it worked (on the first try) :-)

Well, I did test an earlier version of your code against an earlier
version of me, so not quite first try ;)

> zeroes, above, the key is pre-hashed.

Yeah, that's what the Bird implementation is doing too. I'm also
limiting the *max* length of keys to avoid the pre-hashing as I figured
that would be an interoperability issue. But if it's standard practice
maybe that is not necessary?

> Now babeld requires keys of 32 bytes for both algorithms.

Great!

> losing the idea that interfaces may share keysets and keysets may

The obvious thing would be to assign keys to a "default" keyset if no
explicit keyset is configured, and assign that default keyset to all
interfaces that don't specify one?

I think that would be safer anyway, as right now it's quite easy to
think you've configured authentication, but it's not actually enabled
because there was not key assigned. So I think the interface config
should be bailing out if 'mac true' is set but no key is available...

> the configuration for common usecases.

Yeah, for more complex configurations that will help, of course. I did
not test the config interface extensively, just did the minimum to get
things working ;)

-Toke

---

From: Juliusz Chroboczek <[email-masked]>
Date: Wed, 23 Sep 2020 17:51:38 +0200

> losing the idea that interfaces may share keysets and keysets may

Hmm... let's discuss the following ideas:

  - make "use both" optional;
  - rename "add-keyset" to just "keyset";
  - rename "keyset-add-key" to just "add-key";
  - allow the interface configuration to say "key", in which case we
    internally create an anonymous keyset with just one key.

This would make the minimal configuration be just:

  key name mykey algorithm hmac-sha256 value xxx
  interface veth0 mac true key mykey

---

From: Antonin Décimo <[email-masked]>
Date: Mon, 28 Sep 2020 15:31:29 +0200

> version of me, so not quite first try ;)

Second first try, then.

> maybe that is not necessary?

It is standard. If the key length is smaller than the output (block)
size, then the key is padded with zeroes. If the key length is bigger,
then it is pre-hashed. See https://tools.ietf.org/html/rfc2104#section-2

Right know, babeld requires keys to be of the exact output size of the
hash.

> interfaces that don't specify one?

I want to use the 'default' pseudo interface so that each keyset that
is added to the default interface is automatically added to each
interface.

> should be bailing out if 'mac true' is set but no key is available...

It’s not clear when this check should be, because keys could be added
from another channel, for instance a local socket but not the
babeld.conf file.
I can add warnings when signing and verifying packets if there are no
key available.

-- Antonin

---

From: Antonin Décimo <[email-masked]>
Date: Mon, 28 Sep 2020 16:05:24 +0200

> - make "use both" optional;

I dislike "use both" anyway. I’ve removed it. If you don't specify the
use, then the key is used for signing and verifying. If you specify a
use, then the key is used only for it.

> - rename "add-keyset" to just "keyset";

I disagree, this would break symmetry with "rm-keyset".

> - rename "keyset-add-key" to just "add-key";

Done.

> internally create an anonymous keyset with just one key.

Done. The anonymous keyset takes the name of the interface.

---

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Mon, 28 Sep 2020 16:38:28 +0200

>> version of me, so not quite first try ;)

Haha, yeah :)

> then it is pre-hashed. See https://tools.ietf.org/html/rfc2104#section-2

Ah, good to know!

> Right know, babeld requires keys to be of the exact output size of the

I ended up with something similar, except for HMAC keys it allows
OUTPUT_SIZE <= key size <= BLOCK_SIZE:

https://github.com/tohojo/bird/blob/babel-mac-02/proto/babel/config.Y#L112

> is added to the default interface is automatically added to each

Yes, that does make things simpler, but still requires explicit
assignment of a key, even if it's just to the default interface. Maybe
you could add a keyword to the key definition itself ('default'?) that
does this without a separate keyset line?

> I can add warnings when signing and verifying packets if there are no

Yeah, dynamic config does make things a bit more complicated, but I
still think it's possible to add a check that will prevent most errors.
I don't think warning on processing is enough, it has to be on config
time (or an operator can still restart the daemon, not get an error, and
forget to check the logs).

You could simply reject 'mac true' if no key is configured (i.e., reject
interface bring-up or reconfig, as appropriate depending on context).
This imposes constraints on the ordering of config stanzas, but I think
that is better than allowing an interface to accidentally run without
mac enforcement because there is no key.

If you want to relax the ordering in the config file, you could defer
the check until you've read the whole thing, which would mean the
ordering constraint only exists on dynamic reconfig; but not sure that's
even necessary, forcing people to put the key definition first is not
that onerous...

-Toke

---

From: Juliusz Chroboczek <[email-masked]>
Date: Mon, 28 Sep 2020 17:23:07 +0200

> Ah, good to know!

It's also not a very good idea.

MAC exposes large numbers of packets and their signatures to an attacker.
It is therefore vulnerable to dictionary attacks.  HMAC keys should either
be drawn randomly using a cryptographic RNG, or generated using a hashing
procedure that is resistant to dictionary attacks.  This is expanded upon
in the Security Considerations section of the draft.

I am of a divided mind on whether babeld should implement the RFC 2104
procedure, as proposed by Antonin.  On the one hand, the procedure in
RFC 2104 Section 2 is extremely naive from a modern point of view, and
implementing it might induce people into deploying MAC in an insecure
manner.  On the other hand, it is convenient, and inconvenient security
protocols are less likely to be deployed in production (*cough*PGP*cough).

On the gripping hand, MAC only secures Babel traffic -- it does nothing to
protect neighbour discovery, let alone data traffic.  One might therefore
argue that Babel-MAC and Babel-DTLS are both pointless, and that Babel
should be deployed over secure links.  There is some merit to this position,
and we should consider doing some work on integrating babeld with Wireguard.
Please see the discussion on

  https://github.com/jech/babeld/issues/58

-- Juliusz

---

From: Juliusz Chroboczek <[email-masked]>
Date: Mon, 28 Sep 2020 17:30:59 +0200

> interface bring-up or reconfig, as appropriate depending on context).

Suppose you were running Babel together with a keying daemon.  Say, one
that periodically performs an authenticated supersingular isogeny
Diffie-Helman exchange and then feeds the resulting key to the Babel
daemon.

You could of course delay starting the Babel daemon until you got yourself
a non-empty set of keys, but wouldn't it be more robust to start Babel in
authenticated mode with no keys (which would cause it to drop packets) and
then incrementally feed it keys as they are learned?

-- Juliusz

---

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Mon, 28 Sep 2020 17:39:05 +0200

> in the Security Considerations section of the draft.

Yeah, that discussion is why I added the restriction in Bird.

> protocols are less likely to be deployed in production (*cough*PGP*cough).

In any case, Blake2 does not have any such mechanism, so keys would have
to be a certain size there (or we'd end up inventing something new,
which is probably not a good idea). So I think restricting the key size
is fine; and, well, if users want to make that key be all zeroes (or
something equally insecure) we can't really stop that anyway...

>   https://github.com/jech/babeld/issues/58

Ah cool, I've been wanting to add wg support to Bird as well; maybe we
should write a spec for how that is supposed to work with the Babel
protocol (I don't agree with the reporter of that bug that this will
need changes to wireguard, if we allow plaintext discovery outside of
the tunnel, babel can just to unicast peers on the inside).

-Toke

---

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Mon, 28 Sep 2020 17:45:45 +0200

> then incrementally feed it keys as they are learned?

Hmm, not sure I have any opinion about which would be more robust off
the top of my head. But I can see your point that someone might
implement it that way; and I suppose I could be convinced that such a
configuration could be allowed, as long as it fails safe, of course. I
think that at least emitting a clear warning on startup would help users
avoid the most common configuration errors, though...

-Toke