build: use official Findlibsodium + add option to download and build libsodium

[aminya]

• Updates Findlibsodium to the official module provided by libsodium itself

• This adds an option for libsodium to download and build it from the source and respect the options of libzmq. I verified the built with WITH_LIBSODIUM_STATIC set to both ON and OFF.

It uses libsodium-cmake with CPM for the building libsodium.

Fixes #4484
Closes zeromq/zeromq.js#554
Closes zeromq/zeromq.js#553
Related to zeromq/zeromq.js#529

[Bartel-C8]

The failing CI build is a setup quirk? (As e.g. this commit on master also failed: https://github.com/zeromq/libzmq/actions/runs/4671679606/jobs/8272961115)

[bluca]

This adds a fallback for libsodium to build it from the source and respect the options of libzmq. I verified the built with WITH_LIBSODIUM_STATIC set to both ON and OFF.

Sorry, but no, this is broken by design. We are not going to embed a security critical library. If you use it, you must get a sensible maintenance and deployment model, supported by a security team.

[aminya]

This PR doesn't change the behaviour. Everything is opt-in determined by the user of libzmq.

I can argue that the current build system has more security issues. As it is almost impossible to get libzmq to build statically with libsodium with the master's CMake. See #4484 for context. People have suggested not using libsodium altogether because of this!
zeromq/zeromq.js#554

[bluca]

The wrong part is not the static linking, is downloading and embedding the sources. The built library needs to be provided by an external, managed, maintained source.

[bluca]

so to be extra clear, if you want to improve static linking in cmake, by all means update the PR to do that, but no integration with downloading and building external sources, if people want to shoot themselves in the foot and get pwned by vendoring random versions of cryptographic primitives, they'll have to do it themselves, we will have no part in facilitating that

[Bartel-C8]

What would be the difference in downloading libsodium yourself, make some malicious changes and link your libzmq build statically against that?

Could we do something like hash checking on the download files, to be a bit more sure?
Like: https://stackoverflow.com/questions/41667988/cmake-check-hash-md5-sha256-for-downloaded-file
Or like libsodium suggests, use minisig for integrity checking: https://doc.libsodium.org/installation#integrity-checking ?

The "do not compile libsodium yourself" is a bit harsh in my opinion, as like said, a capable person with bad intentions could do it either way easily. And, given the fact that libsodium itself doesn't even provide binaries for Linux, and suggests to build it from source? : https://doc.libsodium.org/installation#compilation-on-unix-like-system

@aminya for zeromq.js , it could be also possible to default build without Curve (libsodium/NaCl), and when you do need it, there is a guide on how to statically link it.

Anyway, if the PR is not accepted with building from source, it is a great PR anyway without it for linking statically. Such that the prebuilts for zeromq.js have an official statically linked libsodium packaged.

[bluca]

What would be the difference in downloading libsodium yourself, make some malicious changes and link your libzmq build statically against that?

The difference is that it's someone else who does that, so they will have the blame when it inevitably leads to the expected catastrophic results, without any endorsement from this library. Linux distributions exist for a reason, use them.

[aminya]

To address the concerns, added an opt-in option for downloading via CPM.

Additionally, forked libsodium-cmake into the @zeromq organization. The commit used in CPM is now hashed via Sha256 so it will not change.
https://github.com/zeromq/libsodium-cmake/

Also, since Libsodium merged my patch for FindLibsodium, replaced the CMake module with the upstream one. So it is identical:
https://github.com/jedisct1/libsodium/blob/a1348978e6fd4df033f4a0a719c1aa196d18c842/contrib/Findsodium.cmake#L1

[bluca]

To address the concerns, added an opt-in option for downloading via CPM.

Not good enough, please remove it completely. It is not acceptable in any way, whether it is optional or not.

[bluca]

Additionally, forked libsodium-cmake into the @zeromq organization. The commit used in CPM is now hashed via Sha256 so it will not change. https://github.com/zeromq/libsodium-cmake/

Before doing something like that, next time consult with the project please. This is not acceptable. I've deleted it now. I cannot possibly be any clearer: we are not going to maintain forks of cryptographic primitives anywhere here

[aminya]

This raises some concerns for me. Maybe, @zeromq/core @zeromq/zeromq-js can clarify if as the maintainer of zeromq.js, I am allowed to fork a repository into the organization or not. Even if you do not want to merge this to zeromq, you may not delete the repository I created.

[bluca]

This raises some concerns for me. Maybe, @zeromq/core @zeromq/zeromq-js can clarify if as the maintainer of zeromq.js, I am allowed to fork a repository into the organization or not. Even if you do not want to merge this to zeromq, you may not delete the repository I created.

You can add repositories to the organizations when it is sensible to do so, and most of the times it is, as you can see from the list of repositories that are part of it.

Dumping an unmantainable fork of cryptographic primitives is NOT one of those occasions, especially as a lazy shortcut. Please do not do that. We take security seriously here.

[aminya]

This definition of security doesn't seem to align with what others think. I just added the fork as an additional layer of security to lock up any changes because you requested the change.

Otherwise, other security-conscious repositories in this organization like zmq.rs are very happy to use package managers.

https://github.com/zeromq/zmq.rs/blob/7baeeffde9e4cb9741d1841cfdee5f00f354b578/Cargo.toml#L18

[bluca]

This definition of security doesn't seem to align with what others think. I just added the fork as an additional layer of security to lock up any changes because you requested the change.

Not only I have made no such request, I have only been abundantly clear: there shall be no embedded security critical library here. There is no cryptographer, nor a security team available 24/7 on-call around here. Leave maintenance of cryptography primitives to those who understand them and have the resources to do so.

[bluca]

To address the concerns, added an opt-in option for downloading via CPM.

Not good enough, please remove it completely. It is not acceptable in any way, whether it is optional or not.

Not addressed in many months, closing