Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add socket opts for zeromq 4.x security mechanisms #278

Merged
merged 4 commits into from

3 participants

@msealand

No description provided.

@msealand

Re: failing travis build; see @kkoopa's comment on #277

@kkoopa

Looks good at a first glance. However, it would be really good if you could add some tests of the actual functionality that setting these options is supposed to provide. The main reason is that certain things that work in zeromq do not work in zeromq.node. Some of these can be fixed, others are just incompatible. Tests make it far easier to determine if something is working as intended or not and whether new changes cause stuff to break or not.

@msealand

Yeah, sorry, I should have had tests in there from the beginning; I've added some now. They don't cover everything by any means, but they do at least show that setting the options works as intended. Unlike the other tests, these use tcp sockets instead of inproc because the security mechanisms only work over tcp.

I also had to write a ZAP handler to make the tests work, but I just made it authenticate any valid request. I'm assuming that's ok since the point wasn't really to test the security mechanism themselves but, rather, that setting the options enabled the appropriate mechanism on the socket. I've started work on a more full featured ZAP handler here, if anyone is interested.

@kkoopa

Yes, the dummy handler is probably ok. The idea is just to test that the binding binds correctly and that everything can be used somehow.

test/test.socket.zap.curve.js fails with 4.0.3 according to Travis

@msealand

Hmm, so it does. On first glance it looks like test/test.scoket.zap.plain.js passes so I suspect it's because libsodium isn't available. I'll look in to it a little more and see if I can clean that up.

@kkoopa

Something is wrong with the options declaration you added to the binding. Maybe types, I think all of them were added as binary. No, that's not the issue as such. I got the same thing locally. Don't think I have libsodium installed either.

@msealand

It looks like the ZMQ_CURVE_* options are supposed to fail with EINVAL if libsodium isn't installed: libzmq - options.cpp
So, I think things are still working as they should, it's just a matter of getting libsodium working in the travis build (preferably only for zmq 4.x I'd guess). I don't have much experience with travis configuration, but I can give that a shot.

@kkoopa

Ah, I didn't realize it needed additional libraries. It should just be a matter of adding some lines to install them in the before_install step. Possibly conditional on ZMQ4, if that is possible to specify in travis. I don't think it is. Go ahead and give it a shot, it should be similar to the setup for libzmq that is already being done. http://docs.travis-ci.com/user/build-configuration/

@msealand

Adding libsodium to the travis build configuration fixed test/test.socket.zap.curve.js. It looks like the only reason the build is failing now is due to node 0.11.11 issues.

@kkoopa

Great job! I see you even got it to only install libsodium when used.

@msealand

Thanks!

@ronkorving
Collaborator

While travisci discourages it, it seems we could peg Node 0.11 to a specific version (if available). Perhaps we should give that a try?

http://docs.travis-ci.com/user/languages/javascript-with-nodejs/

@kkoopa

It should work until they remove the older version. Don't know when they do that.
It is also possible to install nodejs yourself in the pre_install part.
On the other hand, I suspect 0.11.12 to come out within a week or two, based on recent time between releases.

@msealand

I just tried this on a separate branch on my fork, but it didn't work. Travis complained that 0.11.10 wasn't available and fell back to node 0.10.25. So, the tests did pass, but it wasn't using the correct node version.

From the build log:

$ nvm use 0.11.10
N/A version is not installed
v0.10.18  v0.10.25  v0.11.11  v0.6.21  v0.8.23  v0.8.25  v0.8.26
...
$ node --version
v0.10.25
@ronkorving
Collaborator

Ah, so they already removed it? That's too bad. Thanks for trying that out.

@ronkorving
Collaborator

One question regarding libsodium. Does this mean that if I use zmq 4, and I don't install libsodium, I have a broken setup? (or in the very least broken unit tests?)

@msealand

zmq 4.x is perfectly usable without libsodium, the only thing that won't work is the curve authentication/encryption stuff; if you try to use any of the socket options related to curve, it'll throw an EINVAL. That does mean that the test/test.socket.zap.curve.js unit test will fail without libsodium installed though (at least with the current setup).

@ronkorving
Collaborator

If it's limited to unit tests, I can't say I really mind it. If there's any way to test for it though, we could at least first assert that libsodium is there (I see a race condition in my reasoning though).

@msealand

I suppose we could put something like this in test/test.socket.zap.curve.js:

try {
  rep.curve_server = 0;
} catch(e) {
  console.log("libsoduim seems to be missing; skipping curve test");
  process.exit();
}

