Implement client-side StartTLS support

Author: pfmooney

lib/client/client.js

@@ -355,7 +355,7 @@ function Client(options) {
     });
   }
 
-  this.socket = null;
+  this._socket = null;
   this.connected = false;
   this.connect();
 }
@@ -792,6 +792,7 @@ Client.prototype.search = function search(base,
 
   var self = this;
   var baseDN = ensureDN(base, this.strictDN);
+
   function sendRequest(ctrls, emitter, cb) {
     var req = new SearchRequest({
       baseObject: baseDN,
@@ -834,7 +835,7 @@ Client.prototype.search = function search(base,
     pager.on('search', sendRequest);
     pager.begin();
   } else {
-    sendRequest(controls, new EventEmitter, callback);
+    sendRequest(controls, new EventEmitter(), callback);
   }
 };
 
@@ -859,14 +860,106 @@ Client.prototype.unbind = function unbind(callback) {
   // user-initiated unbind or something else.
   this.unbound = true;
 
-  if (!this.socket)
+  if (!this._socket)
     return callback();
 
   var req = new UnbindRequest();
   return this._send(req, 'unbind', null, callback);
 };
 
 
+/**
+ * Attempt to secure connection with StartTLS.
+ */
+Client.prototype.starttls = function starttls(options,
+                                              controls,
+                                              callback,
+                                              _bypass) {
+  assert.optionalObject(options);
+  options = options || {};
+  callback = once(callback);
+  var self = this;
+
+  if (this._starttls) {
+    return callback(new Error('STARTTLS already in progress or active'));
+  }
+
+  function onSend(err, emitter) {
+    if (err) {
+      callback(err);
+      return;
+    }
+    /*
+     * Now that the request has been sent, block all outgoing messages
+     * until an error is received or we successfully complete the setup.
+     */
+    // TODO: block traffic
+    self._starttls = {
+      started: true
+    };
+
+    emitter.on('error', function (err) {
+      self._starttls = null;
+      callback(err);
+    });
+    emitter.on('end', function (res) {
+      var sock = self._socket;
+      /*
+       * Unplumb socket data during SSL negotiation.
+       * This will prevent the LDAP parser from stumbling over the TLS
+       * handshake and raising a ruckus.
+       */
+      sock.removeAllListeners('data');
+
+      options.socket = sock;
+      var secure = tls.connect(options);
+      secure.once('secureConnect', function () {
+        /*
+         * Wire up 'data' and 'error' handlers like the normal socket.
+         * Handling 'end' events isn't necessary since the underlying socket
+         * will handle those.
+         */
+        secure.removeAllListeners('error');
+        secure.on('data', function onData(data) {
+          if (self.log.trace())
+            self.log.trace('data event: %s', util.inspect(data));
+
+          self._tracker.parser.write(data);
+        });
+        secure.on('error', function (err)  {
+          if (self.log.trace())
+            self.log.trace({err: err}, 'error event: %s', new Error().stack);
+
+          self.emit('error', err);
+          sock.destroy();
+        });
+        callback(null);
+      });
+      secure.once('error', function (err) {
+        // If the SSL negotiation failed, to back to plain mode.
+        self._starttls = null;
+        secure.removeAllListeners();
+        callback(err);
+      });
+      self._starttls.success = true;
+      self._socket = secure;
+    });
+  }
+
+  var req = new ExtendedRequest({
+    requestName: '1.3.6.1.4.1.1466.20037',
+    requestValue: null,
+    controls: controls
+  });
+
+  return this._send(req,
+      [errors.LDAP_SUCCESS],
+      new EventEmitter(),
+      onSend,
+      _bypass);
+};
+
+
 /**
  * Disconnect from the LDAP server and do not allow reconnection.
  *
@@ -889,8 +982,8 @@ Client.prototype.destroy = function destroy(err) {
   });
   if (this.connected) {
     this.unbind();
-  } else if (this.socket) {
-    this.socket.destroy();
+  } else if (this._socket) {
+    this._socket.destroy();
   }
   this.emit('destroy', err);
 };
@@ -906,6 +999,7 @@ Client.prototype.connect = function connect() {
   var self = this;
   var log = this.log;
   var socket;
+  var tracker;
 
   // Establish basic socket connection
   function connectSocket(cb) {
@@ -930,8 +1024,8 @@ Client.prototype.connect = function connect() {
         .removeAllListeners('connect')
         .removeAllListeners('secureConnect');
 
-      socket.ldap.id = nextClientId() + '__' + socket.ldap.id;
-      self.log = self.log.child({ldap_id: socket.ldap.id}, true);
+      tracker.id = nextClientId() + '__' + tracker.id;
+      self.log = self.log.child({ldap_id: tracker.id}, true);
 
       // Move on to client setup
       setupClient(cb);
@@ -953,7 +1047,7 @@ Client.prototype.connect = function connect() {
       self.connectTimer = setTimeout(function onConnectTimeout() {
         if (!socket || !socket.readable || !socket.writeable) {
           socket.destroy();
-          self.socket = null;
+          self._socket = null;
           onResult(new ConnectionError('connection timeout'));
         }
       }, self.connectTimeout);
@@ -962,7 +1056,7 @@ Client.prototype.connect = function connect() {
 
   // Initialize socket events and LDAP parser.
   function initSocket() {
-    socket.ldap = new MessageTracker({
+    tracker = new MessageTracker({
       id: self.url ? self.url.href : self.socketPath,
       parser: new Parser({log: log})
     });
@@ -979,13 +1073,13 @@ Client.prototype.connect = function connect() {
       if (log.trace())
         log.trace('data event: %s', util.inspect(data));
 
-      socket.ldap.parser.write(data);
+      tracker.parser.write(data);
     });
 
     // The "router"
-    socket.ldap.parser.on('message', function onMessage(message) {
-      message.connection = socket;
-      var callback = socket.ldap.fetch(message.messageID);
+    tracker.parser.on('message', function onMessage(message) {
+      message.connection = self._socket;
+      var callback = tracker.fetch(message.messageID);
 
       if (!callback) {
         log.error({message: message.json}, 'unsolicited message');
@@ -995,9 +1089,9 @@ Client.prototype.connect = function connect() {
       return callback(message);
     });
 
-    socket.ldap.parser.on('error', function onParseError(err) {
+    tracker.parser.on('error', function onParseError(err) {
       self.emit('error', new VError(err, 'Parser error for %s',
-            socket.ldap.id));
+            tracker.id));
       self.connected = false;
       socket.end();
     });
@@ -1018,7 +1112,8 @@ Client.prototype.connect = function connect() {
     socket.once('end', bail);
     socket.once('timeout', bail);
 
-    self.socket = socket;
+    self._socket = socket;
+    self._tracker = tracker;
 
     // Run any requested setup (such as automatically performing a bind) on
     // socket before signalling successful connection.
@@ -1152,14 +1247,15 @@ Client.prototype._flushQueue = function _flushQueue() {
  * Clean up socket/parser resources after socket close.
  */
 Client.prototype._onClose = function _onClose(had_err) {
-  var socket = this.socket;
+  var socket = this._socket;
+  var tracker = this._tracker;
   socket.removeAllListeners('connect')
     .removeAllListeners('data')
     .removeAllListeners('drain')
     .removeAllListeners('end')
     .removeAllListeners('error')
     .removeAllListeners('timeout');
-  this.socket = null;
+  this._socket = null;
   this.connected = false;
 
   ((socket.socket) ? socket.socket : socket).removeAllListeners('close');
@@ -1170,12 +1266,12 @@ Client.prototype._onClose = function _onClose(had_err) {
   this.emit('close', had_err);
   // On close we have to walk the outstanding messages and go invoke their
   // callback with an error.
-  socket.ldap.pending.forEach(function (msgid) {
-    var cb = socket.ldap.fetch(msgid);
-    socket.ldap.remove(msgid);
+  tracker.pending.forEach(function (msgid) {
+    var cb = tracker.fetch(msgid);
+    tracker.remove(msgid);
 
     if (socket.unbindMessageID !== parseInt(msgid, 10)) {
-      return cb(new ConnectionError(socket.ldap.id + ' closed'));
+      return cb(new ConnectionError(tracker.id + ' closed'));
     } else {
       // Unbinds will be communicated as a success since we're closed
       var unbind = new UnbindResponse({messageID: msgid});
@@ -1184,8 +1280,9 @@ Client.prototype._onClose = function _onClose(had_err) {
     }
   });
 
-  delete socket.ldap.parser;
-  delete socket.ldap;
+  // Trash any parser or starttls state
+  this._tracker = null;
+  delete this._starttls;
 
   // Automatically fire reconnect logic if the socket was closed for any reason
   // other than a user-initiated unbind.
@@ -1212,8 +1309,8 @@ Client.prototype._updateIdle = function _updateIdle(override) {
   var self = this;
   function isIdle(disable) {
     return ((disable !== true) &&
-      (self.socket && self.connected) &&
-      (self.socket.ldap.pending.length === 0));
+      (self._socket && self.connected) &&
+      (self._tracker.pending.length === 0));
   }
   if (isIdle(override)) {
     if (!this._idleTimer) {
@@ -1242,14 +1339,14 @@ Client.prototype._send = function _send(message,
                                         _bypass) {
   assert.ok(message);
   assert.ok(expect);
-  assert.ok(typeof (emitter) !== undefined);
+  assert.optionalObject(emitter);
   assert.ok(callback);
 
   // Allow connect setup traffic to bypass checks
-  if (_bypass && this.socket && this.socket.writable) {
+  if (_bypass && this._socket && this._socket.writable) {
     return this._sendSocket(message, expect, emitter, callback);
   }
-  if (!this.socket || !this.connected) {
+  if (!this._socket || !this.connected) {
     if (!this.queue.enqueue(message, expect, emitter, callback)) {
       callback(new ConnectionError('connection unavailable'));
     }
@@ -1268,7 +1365,8 @@ Client.prototype._sendSocket = function _sendSocket(message,
                                                     expect,
                                                     emitter,
                                                     callback) {
-  var conn = this.socket;
+  var conn = this._socket;
+  var tracker = this._tracker;
   var log = this.log;
   var self = this;
   var timer = false;
@@ -1313,7 +1411,7 @@ Client.prototype._sendSocket = function _sendSocket(message,
       event = event[0].toLowerCase() + event.slice(1);
       return _done(event, msg);
     } else {
-      conn.ldap.remove(message.messageID);
+      tracker.remove(message.messageID);
       // Potentially mark client as idle
       self._updateIdle();
 
@@ -1332,7 +1430,7 @@ Client.prototype._sendSocket = function _sendSocket(message,
 
   function onRequestTimeout() {
     self.emit('timeout', message);
-    var cb = conn.ldap.fetch(message.messageID);
+    var cb = tracker.fetch(message.messageID);
     if (cb) {
       //FIXME: the timed-out request should be abandoned
       cb(new errors.TimeoutError('request timeout (client interrupt)'));
@@ -1342,9 +1440,9 @@ Client.prototype._sendSocket = function _sendSocket(message,
   function writeCallback() {
     if (expect === 'abandon') {
       // Mark the messageID specified as abandoned
-      conn.ldap.abandon(message.abandonID);
+      tracker.abandon(message.abandonID);
       // No need to track the abandon request itself
-      conn.ldap.remove(message.id);
+      tracker.remove(message.id);
       return callback(null);
     } else if (expect === 'unbind') {
       conn.unbindMessageID = message.id;
@@ -1363,7 +1461,7 @@ Client.prototype._sendSocket = function _sendSocket(message,
   } // end writeCallback()
 
   // Start actually doing something...
-  conn.ldap.track(message, messageCallback);
+  tracker.track(message, messageCallback);
   // Mark client as active
   this._updateIdle(true);
 

lib/messages/ext_request.js

@@ -73,9 +73,9 @@ ExtendedRequest.prototype._toBer = function (ber) {
   assert.ok(ber);
 
   ber.writeString(this.requestName, 0x80);
-  if (Buffer.isBuffer(this.requestValue))
+  if (Buffer.isBuffer(this.requestValue)) {
     ber.writeBuffer(this.requestValue, 0x81);
-  else {
+  } else if (typeof (this.requestValue) === 'string') {
     ber.writeString(this.requestValue, 0x81);
   }
 

test/client.test.js

@@ -987,7 +987,7 @@ test('setup reconnect', function (t) {
           // can't test had_err because the socket error is being faked
           cb();
         });
-        rClient.socket.emit('error', new Error(msg));
+        rClient._socket.emit('error', new Error(msg));
       },
       doSearch
     ]
@@ -1081,15 +1081,15 @@ test('reconnect on server close', function (t) {
     });
   });
   clt.once('connect', function () {
-    t.ok(clt.socket);
+    t.ok(clt._socket);
     clt.once('connect', function () {
       t.ok(true, 'successful reconnect');
       clt.destroy();
       t.end();
     });
 
     // Simulate server-side close
-    clt.socket.destroy();
+    clt._socket.destroy();
   });
 });