Support libcrypto for hmac and sha256

[tobhe]

In a recent security review for the Ubuntu package of dhcpcd we found that the packaged version uses the built-in crypto primitives from compat/crypt.

We try to keep the number of low-level crypto implementations at a minimum to make it easier for the Ubuntu security team to review them, track eventual bugs as and ensure compliance in certified environments.

This PR adds alternative compat/crypt wrappers based on libcrypto and some tests to make sure they work correctly. I made sure that they also work across different OpenSSL versions and with LibreSSL.

[tobhe]

Uploaded a new version with a few fixes. It should now respect ALLOW_USR_LIBS and only pull in libcrypto if a prefix is configured. I reshuffled things a bit in configure to make sure --with-openssl works as intended and fixed some whitespaces.

[tobhe]

Pushed another update based on the latest feedback. The prioritization in configure is tricky to get right but I think the current version does a good job in doing what you requested. When anything other than openssl is available it will use that first, unless --with-openssl is explicitly passed as an argument.

I also moved the openssl shims to compat/crypt_openssl to leave the NetBSD files untouched.

I have tested on Ubuntu and OpenBSD, as mentioned above I would appreciate feedback on the DragonflyBSD diff. I left the workaround for MD5 in place but removed the "openssl/sha.h" part since that should now be handled automatically :)

[tobhe]

Newest iteration uses pkg-config to find libcrypto and hopefully fixed the if else jungle.

[perkelix]

Does dhcpcd even need to ship its own implementations in compat? It would make more sense to always build against upstream libraries that see active maintenance.

[rsmarples]

There are several OS that exist where nothing is used in compat as these functions exist within libc.

I don't even know if all these functions exist in any libraries available in Linux so it makes more sense to do it like this because as I said earlier some systems expect /usr to be network mounted after dhcpcd has started so just blindly using openssl isn't an option as it generally lives in /usr.

[borkra]

A lot of the functions you have in compat are a part of libbsd in Linux, I am not sure making libbsd a requirement is a good thing, since it's not a required install.

[perkelix]

Which would be this library: https://libbsd.freedesktop.org/

[borkra]

Making OpenSSL a requirement, is not a good thing. Big distros like Ubuntu have all crypto libraries installed, but if you look on Embedded use, things are much more fragmented. There are OpenSSL, GnuTLS, wolfSSL and even "No SSL" builds... Are you going to support them all? OpenSSL is huge I wouldn't want to be required to include it.

[perkelix]

This can currently build using the minimal libmd instead of the built-ins shipped in compat/crypto.

My guess is that Ubuntu would just rather use OpenSSL for all crypto. As long as this is configurable at build time and doesn't remove the possibility to use libmd or built-ins for other distros, we're good.

[tobhe]

Making OpenSSL a requirement, is not a good thing

I am not planning to limit the portability of dhcpcd in any way or even change the default. We would just like the be able to use OpenSSL where it is already available.

My guess is that Ubuntu would just rather use OpenSSL for all crypto

Right, our main motivation is having a small set of well reviewed cryptography in our base system. OpenSSL just happens to be the most widespread one and matches that criteria.

[borkra]

Looking at libbsd on Linux, in detail, it includes libmd with all SHA functions, maybe we just replace compat with libbsd on Linux.

[tobhe]

Uploaded one more tiny fix to not have HAVE_MD5_H defined when using openssl.

I think the rest of the compat code is fine as it is mostly thin wrappers around existing libc api, some have even found their way into glibc like arc4random.
My plan is to make sure all of them are up to date with their respective *BSD versions and if not open a separate PR to sync them.

[tobhe]

I am sure the config refactoring wasn't easy to follow, so here's what the current version should default to:

• If any of SHA2 or HMAC are available directly, use those.
• If not check if OpenSSL libcrypto is available and use it for hmac via compat/crypt_openssl/hmac.c.
  With OpenSSL we don't need MD5 because that is only used in compat/hmac.c.
  • If libcrypto implements SHA2 directly (older versions and LibreSSL) we use that
  • Otherwise we use the new compat/crypt_openssl/sha2.c which implments the SHA2 interface via the higher level EVP_* API.
• If neither SHA/HMAC nor OpenSSL are available use compat/crypt/*

Additionally, if --with-openssl is passed explicitly, we always try to use OpenSSL.

[tobhe]

Any advice on how we could move forward with this? Would it make more sense to not change the defaults for now and only enable openssl support if --with-openssl is explicitly passed as an argument?

[perkelix]

@rsmarples have we reached a conclusion about this (and other pending) merge requests?

[perkelix]

@ido Any feedback?

[ido]

@ido Any feedback?

@perkelix will take a look this weekend.

[tobhe]

One more update. I added an explicit call to OPENSSL_init_crypto() early during daemon startup because some versions of openssl do this lazily on first use of the API which clashes with seccomp. I also updated the configure test to probe for the same API.
Since this is a one-time setup action this seems like the better way rather than explicitly allowing the syscalls needed for this later.

[tobhe]

I made another interesting discovery today which is that even if openssl doesn't expose SHA256_* it might still use the symbols internally which leads to some funny effects. I think the same can happen if libmd and libcrypto coexist in the same address space. Because SHA256_Init exists twice in the address space OpenSSL's internal EVP_* logic might actually call into the libmd version which might or might not work.
I think we best work around this by prefixing the compat/crypt_openssl names with dhcpcd_ and then map them with defines in the header. For libmd and openssl we should make sure that they can't be used at the same time.

[rsmarples]

That's always the danger of linking which I try and avoid in the first place :)

I'm on holiday next week and without internet and will try and look at this some more when I get back.

[perkelix]

I'm on holiday next week and without internet and will try and look at this some more when I get back.

@rsmarples, is there any progress on this?

[rsmarples]

@perkelix sorry for the late reply, my personal life has been interesting of late.

@tobhe I've been hammering this PR on a lot of various OS's can cannot find any issue with it, so from the technical perspective it's good to merge and well done for getting here!

I'll spend some time having a final look over the weekend to refresh and then merge it.