[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-dev] Draft patchset for PT2.0 support (ticket #21816)



Good day, tor-dev;

As some of you know, I've been working on a patchset for Tor to allow
it to participate in Pluggable Transports 2.0 configuration, primarily
the new JSON Parameter Block SOCKS method (at least that's what I've
been calling it in the absence of a more official name), almost but
not quite as described in section 3.3.4 of PT2 draft 2 [1]---i.e.,
basically #21816 [2].

[1] https://www.pluggabletransports.info/assets/PTSpecV2Draft2.pdf
[2] https://trac.torproject.org/projects/tor/ticket/21816

I'm attaching a draft patchset which adds this functionality, with the
intent of getting feedback and making remaining cleanups or other
modifications necessary to get it merged into Tor. I have successfully
completed circuits through an obfs4 bridge using both obfs4proxy (PT1)
and a version of shapeshifter-dispatcher (PT2) using a patched Tor. I've
tried to follow the local style, but the preferred implementation
strategies aren't always clear, and of course I'd appreciate any reports
of other problems.

A forked Git repository is also available on Bitbucket [5][6], which
will be updated as I make remaining changes.

[5] https://bitbucket.org/DasyatidPrime/tor-rtt2017-21816.git (Git)
[6] https://bitbucket.org/DasyatidPrime/tor-rtt2017-21816/src (Web)

More implementation details are below if you're interested in this;
thanks for your attention. I'll try to be around on IRC more during
the week, so feel free to ping me there as well.

-RTT

... Details:

Not visible above, related to the target functionality:

  - I'm assuming the SOCKS method includes a response with an
    analogous structure to RFC 1929; PT2 draft 2 doesn't specify
    one. I've cleared this with blanu, and that's intended for the
    next draft of the PT2 specification.

  - Similarly, the length prefix is in fact big-endian; the example in
    PT2 draft 2 is wrong, though the text is correct.

  - We're looking into getting an IANA assignment for the method
    number. Technically this probably meets the requirements for the
    private use block, but I feel like interop might be easier later
    on, and it could simplify code paths in places to have a registered
    number (mainly if it's possible to decouple the method negotiation
    from the configuration-version plumbing). I believe this is still
    in limbo.

  - For the PT2 side, I've been testing against my branch of
    shapeshifter-dispatcher [3] compiled with my branch of
    shapeshifter-ipc [4] since there were some breaking changes to
    Shapeshifter upstream which were otherwise preventing it from
    interoperating with Tor. I'm planning to help merge fixes into
    Shapeshifter upstream as I can; it's my current understanding that
    there aren't any other PT2 managed transport implementations to
    test against.

[3]
https://github.com/OperatorFoundation/shapeshifter-dispatcher/tree/rtt2017
[4] https://github.com/OperatorFoundation/shapeshifter-ipc/tree/rtt2017

Issues on my radar currently (comments appreciated):

  - We probably want unit tests for the (limited) JSON encoding
    functions, and for the factored-out RFC 1929 encoding functions.
    Anything else that looks feasibly testable here?

  - It's not clear to me whether negotiating a PT2 configuration
    version still allows PT1-style RFC 1929 parameter encoding so that
    managed transports can support the new configuration version and
    the new SOCKS method separately. I've assumed it might be
    possible, so far, but that's not being tested against anything.

  - I currently restrict parameters to ASCII to avoid either writing
    a JSON encoder that can spit out invalid JSON or writing a JSON
    encoder that has to validate incoming UTF-8. The impression I've
    gotten is that this is probably okay, but if there are counter-
    examples, I can put in UTF-8 passthrough.

  - The commit sequence isn't the cleanest. How high a priority is it
    to reorder/combine patch hunks to make a cleaner one?

  - We still need a 'changes' file. (What would be an appropriate
    heading for this? Is this a minor feature, for instance?)

A few other questions:

  - Is there an effective way of doing automated testing of the SOCKS
    state machine currently in Tor? I didn't see anything obvious in
    the test directory. This seems like the most fragile part,
    especially since both the original and modified versions are not
    very explicit in their state machine nature and are split between
    multiple files.

  - Can there ever be more than one managed_proxy_t to a transport
    name? More generally, is there a relational diagram of the main
    Tor data structures somewhere? A lot of the way the plumbing for
    state and configuration information is set up feels kind of
    fragile.

From 56234341be9dcd72768c1f84e130a76d456a9dc5 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:27:49 -0500
Subject: [PATCH 01/12] PT2.0: Add JSON encoding functions for new arg blocks

These will be needed for the new SOCKS authentication format that
supports larger argument blocks.  We avoid taking Interesting Unicode
Processing dependencies by restricting the arguments to printable
ASCII, since this is likely the de facto situation in the current
SOCKS argument encoding anyway.
---
 src/or/transports.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/or/transports.h |   3 ++
 2 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/src/or/transports.c b/src/or/transports.c
index 31849a8d1..18a1d9836 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -1697,6 +1697,125 @@ pt_get_socks_args_for_proxy_addrport(const tor_addr_t *addr, uint16_t port)
   return pt_stringify_socks_args(socks_args);
 }
 
