zeromq / zeromq.js

:zap: Node.js bindings to the ØMQ library
http://zeromq.github.io/zeromq.js/
MIT License
1.45k stars 210 forks source link

zeromq links dynamically to libsodium on macOS #557

Open kevinushey opened 1 year ago

kevinushey commented 1 year ago

For example, I see:

kevin@MBP-P2MQ:~/.nvm/versions/node/v16.14.0/lib/node_modules/zeromq [(no branch)]
$ otool -L prebuilds/darwin-arm64/node.napi.glibc.node
prebuilds/darwin-arm64/node.napi.glibc.node:
        /Users/runner/arm-target/opt/libsodium/lib/libsodium.23.dylib (compatibility version 27.0.0, current version 27.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1200.3.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

I believe this occurs because zeromq tries to link to libsodium on macOS here:

https://github.com/zeromq/zeromq.js/blob/47bb35c1941cb4fa8b510fb7da4d40b37cfa2e5f/binding.gyp#L85

But this gives the following linker flags:

$ pkg-config libsodium --libs
-L/opt/homebrew/Cellar/libsodium/1.0.18_1/lib -lsodium

And the above folder has:

$ ll /opt/homebrew/Cellar/libsodium/1.0.18_1/lib
total 1176
drwxr-xr-x   6 kevin  admin   192B Feb  9 15:47 ./
drwxr-xr-x  10 kevin  admin   320B Feb  6 16:15 ../
-r--r--r--   1 kevin  admin   247K Feb  6 16:15 libsodium.23.dylib
-r--r--r--   1 kevin  admin   340K May 30  2019 libsodium.a
lrwxr-xr-x   1 kevin  admin    18B May 30  2019 libsodium.dylib@ -> libsodium.23.dylib
drwxr-xr-x   3 kevin  admin    96B Feb  6 16:15 pkgconfig/

Because both dynamic and static libraries are available here, the dynamic library is preferred and used during the link step instead.

If the intention here is to statically link to libsodium (I think it is?) then we can just provide the path to the static library directly with something like:

'<!@(pkg-config libsodium --variable=libdir)/libsodium.a'

Would you accept a PR making this change?

parker-codes commented 1 year ago

I'm having the same issue on macOS.

aminya commented 1 year ago

This is not fixable in this organization https://github.com/zeromq/zeromq.js/issues/576

parker-codes commented 1 year ago

This is not fixable in this organization https://github.com/zeromq/zeromq.js/issues/576

Yeah, I read that. I'm sorry to see this happen to you. I understand the security implications mentioned, but I disagree with how the governance worked.

Best of luck!

Bartel-C8 commented 1 year ago

Still in favour of merging this, so stripping out libsodium alltogether : https://github.com/zeromq/zeromq.js/pull/554

With maybe the addition to have an install option like --zmq-draft , e.g. --zmq-curve in which the end-user is responsible themselves for rebuilding and packaging.

As, a quick GitHub search on public repo's didn't expose one repo using the curve mechanism... Only found this one: https://github.com/tykntech/indy-zmq-lib/blob/master/src/indy-zmq-lib.ts , which is using v5 compat either way and loading/linking libsodium with https://www.npmjs.com/package/libsodium-wrappers. Maybe this could be an option to this project too ? As opposed to to TweetNaCl.js

Don't know about other private users, but e.g. another big party using ZMQ in JS is not using CURVE either: https://github.com/microsoft/vscode-jupyter

Bartel-C8 commented 10 months ago

Can be closed as https://github.com/zeromq/zeromq.js/pull/554 is merged!

kevinushey commented 10 months ago

Great news :-)

aminya commented 10 months ago

Please keep this open as #554 is just a temporary fix.