From 7daffcbba8d23e278545830d13bf45c0f61cfc39 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 24 Sep 2021 18:31:39 +0800 Subject: [PATCH] tor: add a new response reader for tor controller This commit adds a new response reader which replaces the old textproto.Reader.ReadResponse. The older reader cannot handle the case when the reply from Tor server contains a data reply line, which uses the symbol "+" to signal such a case. --- tor/controller.go | 132 ++++++++++++++++++++++++-- tor/controller_test.go | 204 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 329 insertions(+), 7 deletions(-) diff --git a/tor/controller.go b/tor/controller.go index f2120a92c..c401cb0bf 100644 --- a/tor/controller.go +++ b/tor/controller.go @@ -65,6 +65,10 @@ var ( // message from the controller. controllerKey = []byte("Tor safe cookie authentication " + "controller-to-server hash") + + // errCodeNotMatch is used when an expected response code is not + // returned. + errCodeNotMatch = errors.New("unexpected code") ) // Controller is an implementation of the Tor Control protocol. This is used in @@ -168,16 +172,17 @@ func (c *Controller) Stop() error { // sendCommand sends a command to the Tor server and returns its response, as a // single space-delimited string, and code. func (c *Controller) sendCommand(command string) (int, string, error) { - if err := c.conn.Writer.PrintfLine(command); err != nil { + id, err := c.conn.Cmd(command) + if err != nil { return 0, "", err } - // We'll use ReadResponse as it has built-in support for multi-line - // text protocol responses. - code, reply, err := c.conn.Reader.ReadResponse(success) - log.Tracef("sendCommand: %v got , ", - command, code, reply) + // Make sure our reader only process the response returned from the + // above command. + c.conn.StartResponse(id) + defer c.conn.EndResponse(id) + code, reply, err := c.readResponse(success) if err != nil { log.Debugf("sendCommand:%s got err:%v, reply:%v", command, err, reply) @@ -187,6 +192,119 @@ func (c *Controller) sendCommand(command string) (int, string, error) { return code, reply, nil } +// readResponse reads the replies from Tor to the controller. The reply has the +// following format, +// +// Reply = SyncReply / AsyncReply +// SyncReply = *(MidReplyLine / DataReplyLine) EndReplyLine +// AsyncReply = *(MidReplyLine / DataReplyLine) EndReplyLine +// +// MidReplyLine = StatusCode "-" ReplyLine +// DataReplyLine = StatusCode "+" ReplyLine CmdData +// EndReplyLine = StatusCode SP ReplyLine +// ReplyLine = [ReplyText] CRLF +// ReplyText = XXXX +// StatusCode = 3DIGIT +// +// Unless specified otherwise, multiple lines in a single reply from Tor daemon +// to the controller are guaranteed to share the same status code. Read more on +// this topic: +// https://gitweb.torproject.org/torspec.git/tree/control-spec.txt#n158 +// +// NOTE: this code is influenced by https://github.com/Yawning/bulb. +func (c *Controller) readResponse(expected int) (int, string, error) { + // Clean the buffer inside the conn. This is needed when we encountered + // an error while reading the response, the remaining lines need to be + // cleaned before next read. + defer func() { + if _, err := c.conn.R.Discard(c.conn.R.Buffered()); err != nil { + log.Errorf("clean read buffer failed: %v", err) + } + }() + + reply, code := "", 0 + hasMoreLines := true + + for hasMoreLines { + line, err := c.conn.Reader.ReadLine() + if err != nil { + return 0, reply, err + } + log.Tracef("Reading line: %v", line) + + // Line being shortter than 4 is not allowed. + if len(line) < 4 { + err = textproto.ProtocolError("short line: " + line) + return 0, reply, err + } + + // Parse the status code. + code, err = strconv.Atoi(line[0:3]) + if err != nil { + return code, reply, err + } + + switch line[3] { + // EndReplyLine = StatusCode SP ReplyLine. + // Example: 250 OK + // This is the end of the response, so we mark hasMoreLines to + // be false to exit the loop. + case ' ': + reply += line[4:] + hasMoreLines = false + + // MidReplyLine = StatusCode "-" ReplyLine. + // Example: 250-version=... + // This is a continued response, so we keep reading the next + // line. + case '-': + reply += line[4:] + + // DataReplyLine = StatusCode "+" ReplyLine CmdData. + // Example: 250+config-text= + // line1 + // line2 + // more lines... + // . + // This is a data response, meaning the following multiple + // lines are the actual data, and a dot(.) in the end means the + // end of the data response. The response will be formatted as, + // key=line1,line2,... + // The above example will then be, + // config-text=line1,line2,... + case '+': + // Add the key(config-text=) + reply += line[4:] + + // Add the values. + resp, err := c.conn.Reader.ReadDotLines() + if err != nil { + return code, reply, err + } + reply += strings.Join(resp, ",") + + // Invalid line separator found. + default: + err = textproto.ProtocolError("invalid line: " + line) + return code, reply, err + } + + // We check the code here so that the error message is parsed + // from the line. + if code != expected { + return code, reply, errCodeNotMatch + } + + // Separate each line using "\n". + if hasMoreLines { + reply += "\n" + } + } + + log.Tracef("Parsed reply: %v", reply) + return code, reply, nil +} + // parseTorReply parses the reply from the Tor server after receiving a command // from a controller. This will parse the relevant reply parameters into a map // of keys and values. @@ -225,6 +343,8 @@ func (c *Controller) authenticate() error { return err } + log.Debugf("received protocol info: %v", protocolInfo) + // With the version retrieved, we'll cache it now in case it needs to be // used later on. c.version = protocolInfo.version() diff --git a/tor/controller_test.go b/tor/controller_test.go index b040a4f9d..00926a3d8 100644 --- a/tor/controller_test.go +++ b/tor/controller_test.go @@ -1,6 +1,12 @@ package tor -import "testing" +import ( + "net" + "net/textproto" + "testing" + + "github.com/stretchr/testify/require" +) // TestParseTorVersion is a series of tests for different version strings that // check the correctness of determining whether they support creating v3 onion @@ -74,3 +80,199 @@ func TestParseTorVersion(t *testing.T) { } } } + +// testProxy emulates a Tor daemon and contains the info used for the tor +// controller to make connections. +type testProxy struct { + // server is the proxy listener. + server net.Listener + + // serverConn is the established connection from the server side. + serverConn net.Conn + + // serverAddr is the tcp address the proxy is listening on. + serverAddr string + + // clientConn is the established connection from the client side. + clientConn *textproto.Conn +} + +// cleanUp is used after each test to properly close the ports/connections. +func (tp *testProxy) cleanUp() { + // Don't bother cleanning if there's no a server created. + if tp.server == nil { + return + } + + if err := tp.clientConn.Close(); err != nil { + log.Errorf("closing client conn got err: %v", err) + } + if err := tp.server.Close(); err != nil { + log.Errorf("closing proxy server got err: %v", err) + } +} + +// createTestProxy creates a proxy server to listen on a random address, +// creates a server and a client connection, and initializes a testProxy using +// these params. +func createTestProxy(t *testing.T) *testProxy { + // Set up the proxy to listen on given port. + // + // NOTE: we use a port 0 here to indicate we want a free port selected + // by the system. + proxy, err := net.Listen("tcp", ":0") + require.NoError(t, err, "failed to create proxy") + + t.Logf("created proxy server to listen on address: %v", proxy.Addr()) + + // Accept the connection inside a goroutine. + serverChan := make(chan net.Conn, 1) + go func(result chan net.Conn) { + conn, err := proxy.Accept() + require.NoError(t, err, "failed to accept") + + result <- conn + }(serverChan) + + // Create the connection using tor controller. + client, err := textproto.Dial("tcp", proxy.Addr().String()) + require.NoError(t, err, "failed to create connection") + + tc := &testProxy{ + server: proxy, + serverConn: <-serverChan, + serverAddr: proxy.Addr().String(), + clientConn: client, + } + + return tc +} + +// TestReadResponse contructs a series of possible responses returned by Tor +// and asserts the readResponse can handle them correctly. +func TestReadResponse(t *testing.T) { + // Create mock server and client connection. + proxy := createTestProxy(t) + defer proxy.cleanUp() + server := proxy.serverConn + + // Create a dummy tor controller. + c := &Controller{conn: proxy.clientConn} + + testCase := []struct { + name string + serverResp string + + // expectedReply is the reply we expect the readResponse to + // return. + expectedReply string + + // expectedCode is the code we expect the server to return. + expectedCode int + + // returnedCode is the code we expect the readResponse to + // return. + returnedCode int + + // expectedErr is the error we expect the readResponse to + // return. + expectedErr error + }{ + { + // Test a simple response. + name: "succeed on 250", + serverResp: "250 OK\n", + expectedReply: "OK", + expectedCode: 250, + returnedCode: 250, + expectedErr: nil, + }, + { + // Test a mid reply(-) response. + name: "succeed on mid reply line", + serverResp: "250-field=value\n" + + "250 OK\n", + expectedReply: "field=value\nOK", + expectedCode: 250, + returnedCode: 250, + expectedErr: nil, + }, + { + // Test a data reply(+) response. + name: "succeed on data reply line", + serverResp: "250+field=\n" + + "line1\n" + + "line2\n" + + ".\n" + + "250 OK\n", + expectedReply: "field=line1,line2\nOK", + expectedCode: 250, + returnedCode: 250, + expectedErr: nil, + }, + { + // Test a mixed reply response. + name: "succeed on mixed reply line", + serverResp: "250-field=value\n" + + "250+field=\n" + + "line1\n" + + "line2\n" + + ".\n" + + "250 OK\n", + expectedReply: "field=value\nfield=line1,line2\nOK", + expectedCode: 250, + returnedCode: 250, + expectedErr: nil, + }, + { + // Test unexpected code. + name: "fail on codes not matched", + serverResp: "250 ERR\n", + expectedReply: "ERR", + expectedCode: 500, + returnedCode: 250, + expectedErr: errCodeNotMatch, + }, + { + // Test short response error. + name: "fail on short response", + serverResp: "123\n250 OK\n", + expectedReply: "", + expectedCode: 250, + returnedCode: 0, + expectedErr: textproto.ProtocolError( + "short line: 123"), + }, + { + // Test short response error. + name: "fail on invalid response", + serverResp: "250?OK\n", + expectedReply: "", + expectedCode: 250, + returnedCode: 250, + expectedErr: textproto.ProtocolError( + "invalid line: 250?OK"), + }, + } + + for _, tc := range testCase { + tc := tc + t.Run(tc.name, func(t *testing.T) { + + // Let the server mocks a given response. + _, err := server.Write([]byte(tc.serverResp)) + require.NoError(t, err, "server failed to write") + + // Read the response and checks all expectations + // satisfied. + code, reply, err := c.readResponse(tc.expectedCode) + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.returnedCode, code) + require.Equal(t, tc.expectedReply, reply) + + // Check that the read buffer is cleaned. + require.Zero(t, c.conn.R.Buffered(), + "read buffer not empty") + }) + } +}