+/** Helper: convert the key-value pair in <b>arg</b> into a member
+ *  suitable for being part of a JSON object.  In other words, the
+ *  string <code>k=v</code> becomes <code>"k":"v"</code>, escaping any
+ *  characters in <code>k</code> or <code>v</code> which require it
+ *  according to the JSON specification.  A trailing comma is not
+ *  included.  Return a newly allocated string with the member, or
+ *  NULL if unsupported characters are detected in <b>arg</b>.
+ *
+ *  Currently, <b>arg</b> must be an ASCII string.
+ **/
+static char *
+json_member_for_socks_arg(const char *arg)
+{
+  /* An ASCII key or value character results in at most two characters
+     when escaped.  There are five overhead characters.  (The actual
+     bound is slightly lower, but this is easier to understand.) */
+  size_t out_max_chars = strlen(arg) * 2 + 5;
+  char *out_buf = tor_malloc(out_max_chars + 1);
+  /* Invariant: writable(out, out_end - out + 1) && out_buf <= out &&
+     out <= out_end. */
+  char *out = out_buf, *out_end = &out_buf[out_max_chars];
+  /* Invariant: readable(in, 1) && arg <= in && in <= arg_end,
+     where arg_end = strchr(arg, '\0'). */
+  const char *in = arg;
+  int saw_equals = 0;
+
+  *(out++) = '"';
+
+  for (char ch = *in; ch != '\0'; ch = *(++in)) {
+    if (ch == '=' && !saw_equals) {
+      /* Add delimiter between key and value, at most once per call. */
+      tor_assert(3 <= out_end - out);
+      *(out++) = '"';
+      *(out++) = ':';
+      *(out++) = '"';
+      saw_equals = 1;
+    } else if (ch == '"' || *in == '\\') {
+      /* Add escaped character. */
+      tor_assert(2 <= out_end - out);
+      *(out++) = '\\';
+      *(out++) = *in;
+    } else if (0x20 <= ch && ch <= 0x7e) {
+      /* Add character raw. */
+      tor_assert(1 <= out_end - out);
+      *(out++) = *in;
+    } else {
+      /* Non-ASCII characters are not supported in this function. */
+      goto fail;
+    }
+  }
+
+  *(out++) = '"';
+  *out = '\0';
+  tor_assert(out <= out_end);
+
+  /* We don't bother to realloc here because it's not necessary for
+     the function contract, invites an extra arithmetic error, and the
+     wasted memory will currently always be freed shortly in
+     pt_jsonify_socks_args anyway. */
+  return out_buf;
+
+ fail:
+  tor_free(out_buf);
+  return NULL;
+}
+
+/** Encode the SOCKS arguments in <b>socks_args</b> as a JSON object
+ *  according to the PT2 specification.  Return a string containing
+ *  the JSON object, or NULL if the arguments could not be encoded.
+ *  The string is allocated on the heap and it's the responsibility of
+ *  the caller to free it after use. */
+char *
+pt_jsonify_socks_args(const smartlist_t *socks_args)
+{
+  smartlist_t *sl_members = NULL;
+  char *joined_members = NULL;
+  char *result_json = NULL;
+
+  tor_assert(socks_args);
+  tor_assert(smartlist_len(socks_args) > 0);
+
+  sl_members = smartlist_new();
+
+  SMARTLIST_FOREACH_BEGIN(socks_args, const char *, s) {
+    char *member = json_member_for_socks_arg(s);
+    if (!member)
+      goto done;
+
+    smartlist_add(sl_members, member);
+  } SMARTLIST_FOREACH_END(s);
+
+  joined_members = smartlist_join_strings(sl_members, ",", 0, NULL);
+  tor_asprintf(&result_json, "{%s}", joined_members);
+  tor_free(joined_members);
+
+done:
+  SMARTLIST_FOREACH(sl_members, char *, s, tor_free(s));
+  smartlist_free(sl_members);
+
+  return result_json;
+}
+
+/** Return a JSON string of an object of arguments that we should pass
+ *  to the pluggable transports proxy in <b>addr</b>:<b>port</b>,
+ *  according to the PT2 specification.  The string is allocated on
+ *  the heap and it's the responsibility of the caller to free it
+ *  after use. */
+char *
+pt_get_json_object_for_proxy_addrport(const tor_addr_t *addr, uint16_t port)
+{
+  const smartlist_t *socks_args = NULL;
+
+  socks_args = get_socks_args_by_bridge_addrport(addr, port);
+  if (!socks_args)
+    return NULL;
+
+  return pt_jsonify_socks_args(socks_args);
+}
+
 /** The tor config was read.
  *  Destroy all managed proxies that were marked by a previous call to
  *  prepare_proxy_list_for_config_read() and are not used by the new
@@ -1740,4 +1859,3 @@ pt_free_all(void)
     managed_proxy_list=NULL;
   }
 }
-
diff --git a/src/or/transports.h b/src/or/transports.h
index 44a9626e5..c90ff34dd 100644
--- a/src/or/transports.h
+++ b/src/or/transports.h
@@ -61,9 +61,12 @@ void sweep_proxy_list(void);
 
 smartlist_t *get_transport_proxy_ports(void);
 char *pt_stringify_socks_args(const smartlist_t *socks_args);
+char *pt_jsonify_socks_args(const smartlist_t *socks_args);
 
 char *pt_get_socks_args_for_proxy_addrport(const tor_addr_t *addr,
                                             uint16_t port);
+char *pt_get_json_object_for_proxy_addrport(const tor_addr_t *addr,
+                                            uint16_t port);
 
 #ifdef PT_PRIVATE
 /** State of the managed proxy configuration protocol. */
-- 
2.14.0


From 7c5ab060db1b60c6e9729a6da6bfb2f4090aa950 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:28:04 -0500
Subject: [PATCH 02/12] PT2.0: Add WANT_AUTH_METHOD_PT2 flows to SOCKS handling

The new proxy state, PROXY_SOCKS5_WANT_AUTH_METHOD_PT2, implies that
we've requested either PT2 JSON parameter-block pseudo-authentication
_or_ RFC1929 username/password authentication repurposed for parameter
purposes.  It also causes a proxy connection to be aborted if
parameters existed that could not be transmitted.

This also pulls out some of the RFC1929 encoding that was previously
in connection_read_proxy_handshake into two new functions,
encode_rfc1929_spill and encode_rfc1929_separate, mostly since
encode_rfc1929_spill now has to potentially happen in two places.

In theory, we should be able to omit one of these places, but this is
being kept defensively for now in case there are broken PT1
implementations which do not correctly implement SOCKS5 subnegotiation
handling. (In this case, a separate configuration flag would still
have to be added later, but it would trigger the current code path.)
---
 src/or/buffers.c    |  42 ++++++++-
 src/or/connection.c | 255 +++++++++++++++++++++++++++++++++++++++++++++-------
 src/or/connection.h |   4 +
 src/or/or.h         |  12 ++-
 4 files changed, 279 insertions(+), 34 deletions(-)

diff --git a/src/or/buffers.c b/src/or/buffers.c
index bd84103c3..f2f6f5ab5 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1878,8 +1878,12 @@ parse_socks(const char *data, size_t datalen, socks_request_t *req,
  * <b>reason</b> is set to a descriptive message (free() when finished
  * with it).
  *
- * As a special case, 2 is returned when user/pass is required
- * during SOCKS5 handshake and user/pass is configured.
+ * Two other return values signal entry into subnegotiations:
+ *
+ * <ul>
+ * <li>2: RFC1929 username/password authentication is needed.</li>
+ * <li>3: PT2 parameter-block pseudo-authentication is needed.</li>
+ * </ul>
  */
 int
 fetch_from_buf_socks_client(buf_t *buf, int state, char **reason)
@@ -1960,6 +1964,39 @@ parse_socks_client(const uint8_t *data, size_t datalen,
                            "authentication methods");
       return -1;
 
+    case PROXY_SOCKS5_WANT_AUTH_METHOD_PT2:
+      /* We have PT parameters. Return 2 if we can proceed with PT1-style
+       * user/pass authentication, or 3 for PT2-style parameter-block. */
+      switch (data[1]) {
+        case 0x00:
+          /* If the server (which is a PT client) returns "no
+           * authentication", this could actually be quite bad if we
+           * have parameters, but that's handled in
+           * connection_read_proxy_handshake. */
+          log_info(LD_NET, "SOCKS 5 client: server refuses PT parameters.");
+          *drain_out = -1;
+          return 1;
+
+        case 0x02:
+          log_info(LD_NET, "SOCKS 5 client: using username/password "
+                           "pseudo-authentication for PT parameters.");
+          *drain_out = -1;
+          return 2;
+
+          /* TODO: symbolic constants for SOCKS5 auth method numbers */
+        case 0x80:
+          log_info(LD_NET, "SOCKS 5 client: using PT2 pseudo-"
+                           "authentication for PT parameters.");
+          *drain_out = -1;
+          return 3;
+
+        /* fall through */
+      }
+
+      *reason = tor_strdup("server doesn't support any of our available "
+                           "PT parameter passing methods");
+      return -1;
+
     case PROXY_SOCKS5_WANT_AUTH_RFC1929_OK:
       /* handle server reply to rfc1929 authentication */
       if (data[1] != 0x00) {
@@ -2211,4 +2248,3 @@ assert_buf_ok(buf_t *buf)
     tor_assert(buf->datalen == total);
   }
 }