The idea being that if we can't set curve_server to 0 (its default value), then that must be because libsodium isn't installed, so we should skip the curve test. Of course, that assumes that the curve_server property is working properly in the first place, which slightly negates the point of the test. The test does quite a bit more than just test that property though, so maybe it's worth it.

I did dig around in the libzmq code a bit and I didn't see anything in there that would indicate that libsodium was or wasn't installed other than returning EINVAL when trying to set a socket option that depends on it.

@ronkorving
Collaborator

What you just explained, is what I considered the race condition in my reasoning :) But I think this could be worth it. Mind adding that condition?

@kkoopa

There isn't a #define somewhere that tells if the code path is enabled or not? Or in libsodium (maybe the .h file has a suitable include guard?) that the binding could detect and expose a symbol on the js side like zmq.ZMQ_HAS_ZAP which the js-side of the binding could then test against?

@msealand

libzmq has a #define HAVE_LIBSODIUM, but it doesn't seem to be in a public header anywhere that we can use. I hadn't thought about looking in the libsodium headers, that's a good idea.

Re: The symbol on the js side, I think it should be more like zmq.ZMQ_HAS_CURVE since the rest of the ZAP functionality (NULL and PLAIN mechanisms) will work fine without libsodium.

I'll see if I can find something in the libsodium headers and get that working. If not, I can fall back to adding the try/catch per @ronkorving.

@msealand

The libsodium headers aren't included in the zmq headers (which I think makes sense since then HAVE_LIBSODIUM would have to have been accessible in there to know that the libsodium headers were even available). I can't just include the libsodium header in binding.cc since the include might not exist and then we'll get a compile error. I guess we could setup some sort of autoconf thing to detect libsodium and then add a -DHAVE_LIBSODIUM to the compiler flags, but I don't know enough about node-gyp to know if that's even possible, and I'm not sure that it's really worth it.

I don't think I'm missing anything but, honestly, I feel a little rusty in this area of c/c++, so I could be :) Do you guys have any other ideas before I fall back to using the try/catch check?

@kkoopa

I'm afraid I'm out of ideas. Make the try/catch and it will be good. Perhaps upstream can be convinced to include a define in a normal header at some future point. They really should have done this already.

@ronkorving
Collaborator

Thanks guys! Awesome job.

@ronkorving ronkorving merged commit 26f790b into JustinTulloss:master

1 check failed

Details default The Travis CI build could not complete due to an error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 8, 2014
  1. @msealand
Commits on Feb 10, 2014
  1. @msealand
Commits on Feb 11, 2014
  1. @msealand
Commits on Feb 13, 2014
  1. @msealand
