nassh: sftp: improve server version packet parsing

The version packet from the server can be parsed like any other
response packet, other than the fact that requestId is not set.
Since we're already stuffing the pendingRequests_ with 'init' as
a fake requestId, lets use that in our version packets too.

This lets us drop the custom VERSION check in the onPacket call.

While we're here, lets parse out the optional extensions the
server might send over in its VERSION response.

Bug: chromium:722175
Change-Id: I7c20b49f3e37aaa00767ea68674fffb8dda0a429
Reviewed-on: https://chromium-review.googlesource.com/c/1286249
Reviewed-by: Vitaliy Shipitsyn <[email protected]>
Tested-by: Mike Frysinger <[email protected]>

Author: vapier

nassh/js/nassh_sftp_client.js

@@ -13,7 +13,11 @@ nassh.sftp = {};
  */
 nassh.sftp.Client = function(opt_basePath='') {
   // The version of the protocol we're using.
-  this.protoVersion = 3;
+  this.protocolClientVersion = 3;
+
+  // Extensions that the server supports.
+  this.protocolServerVersion = null;
+  this.protocolServerExtensions = {};
 
   // The packet request id counter.
   this.requestId_ = 0;
@@ -99,10 +103,6 @@ nassh.sftp.Client.prototype.parseBuffer = function() {
  */
 nassh.sftp.Client.prototype.onPacket = function(packet) {
   var packetType = packet.getUint8();
-  if (packetType == nassh.sftp.packets.RequestPackets.VERSION) {
-    this.pendingRequests_['init']();
-    return true;
-  }
 
   // Obtain the response packet's constructor and create it.
   var ResponseType = nassh.sftp.packets.ResponsePackets[packetType]
@@ -234,11 +234,13 @@ nassh.sftp.Client.prototype.init = function() {
   var packet = new nassh.sftp.Packet();
   packet.setUint32(5); // length, 5 bytes for type and version fields
   packet.setUint8(nassh.sftp.packets.RequestPackets.INIT);
-  packet.setUint32(this.protoVersion);
+  packet.setUint32(this.protocolClientVersion);
 
-  this.pendingRequests_['init'] = () => {
+  this.pendingRequests_['init'] = (packet) => {
     console.log('init: SFTP');
     this.onInit();
+    this.protocolServerVersion = packet.version;
+    this.protocolServerExtensions = packet.extensions;
     this.isInitialised = true;
   };
   this.sendToPlugin_('onRead', [0, btoa(packet.toString())]);

nassh/js/nassh_sftp_packet.js

@@ -159,6 +159,15 @@ nassh.sftp.Packet.prototype.getLength = function() {
   return this.packet_.length;
 };
 
+/**
+ * Check whether the end of the packet data has been reached.
+ *
+ * @returns {!boolean} true If the end of the packet data has been reached.
+ */
+nassh.sftp.Packet.prototype.eod = function() {
+  return this.offset_ === this.packet_.length;
+};
+
 /**
  * Converts a byte array string to an int
  */

nassh/js/nassh_sftp_packet_types.js

@@ -78,6 +78,73 @@ nassh.sftp.packets.AttrsPacket = function(packet) {
   this.attrs = nassh.sftp.packets.getFileAttrs(packet);
 };
 
+/**
+ * Make sure the name conforms to the specification.
+ *
+ * The SFTP RFC defines the extension-name format:
+ * https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#section-4.2
+ *
+ * Which says to follow the SSH "Algorithm and Method Naming" RFC format:
+ * https://tools.ietf.org/html/rfc4251#section-6
+ *
+ * Really we just need this func to filter out extensions that we don't care
+ * about in our implementation.  If another one shows up, we can revisit.
+ *
+ * @param {string} name The protocol name to check.
+ * @return {boolean} True if the name is valid.
+ */
+nassh.sftp.packets.ValidExtension = function(ext) {
+  // The RFC is a little ambiguous, but it uses "name" to refer to the entire
+  // extension name, not just sub-components (when using the @ form).
+  if (ext.length > 64) {
+    return false;
+  }
+
+  // Split apart the extension@domain format.
+  const ary = ext.split('@');
+  if (ary.length > 2) {
+    return false;
+  }
+  const [name, domain] = ary;
+
+  // Names cannot contain control chars or whitespace (0x00-0x20), "@" (0x40),
+  // "," (0x2c), or DEL (0x7f).  So remove all valid chars and make sure the
+  // result is an empty string.
+  if (name.length == 0 ||
+      name.replace(/[\x21-\x2b\x2d-\x3f\x41-\x7e]/g, '').length != 0) {
+    return false;
+  }
+
+  // The domain part is supposed to be a bit more strict ("a valid domain"),
+  // but using the same form as above should be good enough.
+  if (domain !== undefined) {
+    if (domain.replace(/[\x21-\x2b\x2d-\x3f\x41-\x7e]/g, '').length != 0) {
+      return false;
+    }
+  }
+
+  return true;
+};
+
+/**
+ * SFTP Version Packet containing the version and possible extensions.
+ */
+nassh.sftp.packets.VersionPacket = function(packet) {
+  this.requestId = 'init';
+  this.version = packet.getUint32();
+
+  // Pull out all the extensions that might exist.
+  this.extensions = {};
+  while (!packet.eod()) {
+    const name = packet.getString();
+    const data = packet.getString();
+    // The SFTP RFC says we should silently ignore unknown/invalid entries.
+    if (nassh.sftp.packets.ValidExtension(name)) {
+      this.extensions[name] = data;
+    }
+  }
+};
+
 /**
  * Unknown Packet containing the request id (potentially garbage) and associated
  * data (also potentially garbage).
@@ -284,6 +351,7 @@ nassh.sftp.packets.RequestPackets = {
  * Possible SFTP Response Packet types
  */
 nassh.sftp.packets.ResponsePackets = {
+  2: nassh.sftp.packets.VersionPacket,
   101: nassh.sftp.packets.StatusPacket,
   102: nassh.sftp.packets.HandlePacket,
   103: nassh.sftp.packets.DataPacket,