-
diff --git a/src/or/connection.c b/src/or/connection.c
index 5c65e886c..f59079b86 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2357,6 +2357,81 @@ connection_fetch_from_buf_socks_client(connection_t *conn,
   return fetch_from_buf_socks_client(conn->inbuf, state, reason);
 }
 
+/** Encode into <b>buf</b> an RFC1929 authentication record in wire format
+ *  representing the pluggable transport args in <b>args</b>, spilling them
+ *  from the username to the password field as needed.  <b>buf</b> must have
+ *  space for at least <code>MAX_RFC1929_ENCODED_SIZE</code> bytes.  Return the
+ *  number of bytes encoded, or -1 for error.  If <b>args</b> is between 1 and
+ *  <code>MAX_SOCKS5_AUTH_SIZE_TOTAL</code> characters, always succeds. */
+static ssize_t
+encode_rfc1929_spill(unsigned char *buf, const char *args)
+{
+  /* Invariant: buf <= out && out <= &buf[MAX_RFC1929_ENCODED_SIZE] */
+  unsigned char *out = buf;
+  /* Invariant: args <= in && in <= strchr(args, '\0') */
+  const char *in = args;
+  size_t usize = 0, psize = 0;
+  tor_assert(*args != '\0');
+
+  *(out++) = 1; /* negotiation version */
+
+  while (usize < MAX_SOCKS5_AUTH_FIELD_SIZE && *in != '\0')
+    out[1 + (usize++)] = *(in++);
+  if (usize == 0)
+    return -1;
+  out[0] = usize;
+  out += 1 + usize;
+
+  if (*in == '\0') {
+    /* no chars left for password */
+    out[0] = 1;
+    out[1] = '\0';
+    out += 2;
+  } else {
+    while (psize < MAX_SOCKS5_AUTH_FIELD_SIZE && *in != '\0')
+      out[1 + (psize++)] = *(in++);
+    out[0] = psize;
+    out += 1 + psize;
+  }
+
+  /* If there's any chars left, we didn't have enough room. */
+  if (*in != '\0')
+    return -1;
+
+  tor_assert(out <= &buf[MAX_RFC1929_ENCODED_SIZE]);
+  return out - buf;
+}
+
+/** Encode into <b>buf</b> an RFC1929 authentication record in wire format with
+ *  the username <b>user</b> and the password <b>pass</b>.  <b>buf</b> must
+ *  have space for at least <code>MAX_RFC1929_ENCODED_SIZE</code> bytes.
+ *  Return the number of bytes encoded, or -1 for error.  If <b>user</b> and
+ *  <b>pass</b> are each between 1 and <code>MAX_SOCKS5_AUTH_FIELD_SIZE</code>
+ *  characters, always succeeds. */
+static ssize_t
+encode_rfc1929_separate(unsigned char *buf, const char *user,
+                        const char *pass)
+{
+  /* Invariant: buf <= out && out <= &buf[MAX_RFC1929_ENCODED_SIZE] */
+  unsigned char *out = buf;
+  size_t usize = strlen(user), psize = strlen(pass);
+  if (!(0 < usize && usize <= MAX_SOCKS5_AUTH_FIELD_SIZE))
+    return -1;
+  if (!(0 < psize && psize <= MAX_SOCKS5_AUTH_FIELD_SIZE))
+    return -1;
+
+  *(out++) = 1;
+  *(out++) = usize;
+  memcpy(out, user, usize);
+  out += usize;
+  *(out++) = psize;
+  memcpy(out, pass, psize);
+  out += psize;
+
+  tor_assert(out <= &buf[MAX_RFC1929_ENCODED_SIZE]);
+  return out - buf;
+}
+
 /** Call this from connection_*_process_inbuf() to advance the proxy
  * handshake.
  *
@@ -2409,12 +2484,13 @@ connection_read_proxy_handshake(connection_t *conn)
 
       /* send auth if needed, otherwise do connect */
       if (ret == 1) {
+        /* TODO: this doesn't check whether we had pending PT parameters like
+           the next case does. Should it? */
         connection_send_socks5_connect(conn);
         ret = 0;
       } else if (ret == 2) {
-        unsigned char buf[1024];
-        size_t reqsize, usize, psize;
-        const char *user, *pass;
+        unsigned char buf[MAX_RFC1929_ENCODED_SIZE];
+        ssize_t reqsize;
         char *socks_args_string = NULL;
 
         if (get_proxy_type() == PROXY_PLUGGABLE) {
@@ -2428,25 +2504,44 @@ connection_read_proxy_handshake(connection_t *conn)
 
           log_debug(LD_NET, "SOCKS5 arguments: %s", socks_args_string);
           tor_assert(strlen(socks_args_string) > 0);
-          tor_assert(strlen(socks_args_string) <= MAX_SOCKS5_AUTH_SIZE_TOTAL);
+          if (strlen(socks_args_string) > MAX_SOCKS5_AUTH_SIZE_TOTAL) {
+            log_warn(LD_NET, "PT args too long for RFC1929 encoding: %d > %d",
+                     (int)strlen(socks_args_string),
+                     (int)MAX_SOCKS5_AUTH_SIZE_TOTAL);
+            tor_free(socks_args_string);
+            ret = -1;
+            break;
+          }
 
-          if (strlen(socks_args_string) > MAX_SOCKS5_AUTH_FIELD_SIZE) {
-            user = socks_args_string;
-            usize = MAX_SOCKS5_AUTH_FIELD_SIZE;
-            pass = socks_args_string + MAX_SOCKS5_AUTH_FIELD_SIZE;
-            psize = strlen(socks_args_string) - MAX_SOCKS5_AUTH_FIELD_SIZE;
-          } else {
-            user = socks_args_string;
-            usize = strlen(socks_args_string);
-            pass = "\0";
-            psize = 1;
+          reqsize = encode_rfc1929_spill(buf, socks_args_string);
+          if (reqsize < 0) {
+            /* This shouldn't happen because we already satisfied the
+             * precondition above. */
+            log_err(LD_BUG, "Could not encode PT parameters despite precheck!");
+            tor_fragile_assert();
+            tor_free(socks_args_string);
+            ret = -1;
+            break;
           }
         } else if (get_options()->Socks5ProxyUsername) {
-          user = get_options()->Socks5ProxyUsername;
-          pass = get_options()->Socks5ProxyPassword;
+          const char *user = get_options()->Socks5ProxyUsername;
+          const char *pass = get_options()->Socks5ProxyPassword;
           tor_assert(user && pass);
-          usize = strlen(user);
-          psize = strlen(pass);
+          /* Username and password lengths should have been checked during
+             torrc parsing. */
+          tor_assert(strlen(user) <= MAX_SOCKS5_AUTH_FIELD_SIZE);
+          tor_assert(strlen(pass) <= MAX_SOCKS5_AUTH_FIELD_SIZE);
+
+          reqsize = encode_rfc1929_separate(buf, user, pass);
+          if (reqsize < 0) {
+            /* This shouldn't happen because we already satisfied the
+             * precondition above. */
+            log_err(LD_BUG, "Could not encode SOCKS authentication "
+                            "despite precheck!");
+            tor_fragile_assert();
+            ret = -1;
+            break;
+          }
         } else {
           log_err(LD_BUG, "We entered %s for no reason!", __func__);
           tor_fragile_assert();
@@ -2454,23 +2549,124 @@ connection_read_proxy_handshake(connection_t *conn)
           break;
         }
 
-        /* Username and password lengths should have been checked
-           above and during torrc parsing. */
-        tor_assert(usize <= MAX_SOCKS5_AUTH_FIELD_SIZE &&
-                   psize <= MAX_SOCKS5_AUTH_FIELD_SIZE);
-        reqsize = 3 + usize + psize;
+        if (socks_args_string)
+          tor_free(socks_args_string);
+
+        connection_write_to_buf((char *)buf, reqsize, conn);
+
+        conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_RFC1929_OK;
+        ret = 0;
+      }
+      break;
 
-        buf[0] = 1; /* negotiation version */
-        buf[1] = usize;
-        memcpy(buf + 2, user, usize);
-        buf[2 + usize] = psize;
-        memcpy(buf + 3 + usize, pass, psize);
+    case PROXY_SOCKS5_WANT_AUTH_METHOD_PT2:
+      ret = connection_fetch_from_buf_socks_client(conn,
+                                                   conn->proxy_state,
+                                                   &reason);
 
-        if (socks_args_string)
+      if (get_proxy_type() != PROXY_PLUGGABLE) {
+        /* We should never have set the connection to this proxy state
+         * if we're not using a PT proxy. */
+        log_err(LD_BUG, "In PT-oriented proxy state, but proxy is not PT!");
+        tor_fragile_assert();
+        ret = -1;
+        break;
+      }
+
+      if (ret == 1) {
+        /* Server requested no (pseudo-)authentication. */
+        if (get_socks_args_by_bridge_addrport(&conn->addr, conn->port)) {
+          /* If we have PT parameters and no place to put them, this is
+           * actually quite bad, unlike if this were 'real' authentication
+           * where we could just take the open connection.  We don't want to
+           * accidentally establish a PT connection without the right
+           * parameters, so abort the connection.
+           *
+           * Note that we can't do this unconditionally, because the PT 2.0
+           * spec says parameterless PTs are allowed to signal no
+           * authentication at the SOCKS5 layer.
+           */
+          log_warn(LD_NET, "We have PT parameters but server refused them.");
+          ret = -1;
+          break;
+        } else {
+          connection_send_socks5_connect(conn);
+          ret = 0;
+        }
+      } else if (ret == 2) {
+        /* Server requested RFC1929 username/password authentication. */
+        unsigned char buf[MAX_RFC1929_ENCODED_SIZE];
+        ssize_t reqsize;
+        char *socks_args_string = NULL;
+
+        socks_args_string =
+          pt_get_socks_args_for_proxy_addrport(&conn->addr, conn->port);
+        if (!socks_args_string) {
+          log_warn(LD_NET, "Could not create SOCKS args string.");
+          ret = -1;
+          break;
+        }
+
+        log_debug(LD_NET, "SOCKS5 arguments: %s", socks_args_string);
+        tor_assert(strlen(socks_args_string) > 0);
+        if (strlen(socks_args_string) > MAX_SOCKS5_AUTH_SIZE_TOTAL) {
+          log_warn(LD_NET, "PT args too long for RFC1929 encoding: %d > %d",
+                   (int)strlen(socks_args_string),
+                   (int)MAX_SOCKS5_AUTH_SIZE_TOTAL);
+          tor_free(socks_args_string);
+          ret = -1;
+          break;
+        }
+
+        reqsize = encode_rfc1929_spill(buf, socks_args_string);
+        if (reqsize < 0) {
+          /* This shouldn't happen because we already satisfied the
+           * precondition above. */
+          log_err(LD_BUG, "Could not encode PT parameters despite precheck!");
+          tor_fragile_assert();
           tor_free(socks_args_string);
+          ret = -1;
+          break;
+        }
 
         connection_write_to_buf((char *)buf, reqsize, conn);
+        tor_free(socks_args_string);
+
+        conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_RFC1929_OK;
+        ret = 0;
+      } else if (ret == 3) {
+        /* Server requested PT2 parameter-block pseudo-authentication. */
+        char *json_string = NULL;
+        unsigned char size_buf[4];
+        size_t json_size;
+
+        json_string =
+          pt_get_json_object_for_proxy_addrport(&conn->addr, conn->port);
+        if (!json_string) {
+          log_warn(LD_NET, "Could not create PT2 JSON args string.");
+          ret = -1;
+          break;
+        }
+
+        log_debug(LD_NET, "PT2-JSON arguments: %s", json_string);
+        json_size = strlen(json_string);
+        if (json_size > UINT32_MAX) {
+          tor_free(json_string);
+          log_warn(LD_NET, "PT2 JSON args string must be less than 4 GiB");
+          ret = -1;
+          break;
+        }
+
+        size_buf[0] = (unsigned char)(json_size >> 24);
+        size_buf[1] = (unsigned char)(json_size >> 16);
+        size_buf[2] = (unsigned char)(json_size >> 8);
+        size_buf[3] = (unsigned char)json_size;
 
+        connection_write_to_buf((char *)size_buf, 4, conn);
+        connection_write_to_buf(json_string, json_size, conn);
+        tor_free(json_string);
+
+        /* TODO: rename this to something like *_SIMPLE_OK */
         conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_RFC1929_OK;
         ret = 0;
       }
@@ -5196,4 +5392,3 @@ clock_skew_warning(const connection_t *conn, long apparent_skew, int trusted,
                                  apparent_skew, ext_source);
   tor_free(ext_source);
 }