This page is out of date. Refresh to see the latest.
View
11 .travis.yml
@@ -2,9 +2,18 @@ env:
- ZMQ="git://github.com/zeromq/zeromq2-x.git"
- ZMQ="git://github.com/zeromq/zeromq3-x.git -b v3.1.0"
- ZMQ="git://github.com/zeromq/zeromq3-x.git -b v3.2.4"
- - ZMQ="git://github.com/zeromq/zeromq4-x.git -b v4.0.3"
+ - ZMQ="git://github.com/zeromq/zeromq4-x.git -b v4.0.3" SODIUM="git://github.com/jedisct1/libsodium.git -b 0.4.5"
before_install:
- sudo apt-get install uuid-dev
+ - if [ -n "$SODIUM" ]; then
+ - git clone --depth 1 $SODIUM libsodium
+ - cd libsodium
+ - ./autogen.sh
+ - ./configure
+ - make
+ - sudo make install
+ - cd ../
+ - fi
- git clone --depth 1 $ZMQ zmqlib
- cd zmqlib
- ./autogen.sh
View
12 binding.cc
@@ -1133,6 +1133,18 @@ namespace zmq {
opts_uint64.insert(12); // ZMQ_RCVBUF 2.x uint64_t
#endif
+ #if ZMQ_VERSION_MAJOR >= 4
+ opts_int.insert(43); // ZMQ_MECHANISM
+ opts_int.insert(44); // ZMQ_PLAIN_SERVER
+ opts_binary.insert(45); // ZMQ_PLAIN_USERNAME
+ opts_binary.insert(46); // ZMQ_PLAIN_PASSWORD
+ opts_int.insert(47); // ZMQ_CURVE_SERVER
+ opts_binary.insert(48); // ZMQ_CURVE_PUBLICKEY
+ opts_binary.insert(49); // ZMQ_CURVE_SECRETKEY
+ opts_binary.insert(50); // ZMQ_CURVE_SERVERKEY
+ opts_binary.insert(55); // ZMQ_ZAP_DOMAIN
+ #endif
+
NODE_DEFINE_CONSTANT(target, ZMQ_CAN_DISCONNECT);
NODE_DEFINE_CONSTANT(target, ZMQ_CAN_UNBIND);
NODE_DEFINE_CONSTANT(target, ZMQ_CAN_MONITOR);
View
18 lib/index.js
@@ -79,6 +79,15 @@ var longOptions = {
, ZMQ_XPUB_VERBOSE: 40
, ZMQ_ROUTER_RAW: 41
, ZMQ_IPV6: 42
+ , ZMQ_MECHANISM: 43
+ , ZMQ_PLAIN_SERVER: 44
+ , ZMQ_PLAIN_USERNAME: 45
+ , ZMQ_PLAIN_PASSWORD: 46
+ , ZMQ_CURVE_SERVER: 47
+ , ZMQ_CURVE_PUBLICKEY: 48
+ , ZMQ_CURVE_SECRETKEY: 49
+ , ZMQ_CURVE_SERVERKEY: 50
+ , ZMQ_ZAP_DOMAIN: 55
};
Object.keys(longOptions).forEach(function(name){
@@ -113,6 +122,15 @@ var opts = exports.options = {
, recovery_ivl: zmq.ZMQ_RECOVERY_IVL
, sndbuf: zmq.ZMQ_SNDBUF
, swap: zmq.ZMQ_SWAP
+ , mechanism: zmq.ZMQ_MECHANISM
+ , plain_server: zmq.ZMQ_PLAIN_SERVER
+ , plain_username: zmq.ZMQ_PLAIN_USERNAME
+ , plain_password: zmq.ZMQ_PLAIN_PASSWORD
+ , curve_server: zmq.ZMQ_CURVE_SERVER
+ , curve_publickey: zmq.ZMQ_CURVE_PUBLICKEY
+ , curve_secretkey: zmq.ZMQ_CURVE_SECRETKEY
+ , curve_serverkey: zmq.ZMQ_CURVE_SERVERKEY
+ , zap_domain: zmq.ZMQ_ZAP_DOMAIN
};
/**
View
59 test/test.socket.zap.curve.js
@@ -0,0 +1,59 @@
+var should = require('should')
+ , semver = require('semver')
+ , zmq = require('../');
+
+if (semver.gte(zmq.version, '4.0.0')) {
+ var zap = require('./zap')
+ , port = 'tcp://127.0.0.1:12347'
+ , zapSocket = zap.start()
+ , rep = zmq.socket('rep')
+ , req = zmq.socket('req');
+
+ try {
+ rep.curve_server = 0;
+ } catch(e) {
+ console.log("libsoduim seems to be missing; skipping curve test");
+ process.exit(0);
+ }
+
+ var serverPublicKey = new Buffer('7f188e5244b02bf497b86de417515cf4d4053ce4eb977aee91a55354655ec33a', 'hex')
+ , serverPrivateKey = new Buffer('1f5d3873472f95e11f4723d858aaf0919ab1fb402cb3097742c606e61dd0d7d8', 'hex')
+ , clientPublicKey = new Buffer('ea1cc8bd7c8af65497d43fc21dbec6560c5e7b61bcfdcbd2b0dfacf0b4c38d45', 'hex')
+ , clientPrivateKey = new Buffer('83f99afacfab052406e5f421612568034e85f4c8182a1c92671e83dca669d31d', 'hex');
+
+ rep.on('message', function(msg){
+ msg.should.be.an.instanceof(Buffer);
+ msg.toString().should.equal('hello');
+ rep.send('world');
+ });
+
+ rep.zap_domain = "test";
+ rep.curve_server = 1;
+ rep.curve_secretkey = serverPrivateKey;
+ rep.mechanism.should.eql(2);
+
+ var timeout = setTimeout(function() {
+ req.close();
+ rep.close();
+ zapSocket.close();
+ throw new Error("Request timed out");
+ }, 1000);
+
+ rep.bind(port, function(){
+ req.curve_serverkey = serverPublicKey;
+ req.curve_publickey = clientPublicKey;
+ req.curve_secretkey = clientPrivateKey;
+ req.mechanism.should.eql(2);
+
+ req.connect(port);
+ req.send('hello');
+ req.on('message', function(msg){
+ msg.should.be.an.instanceof(Buffer);
+ msg.toString().should.equal('world');
+ clearTimeout(timeout);
+ req.close();
+ rep.close();
+ zapSocket.close();
+ });
+ });
+}
View
41 test/test.socket.zap.null.js
@@ -0,0 +1,41 @@
+var should = require('should')
+ , semver = require('semver')
+ , zmq = require('../');
+
+if (semver.gte(zmq.version, '4.0.0')) {
+ var zap = require('./zap')
+ , port = 'tcp://127.0.0.1:12345'
+ , zapSocket = zap.start()
+ , rep = zmq.socket('rep')
+ , req = zmq.socket('req');
+
+ rep.on('message', function(msg){
+ msg.should.be.an.instanceof(Buffer);
+ msg.toString().should.equal('hello');
+ rep.send('world');
+ });
+
+ rep.zap_domain = "test";
+ rep.mechanism.should.eql(0);
+
+ var timeout = setTimeout(function() {
+ req.close();
+ rep.close();
+ zapSocket.close();
+ throw new Error("Request timed out");
+ }, 1000);
+
+ rep.bind(port, function(){
+ req.mechanism.should.eql(0);
+ req.connect(port);
+ req.send('hello');
+ req.on('message', function(msg){
+ msg.should.be.an.instanceof(Buffer);
+ msg.toString().should.equal('world');
+ clearTimeout(timeout);
+ req.close();
+ rep.close();
+ zapSocket.close();
+ });
+ });
+}
View
45 test/test.socket.zap.plain.js
@@ -0,0 +1,45 @@
+var should = require('should')
+ , semver = require('semver')
+ , zmq = require('../');
+
+if (semver.gte(zmq.version, '4.0.0')) {
+ var zap = require('./zap')
+ , port = 'tcp://127.0.0.1:12346'
+ , zapSocket = zap.start()
+ , rep = zmq.socket('rep')
+ , req = zmq.socket('req');
+
+ rep.on('message', function(msg){
+ msg.should.be.an.instanceof(Buffer);
+ msg.toString().should.equal('hello');
+ rep.send('world');
+ });
+
+ rep.zap_domain = "test";
+ rep.plain_server = 1;
+ rep.mechanism.should.eql(1);
+
+ var timeout = setTimeout(function() {
+ req.close();
+ rep.close();
+ zapSocket.close();
+ throw new Error("Request timed out");
+ }, 1000);
+
+ rep.bind(port, function(){
+ req.plain_username = "user";
+ req.plain_password = "pass";
+ req.mechanism.should.eql(1);
+
+ req.connect(port);
+ req.send('hello');
+ req.on('message', function(msg){
+ msg.should.be.an.instanceof(Buffer);
+ msg.toString().should.equal('world');
+ clearTimeout(timeout);
+ req.close();
+ rep.close();
+ zapSocket.close();
+ });
+ });
+}
View
46 test/zap.js
@@ -0,0 +1,46 @@
+// This is mainly for testing that the security mechanisms themselves are working
+// not the ZAP protocol itself. As long as the request is valid, this will
+// authenticate it.
+
+var zmq = require('../');
+
+module.exports.start = function() {
+ var zap = zmq.socket('router');
+ zap.on('message', function() {
+ var data = Array.prototype.slice.call(arguments);
+
+ if (!data || !data.length) throw new Error("Invalid ZAP request");
+
+ var returnPath = [],
+ frame = data.shift();
+ while (frame && (frame.length != 0)) {
+ returnPath.push(frame);
+ frame = data.shift();
+ }
+ returnPath.push(frame);
+
+ if (data.length < 6) throw new Error("Invalid ZAP request");
+
+ var zapReq = {
+ version: data.shift(),
+ requestId: data.shift(),
+ domain: new Buffer(data.shift()).toString('utf8'),
+ address: new Buffer(data.shift()).toString('utf8'),
+ identity: new Buffer(data.shift()).toString('utf8'),
+ mechanism: new Buffer(data.shift()).toString('utf8'),
+ credentials: data.slice(0)
+ };
+
+ zap.send(returnPath.concat([
+ zapReq.version,
+ zapReq.requestId,
+ new Buffer("200", "utf8"),
+ new Buffer("OK", "utf8"),
+ new Buffer(0),
+ new Buffer(0)
+ ]));
+ });
+
+ zap.bindSync("inproc://zeromq.zap.01");
+ return zap;
+}
Something went wrong with that request. Please try again.