[PATCH 0/4] Add MAC authentication support to the Babel protocol

[Toke Høiland-Jørgensen]

Ondrej Zajicek <santiago at crfreenet.org> writes:

> On Sun, Feb 23, 2020 at 11:56:33PM +0100, Toke Høiland-Jørgensen wrote:
>> This series adds MAC authentication support to the Babel protocol as specified
>> in by the IETF Babel working group in draft-babel-hmac-10:
>
> Hi
>
> Some more comments / questions:
>
>
> 1/4:
>
> BIRD_CHECK_GETRANDOM_SYSCALL - direct syscall case seems unnecessary,
> as we can fallback to /dev/urandom anyways.
>
> BIRD_CHECK_GETRANDOM - just use generic AC_CHECK_FUNCS /
> AC_SEARCH_LIBS ?

OK.

> I think that random_bytes() should not fail.

Preferably not; but we don't really have any guarantees that the syscall
will succeed, do we? I guess I can add some sanity checks on startup and
bail out if (e.g.) /dev/urandom cannot be opened. It would still be
possible for read() or getrandom() to fail later on, though, no?

> 2/4:
>
> blake2 - We definitely need unit tests here. Ideally there should exist some
> reference data / hash pairs for blake2. See mac_test.c

Yup, there does seem to be some test vectors in the blake2 repository;
will add those.

> There are '#if defined(NATIVE_LITTLE_ENDIAN)' in the code, does
> anybody define these?

Hmm, probably not? The FreeBSD Blake implementation seems to have a
#define based on __BYTE_ORDER, so guess we could just add something like
that as well?

> 3/4:
>
> What is point of separating babel_parse_state and babel_read_state?
>
> Why export packet/TLV structures from packets.c?

Well, I did both of these to be able to have all the auth-related code
in a separate file, while still reusing the packet parsing macros etc...

> General pattern in BIRD (including Babel) is that wire format details
> is hidden in packets.c and more abstract structures are exported
> outside (e.g. union babel_msg). Seems to me that it would make sense
> to have low-level auth code (TLV read/write code, packet
> signing/verifying) directly in packets.c, while high-level code
> (challenge response mechanism) in babel.c.

Hmm, I could have sworn that I got the idea of splitting it into its own
file from one of the other protocols, but looking at them now that does
not actually seem to be the case. So I guess I'll just move everything
back into {packets,babel}.c :)

-Toke