-
diff --git a/src/or/connection.h b/src/or/connection.h
index 36e45aef3..4f2ea5f75 100644
--- a/src/or/connection.h
+++ b/src/or/connection.h
@@ -103,6 +103,10 @@ int connection_connect_unix(connection_t *conn, const char *socket_path,
     username and password fields. */
 #define MAX_SOCKS5_AUTH_SIZE_TOTAL 2*MAX_SOCKS5_AUTH_FIELD_SIZE
 
+/** Total maximum size of SOCKS5 RFC1929 authentication record
+    as sent on the wire. */
+#define MAX_RFC1929_ENCODED_SIZE ((2*MAX_SOCKS5_AUTH_FIELD_SIZE) + 3)
+
 int connection_proxy_connect(connection_t *conn, int type);
 int connection_read_proxy_handshake(connection_t *conn);
 void log_failed_proxy_connection(connection_t *conn);
diff --git a/src/or/or.h b/src/or/or.h
index ff11c7279..949c17167 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -254,7 +254,11 @@ typedef enum {
    any authentication . */
 #define PROXY_SOCKS5_WANT_AUTH_METHOD_NONE 4
 /* We use a SOCKS5 proxy and we try to negotiate with
-   Username/Password authentication . */
+   Username/Password authentication.  This can be either true
+   authentication or PT1 parameter-passing pseudo-authentication, but
+   in the latter case we will usually allow PT2 parameter blocks as
+   well and will be in the PROXY_SOCKS5_WANT_AUTH_METHOD_PT2 state
+   instead. */
 #define PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929 5
 /* We use a SOCKS5 proxy and we just sent our credentials. */
 #define PROXY_SOCKS5_WANT_AUTH_RFC1929_OK 6
@@ -262,6 +266,12 @@ typedef enum {
 #define PROXY_SOCKS5_WANT_CONNECT_OK 7
 /* We use a proxy and we CONNECTed successfully!. */
 #define PROXY_CONNECTED 8
+/* We use a SOCKS5 proxy and we try to negotiate with PT2
+   parameter-block pseudo-authentication and/or PT1 Username/Password
+   pseudo-authentication.  In the strange event that we are only
+   willing to provide PT1, we will wind up in
+   PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929 instead. */
+#define PROXY_SOCKS5_WANT_AUTH_METHOD_PT2 9
 
 /** True iff <b>x</b> is an edge connection. */
 #define CONN_IS_EDGE(x) \
-- 
2.14.0


From 88f5b2a9497e98183c666423ca703184cc8262b1 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:28:06 -0500
Subject: [PATCH 03/12] PT2.0: Update connection_proxy_state_to_string for new
 constant

---
 src/or/connection.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/or/connection.c b/src/or/connection.c
index f59079b86..853c9d60c 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2058,6 +2058,7 @@ connection_proxy_state_to_string(int state)
     "PROXY_SOCKS5_WANT_AUTH_RFC1929_OK",
     "PROXY_SOCKS5_WANT_CONNECT_OK",
     "PROXY_CONNECTED",
+    "PROXY_SOCKS5_WANT_AUTH_METHOD_PT2",
   };
 
   if (state < PROXY_NONE || state > PROXY_CONNECTED)
-- 
2.14.0


From 39ff3f4e8ec5760ff9e274ec021e22ad27fc0cb6 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:28:07 -0500
Subject: [PATCH 04/12] PT2.0: Add note of limit of proxy_state bitfield near
 its constants

---
 src/or/or.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/or/or.h b/src/or/or.h
index 949c17167..c660908f0 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -273,6 +273,10 @@ typedef enum {
    PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929 instead. */
 #define PROXY_SOCKS5_WANT_AUTH_METHOD_PT2 9
 
+#define PROXY_STATE_MAX_ 9
+/* !!!! If there is ever a proxy state over 15, we must grow the
+ * proxy_state field in connection_t. */
+
 /** True iff <b>x</b> is an edge connection. */
 #define CONN_IS_EDGE(x) \
   ((x)->type == CONN_TYPE_EXIT || (x)->type == CONN_TYPE_AP)
-- 
2.14.0


From d7c389009cb1335d32b4f0f487bdc34280a78f3e Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:28:08 -0500
Subject: [PATCH 05/12] PT2.0: Change bridge parameter length check from error
 to warning

We don't have a good way of getting the PT version in there (and
probably never will at that stage of execution?) so just always
warn, with a reworded message mentioning that only PT2 proxies
can handle long parameter blocks.
---
 src/or/config.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/or/config.c b/src/or/config.c
index 9b6bf40eb..3f0bfb56d 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -5485,10 +5485,11 @@ validate_transport_socks_arguments(const smartlist_t *args)
   tor_free(socks_string);
 
   if (socks_string_len > MAX_SOCKS5_AUTH_SIZE_TOTAL) {
-    log_warn(LD_CONFIG, "SOCKS arguments can't be more than %u bytes (%lu).",
+    log_warn(LD_CONFIG,
+             "PT1 SOCKS arguments can't be more than %u bytes (%lu).  "
+             "This transport must implement PT2 for these parameters",
              MAX_SOCKS5_AUTH_SIZE_TOTAL,
              (unsigned long) socks_string_len);
-    return -1;
   }
 
   return 0;
-- 
2.14.0


From dfa86cadfe846b26e8f838b17cad0cc5a6da2593 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:28:09 -0500
Subject: [PATCH 06/12] PT2.0: Adjust outgoing SOCKS auth negotiation for PT

This adds the PT2 JSON method when there are PT parameters to
negotiate. (Though possibly it should only do that if we have a
known PT2 on the other side, per #21816 comment 3.) It also
switches to the correct state to activate handling for it: this
is the part that makes the PROXY_SOCKS5_WANT_AUTH_METHOD_PT2
state live.
---
 src/or/connection.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/or/connection.c b/src/or/connection.c
index 853c9d60c..798b2968f 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2215,23 +2215,36 @@ connection_proxy_connect(connection_t *conn, int type)
     }
 
     case PROXY_SOCKS5: {
-      unsigned char buf[4]; /* fields: vers, num methods, method list */
+      unsigned char buf[16]; /* fields: vers, num methods, method list */
 
       /* Send a SOCKS5 greeting (connect request must wait) */
 
       buf[0] = 5; /* version */
+      /* buf[1] will be the count of auth methods, and buf[2...] will
+         be the identifying numbers of the auth methods. */
 
       /* We have to use SOCKS5 authentication, if we have a
          Socks5ProxyUsername or if we want to pass arguments to our
          pluggable transport proxy: */
-      if ((options->Socks5ProxyUsername) ||
-          (get_proxy_type() == PROXY_PLUGGABLE &&
-           (get_socks_args_by_bridge_addrport(&conn->addr, conn->port)))) {
-      /* number of auth methods */
+      if (options->Socks5ProxyUsername) {
         buf[1] = 2;
-        buf[2] = 0x00; /* no authentication */
-        buf[3] = 0x02; /* rfc1929 Username/Passwd auth */
+        buf[2] = 0x00; /* none - credentials weren't needed */
+        buf[3] = 0x02; /* RFC1929 - use username/password */
+
         conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929;
+      } else if (get_proxy_type() == PROXY_PLUGGABLE) {
+        if (get_socks_args_by_bridge_addrport(&conn->addr, conn->port)) {
+          /* Only negotiate methods that have nonempty parameters. */
+          buf[1] = 2;
+          buf[2] = 0x02; /* RFC1929 - params in username/password */
+          buf[3] = 0x80; /* PT2 JSON - params as JSON object */
+        } else {
+          /* Only negotiate parameterless method. */
+          buf[1] = 1;
+          buf[2] = 0x00; /* none - parameterless */
+        }
+
+        conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_PT2;
       } else {
         buf[1] = 1;
         buf[2] = 0x00; /* no authentication */
-- 
2.14.0


From 6a740b2ba00a1a35ad3c36292f1f307d2c2fa7d6 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Mon, 24 Jul 2017 20:43:48 -0500
Subject: [PATCH 07/12] PT2.0: Remove failing test for huge bridge lines

The point of the JSON parameter block support is to allow these to
succeed in PT2 mode, of course, so making sure they fail at the
config stage is contradictory.
---
 src/test/test_config.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/src/test/test_config.c b/src/test/test_config.c
index 6b875a0a0..9c0c1289d 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -501,17 +501,6 @@ test_config_parse_bridge_line(void *arg)
                        "4352e58420e68f5e40bf7c74faddccd9d1349413 what");
   /* no addrport */
   bad_bridge_line_test("asdw");
-  /* huge k=v value that can't fit in SOCKS fields */
-  bad_bridge_line_test(
-           "obfs2 2.2.2.2:1231 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-           "aa=b");
 }
 
 static void
-- 
2.14.0


From d3d89d525ab22819da08f768fd67037b7b6eb780 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Thu, 3 Aug 2017 11:19:47 -0500
Subject: [PATCH 08/12] PT2.0: Add version 2 to configuration negotiation

This still hardcodes both "1" and "2" as acceptable versions in
two places, but there are comments to keep them in sync. If we
ever need more than this, we should probably switch to using a
table, but that still seems far off.
---
 src/or/transports.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/or/transports.c b/src/or/transports.c
index 18a1d9836..4b72d341f 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -125,9 +125,9 @@ static void parse_method_error(const char *line, int is_server_method);
 #define PROTO_PROXY_DONE "PROXY DONE"
 #define PROTO_PROXY_ERROR "PROXY-ERROR"
 
-/** The first and only supported - at the moment - configuration
-    protocol version. */
+/** Currently supported configuration protocol versions. */
 #define PROTO_VERSION_ONE 1
+#define PROTO_VERSION_TWO 2
 
 /** A list of pluggable transports found in torrc. */
 static smartlist_t *transport_list = NULL;
@@ -995,13 +995,21 @@ parse_version(const char *line, managed_proxy_t *mp)
     return -1;
   }
 
-  if (strcmp("1", line+strlen(PROTO_NEG_SUCCESS)+1)) { /* hardcoded temp */
+  const char *version = line+strlen(PROTO_NEG_SUCCESS)+1;
+
+  /* Keep this in sync with the exported value of TOR_PT_MANAGED_TRANSPORT_VER
+     below, and with the log_warn call. If we get any more configuration
+     versions than this, this should probably be moved to a constant table. */
+  if (!strcmp("1", version)) {
+    mp->conf_protocol = PROTO_VERSION_ONE;
+  } else if (!strcmp("2", version)) {
+    mp->conf_protocol = PROTO_VERSION_TWO;
+  } else {
     log_warn(LD_CONFIG, "Managed proxy tried to negotiate on version '%s'. "
-             "We only support version '1'", line+strlen(PROTO_NEG_SUCCESS)+1);
+             "But we only support versions '1' and '2'", version);
     return -1;
   }
 
-  mp->conf_protocol = PROTO_VERSION_ONE; /* temp. till more versions appear */
   return 0;
 }
 
@@ -1321,7 +1329,8 @@ create_managed_proxy_environment(const managed_proxy_t *mp)
     tor_free(state_tmp);
   }
 
-  smartlist_add_strdup(envs, "TOR_PT_MANAGED_TRANSPORT_VER=1");
+  /* Keep this in sync with the string-switch in parse_version above. */
+  smartlist_add_strdup(envs, "TOR_PT_MANAGED_TRANSPORT_VER=1,2");
 
   {
     char *transports_to_launch =
-- 
2.14.0


From 844fc3d69359537df14e4ec7560a3c8d01638201 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Sun, 20 Aug 2017 07:39:44 -0500
Subject: [PATCH 09/12] PT2.0: Fail if we have PT parameters and can't send
 them

This should be safer than proceeding blindly.
---
 src/or/connection.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/or/connection.c b/src/or/connection.c
index 798b2968f..b8fb96e16 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2498,10 +2498,15 @@ connection_read_proxy_handshake(connection_t *conn)
 
       /* send auth if needed, otherwise do connect */
       if (ret == 1) {
-        /* TODO: this doesn't check whether we had pending PT parameters like
-           the next case does. Should it? */
-        connection_send_socks5_connect(conn);
-        ret = 0;
+        if (get_proxy_type() == PROXY_PLUGGABLE
+            && get_socks_args_by_bridge_addrport(&conn->addr, conn->port)) {
+          log_warn(LD_NET, "We have PT parameters but server refused them.");
+          ret = -1;
+          break;
+        } else {
+          connection_send_socks5_connect(conn);
+          ret = 0;
+        }
       } else if (ret == 2) {
         unsigned char buf[MAX_RFC1929_ENCODED_SIZE];
         ssize_t reqsize;
-- 
2.14.0


From 77c423a525d4b650d0368c6244f6075be6075548 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Sun, 20 Aug 2017 10:08:32 -0500
Subject: [PATCH 10/12] PT2.0: Add test for conf proto 2 before negotiating
 method 0x80

Since this method number is in the private use block, we need to
know that the other SOCKS 5 endpoint has already agreed to
interpret it as PT2 JSON Parameter Block pseudo-authentication
rather than something else.

Thus, we add a mechanism for testing transport feature flags from
places that aren't transport.c. (The original mechanism mentioned in
ticket #21816 implied exposing the managed_proxy_t structure, which
seemed a bit much.)
---
 src/or/connection.c | 36 +++++++++++++++++++++++++++++++-----
 src/or/or.h         | 12 +++++-------
 src/or/transports.c | 37 +++++++++++++++++++++++++++++++++++++
 src/or/transports.h |  7 +++++++
 4 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/src/or/connection.c b/src/or/connection.c
index b8fb96e16..8248b5d1e 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2088,6 +2088,22 @@ get_proxy_type(void)
     return PROXY_NONE;
 }
 
+/** Given the <b>addr</b> and <b>port</b> of a bridge, return 1 if that
+ *  bridge uses a pluggable transport for which SOCKS 5 connections
+ *  should use PT2 JSON Parameter Block pseudo-authentication. Otherwise,
+ *  return 0. */
+static int
+pt_socks5_auth_is_pt2(const tor_addr_t *addr, uint16_t port)
+{
+  const char *transport_name =
+    find_transport_name_by_bridge_addrport(addr, port);
+  if (!transport_name)
+    return 0;
+
+  return !!pt_test_features_by_transport_name(transport_name,
+                                              PT_FEATURE_SOCKS5_PT2_AUTH);
+}
+
 /* One byte for the version, one for the command, two for the
    port, and four for the addr... and, one more for the
    username NUL: */
@@ -2235,19 +2251,29 @@ connection_proxy_connect(connection_t *conn, int type)
       } else if (get_proxy_type() == PROXY_PLUGGABLE) {
         if (get_socks_args_by_bridge_addrport(&conn->addr, conn->port)) {
           /* Only negotiate methods that have nonempty parameters. */
-          buf[1] = 2;
-          buf[2] = 0x02; /* RFC1929 - params in username/password */
-          buf[3] = 0x80; /* PT2 JSON - params as JSON object */
+          if (pt_socks5_auth_is_pt2(&conn->addr, conn->port)) {
+            buf[1] = 2;
+            buf[2] = 0x80; /* PT2 JSON - params as JSON object */
+            buf[3] = 0x02; /* RFC1929 - params in username/password */
+
+            conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_PT2;
+          } else {
+            buf[1] = 1;
+            buf[2] = 0x02; /* RFC1929 - params in username/password */
+
+            conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929;
+          }
         } else {
           /* Only negotiate parameterless method. */
           buf[1] = 1;
           buf[2] = 0x00; /* none - parameterless */
-        }
 
-        conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_PT2;
+          conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_NONE;
+        }
       } else {
         buf[1] = 1;
         buf[2] = 0x00; /* no authentication */
+
         conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_NONE;
       }
 
diff --git a/src/or/or.h b/src/or/or.h
index c660908f0..4b61de222 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -255,10 +255,9 @@ typedef enum {
 #define PROXY_SOCKS5_WANT_AUTH_METHOD_NONE 4
 /* We use a SOCKS5 proxy and we try to negotiate with
    Username/Password authentication.  This can be either true
-   authentication or PT1 parameter-passing pseudo-authentication, but
-   in the latter case we will usually allow PT2 parameter blocks as
-   well and will be in the PROXY_SOCKS5_WANT_AUTH_METHOD_PT2 state
-   instead. */
+   authentication or PT1 parameter-passing pseudo-authentication.  If
+   we are allowing PT2 JSON parameter blocks as well, we will be in
+   PROXY_SOCKS5_WANT_AUTH_METHOD_PT2 instead. */
 #define PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929 5
 /* We use a SOCKS5 proxy and we just sent our credentials. */
 #define PROXY_SOCKS5_WANT_AUTH_RFC1929_OK 6
@@ -268,9 +267,8 @@ typedef enum {
 #define PROXY_CONNECTED 8
 /* We use a SOCKS5 proxy and we try to negotiate with PT2
    parameter-block pseudo-authentication and/or PT1 Username/Password
-   pseudo-authentication.  In the strange event that we are only
-   willing to provide PT1, we will wind up in
-   PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929 instead. */
+   pseudo-authentication.  If we are only willing to provide PT1, we
+   will wind up in PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929 instead. */
 #define PROXY_SOCKS5_WANT_AUTH_METHOD_PT2 9
 
 #define PROXY_STATE_MAX_ 9
diff --git a/src/or/transports.c b/src/or/transports.c
index 4b72d341f..7f630e3cf 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -424,6 +424,43 @@ get_managed_proxy_by_argv_and_type(char **proxy_argv, int is_server)
   return NULL;
 }
 
+/** Return a managed proxy which has launched the transport
+ *  <b>transport_name</b>. If no such managed proxy exists, return
+ *  NULL. */
+static managed_proxy_t *
+get_managed_proxy_by_transport_name(const char *transport_name)
+{
+  if (!managed_proxy_list)
+    return NULL;
+
+  SMARTLIST_FOREACH_BEGIN(managed_proxy_list, managed_proxy_t *, mp) {
+    if (smartlist_contains_string(mp->transports, transport_name))
+      return mp;
+  } SMARTLIST_FOREACH_END(mp);
+
+  return NULL;
+}
+
+/** Return a bitmask which, for each bit in <b>features</b>, has the
+ *  corresponding bit set if and only if that feature is applicable to
+ *  the transport <b>transport_name</b>. Valid features are defined as
+ *  PT_FEATURE constants in transports.h.
+ */
+unsigned
+pt_test_features_by_transport_name(const char *transport_name,
+                                   unsigned features)
+{
+  managed_proxy_t *mp = get_managed_proxy_by_transport_name(transport_name);
+  unsigned result = 0;
+
+  if (features & PT_FEATURE_SOCKS5_PT2_AUTH) {
+    if (mp != NULL && mp->conf_protocol >= 2)
+      result |= PT_FEATURE_SOCKS5_PT2_AUTH;
+  }
+
+  return result;
+}
+
 /** Add <b>transport</b> to managed proxy <b>mp</b>. */
 static void
 add_transport_to_proxy(const char *transport, managed_proxy_t *mp)
diff --git a/src/or/transports.h b/src/or/transports.h
index c90ff34dd..f55f8361d 100644
--- a/src/or/transports.h
+++ b/src/or/transports.h
@@ -68,6 +68,13 @@ char *pt_get_socks_args_for_proxy_addrport(const tor_addr_t *addr,
 char *pt_get_json_object_for_proxy_addrport(const tor_addr_t *addr,
                                             uint16_t port);
 
+/** SOCKS5 connections via this transport name should use PT2
+ *  JSON Parameter Block pseudo-authentication */
+#define PT_FEATURE_SOCKS5_PT2_AUTH (1u<<0)
+
+unsigned pt_test_features_by_transport_name(const char *transport_name,
+                                            unsigned features);
+
 #ifdef PT_PRIVATE
 /** State of the managed proxy configuration protocol. */
 enum pt_proto_state {
-- 
2.14.0


From e0a4fdda55e272ff04de42a31fe63ff5a669eaed Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Sun, 20 Aug 2017 10:49:43 -0500
Subject: [PATCH 11/12] PT2.0: Use symbolic constants for SOCKS methods

There were already some present in or.h, so add the PT2 one to that
set and change the outbound SOCKS method negotiations to use them
instead of hex literals.
---
 src/or/buffers.c    | 13 ++++++-------
 src/or/connection.c | 14 +++++++-------
 src/or/or.h         |  1 +
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/or/buffers.c b/src/or/buffers.c
index f2f6f5ab5..9b1d0ebb0 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1934,7 +1934,7 @@ parse_socks_client(const uint8_t *data, size_t datalen,
 
     case PROXY_SOCKS5_WANT_AUTH_METHOD_NONE:
       /* we don't have any credentials */
-      if (data[1] != 0x00) {
+      if (data[1] != SOCKS_NO_AUTH) {
         *reason = tor_strdup("server doesn't support any of our "
                              "available authentication methods");
         return -1;
@@ -1948,12 +1948,12 @@ parse_socks_client(const uint8_t *data, size_t datalen,
       /* we have a username and password. return 1 if we can proceed without
        * providing authentication, or 2 otherwise. */
       switch (data[1]) {
-        case 0x00:
+        case SOCKS_NO_AUTH:
           log_info(LD_NET, "SOCKS 5 client: we have auth details but server "
                             "doesn't require authentication.");
           *drain_out = -1;
           return 1;
-        case 0x02:
+        case SOCKS_USER_PASS:
           log_info(LD_NET, "SOCKS 5 client: need authentication.");
           *drain_out = -1;
           return 2;
@@ -1968,7 +1968,7 @@ parse_socks_client(const uint8_t *data, size_t datalen,
       /* We have PT parameters. Return 2 if we can proceed with PT1-style
        * user/pass authentication, or 3 for PT2-style parameter-block. */
       switch (data[1]) {
-        case 0x00:
+        case SOCKS_NO_AUTH:
           /* If the server (which is a PT client) returns "no
            * authentication", this could actually be quite bad if we
            * have parameters, but that's handled in
@@ -1977,14 +1977,13 @@ parse_socks_client(const uint8_t *data, size_t datalen,
           *drain_out = -1;
           return 1;
 
-        case 0x02:
+        case SOCKS_USER_PASS:
           log_info(LD_NET, "SOCKS 5 client: using username/password "
                            "pseudo-authentication for PT parameters.");
           *drain_out = -1;
           return 2;
 
-          /* TODO: symbolic constants for SOCKS5 auth method numbers */
-        case 0x80:
+        case SOCKS_PRIVATE_PT2_JSON:
           log_info(LD_NET, "SOCKS 5 client: using PT2 pseudo-"
                            "authentication for PT parameters.");
           *drain_out = -1;
diff --git a/src/or/connection.c b/src/or/connection.c
index 8248b5d1e..cb9addf9b 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2244,8 +2244,8 @@ connection_proxy_connect(connection_t *conn, int type)
          pluggable transport proxy: */
       if (options->Socks5ProxyUsername) {
         buf[1] = 2;
-        buf[2] = 0x00; /* none - credentials weren't needed */
-        buf[3] = 0x02; /* RFC1929 - use username/password */
+        buf[2] = SOCKS_NO_AUTH;
+        buf[3] = SOCKS_USER_PASS;
 
         conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929;
       } else if (get_proxy_type() == PROXY_PLUGGABLE) {
@@ -2253,26 +2253,26 @@ connection_proxy_connect(connection_t *conn, int type)
           /* Only negotiate methods that have nonempty parameters. */
           if (pt_socks5_auth_is_pt2(&conn->addr, conn->port)) {
             buf[1] = 2;
-            buf[2] = 0x80; /* PT2 JSON - params as JSON object */
-            buf[3] = 0x02; /* RFC1929 - params in username/password */
+            buf[2] = SOCKS_PRIVATE_PT2_JSON;
+            buf[3] = SOCKS_USER_PASS;
 
             conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_PT2;
           } else {
             buf[1] = 1;
-            buf[2] = 0x02; /* RFC1929 - params in username/password */
+            buf[2] = SOCKS_USER_PASS;
 
             conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_RFC1929;
           }
         } else {
           /* Only negotiate parameterless method. */
           buf[1] = 1;
-          buf[2] = 0x00; /* none - parameterless */
+          buf[2] = SOCKS_NO_AUTH;
 
           conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_NONE;
         }
       } else {
         buf[1] = 1;
-        buf[2] = 0x00; /* no authentication */
+        buf[2] = SOCKS_NO_AUTH;
 
         conn->proxy_state = PROXY_SOCKS5_WANT_AUTH_METHOD_NONE;
       }
diff --git a/src/or/or.h b/src/or/or.h
index 4b61de222..2c26c33f9 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4696,6 +4696,7 @@ static inline void or_state_mark_dirty(or_state_t *state, time_t when)
 #define MAX_SOCKS_ADDR_LEN 256
 #define SOCKS_NO_AUTH 0x00
 #define SOCKS_USER_PASS 0x02
+#define SOCKS_PRIVATE_PT2_JSON 0x80
 
 /** Please open a TCP connection to this addr:port. */
 #define SOCKS_COMMAND_CONNECT       0x01
-- 
2.14.0


From 887352263569f9bec34befd261b5714f6f30be17 Mon Sep 17 00:00:00 2001
From: Robin Tarsiger <rtt@xxxxxxxxxxxxxx>
Date: Sun, 20 Aug 2017 15:06:46 -0500
Subject: [PATCH 12/12] PT2.0: Fix whitespace

---
 src/or/buffers.c    | 1 +
 src/or/connection.c | 4 +++-
 src/or/transports.c | 3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/or/buffers.c b/src/or/buffers.c
index 9b1d0ebb0..690fdf8e2 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -2247,3 +2247,4 @@ assert_buf_ok(buf_t *buf)
     tor_assert(buf->datalen == total);
   }
 }
+
diff --git a/src/or/connection.c b/src/or/connection.c
index cb9addf9b..93cec1f3b 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -2562,7 +2562,8 @@ connection_read_proxy_handshake(connection_t *conn)
           if (reqsize < 0) {
             /* This shouldn't happen because we already satisfied the
              * precondition above. */
-            log_err(LD_BUG, "Could not encode PT parameters despite precheck!");
+            log_err(LD_BUG,
+                    "Could not encode PT parameters despite precheck!");
             tor_fragile_assert();
             tor_free(socks_args_string);
             ret = -1;
@@ -5437,3 +5438,4 @@ clock_skew_warning(const connection_t *conn, long apparent_skew, int trusted,
                                  apparent_skew, ext_source);
   tor_free(ext_source);
 }
+
diff --git a/src/or/transports.c b/src/or/transports.c
index 7f630e3cf..ac6ceefca 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -1838,7 +1838,7 @@ pt_jsonify_socks_args(const smartlist_t *socks_args)
   tor_asprintf(&result_json, "{%s}", joined_members);
   tor_free(joined_members);
 
-done:
+ done:
   SMARTLIST_FOREACH(sl_members, char *, s, tor_free(s));
   smartlist_free(sl_members);
 
@@ -1905,3 +1905,4 @@ pt_free_all(void)
     managed_proxy_list=NULL;
   }
 }
+
-- 
2.14.0


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
tor-dev mailing list
tor-dev@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev