diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml new file mode 100644 index 0000000..4048cbf --- /dev/null +++ b/.github/workflows/go.yaml @@ -0,0 +1,40 @@ +name: Go + +on: + push: + branches: + - master + pull_request: + +jobs: + test: + name: Test + strategy: + matrix: + go_version: + - 1.12 + - 1.13 + - 1.14 + - 1.15 + - 1.16 + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Checkout repo + uses: actions/checkout@v2 + with: + submodules: recursive + + - name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: ${{ matrix.go_version }} + + - name: Build FTP test servers + run: ./build_test_server.sh + + - name: Test Go code + run: go test -v ./... + + - name: Test with race detector + run: go test -v -race ./... diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 21fef4e..0000000 --- a/.travis.yml +++ /dev/null @@ -1,11 +0,0 @@ -language: go - -go: - - "1.10" - - tip - -install: - - ./build_test_server.sh - -before_script: - - echo 0 | sudo tee /proc/sys/net/ipv6/conf/all/disable_ipv6 diff --git a/build_test_server.sh b/build_test_server.sh index 53c4f06..05e0376 100755 --- a/build_test_server.sh +++ b/build_test_server.sh @@ -1,5 +1,10 @@ #!/bin/bash -ex +if [ "$(uname)" == "Darwin" ]; then + cflags=-I/usr/local/opt/openssl/include + ldflags=-L/usr/local/opt/openssl/lib +fi + goftp_dir=`pwd` mkdir -p ftpd @@ -7,20 +12,11 @@ cd ftpd ftpd_dir=`pwd` -curl -O ftp://ftp.proftpd.org/distrib/source/proftpd-1.3.5.tar.gz -tar -xzf proftpd-1.3.5.tar.gz -cd proftpd-1.3.5 - -# fix slow tls data connection handshake (https://github.com/proftpd/proftpd/pull/48) -perl -pi -e 's/(\Qpr_inet_set_proto_nodelay(conn->pool, conn, 1);\E)/$1\n(void) pr_inet_set_proto_cork(conn->wfd, 0);/' contrib/mod_tls.c +proftp_version='1.3.7a' -# fix a segfault on mac -perl -pi -e 's/\Qsstrncpy(cwd, dir, sizeof(cwd));\E/char dircpy[sizeof(cwd)];\nsstrncpy(dircpy, dir, sizeof(dircpy));\nsstrncpy(cwd, dircpy, sizeof(cwd));/' src/fsio.c - -if [ "$(uname)" == "Darwin" ]; then - cflags=-I/usr/local/opt/openssl/include - ldflags=-L/usr/local/opt/openssl/lib -fi +test -f proftpd-${proftp_version}.tar.gz || curl -O ftp://ftp.proftpd.org/distrib/source/proftpd-${proftp_version}.tar.gz +tar -xzf proftpd-${proftp_version}.tar.gz +cd proftpd-${proftp_version} CFLAGS=$cflags LDFLAGS=$ldflags ./configure --with-modules=mod_tls --disable-ident @@ -51,46 +47,15 @@ TLSOptions NoSessionReuseRequired CONF -cat > server.key <<"KEY" ------BEGIN RSA PRIVATE KEY----- -MIICWwIBAAKBgQDOMHBkxoVgenJqLimeIkztEE9Hp3XcIE7cmZILqkMDuo0kGAVU -Mvldyo+sYqop46aPbobiqPxU3knyrHJJ2H0ucFnb67kUH5ITncYo8iNephtgtuMR -D0JKYneaGtJ0Z+kTWIJV3/9f2GFLK8InY7ipoxZX3hGkSeIVyh6F66CZ5QIDAQAB -AoGAfuC/yMOAf4XZsg0F/xEMVTScFHOvyuz2mjjF7fevlTPOdk9xuAZF/LkQ//sW -ywATFl/lEMT7wR2oU3RaP6bAICCf1vGba51U+yFTS/8T1+VJvRuojMX4RVJhaFU4 -HJx9Fd9a8kLzHTaBkaVtGJzK3GxXpa7mwSlJ+Eeeh+CYYAECQQDq4Mb1tgcEOWFB -v9j9StMby+0FF8uitx5+E78r3xPRRjIjgray6UuFCc41U/pPuw2iqreDCkjLKb5G -SIqQ7BrlAkEA4Ls0cAejrONvO/1+NIeNQn30WIgH7zqBmRYJnLqw8xo0xML51pFU -gXYCEp+M2AtDL6yQ0zSN7D2JhtbzLweTAQJAf7rteAIdnrZ1pYPnRRfD5oHny7U9 -EKf09StX80vFQzGhYp5bLMCiSR8j/OxGW8WljKi6U5DsNU/mIeKhOF6t4QJAfUAZ -E69OS9decYLwyfoagsqMWqNGONDU1itwJAfxAyzB6D/62tmYzaaltRdzeh2czn9R -IEWUK+yIL7yxQK7qAQJABD3UYT2yZaZJDNerK9h9RJRptN1f0vJ29qURkj7OpFbY -2hgLZ/lfrSE5RCSmYQtmk2hyuSzMSe928k85Y14+wg== ------END RSA PRIVATE KEY----- -KEY - -cat > server.cert <<"CERT" ------BEGIN CERTIFICATE----- -MIICdzCCAeCgAwIBAgIJAJls3dJsiITyMA0GCSqGSIb3DQEBBQUAMDIxCzAJBgNV -BAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMQ4wDAYDVQQKEwVHb0ZUUDAeFw0x -NTAyMTYwNTQ0MDhaFw0xNTAzMTgwNTQ0MDhaMDIxCzAJBgNVBAYTAlVTMRMwEQYD -VQQIEwpDYWxpZm9ybmlhMQ4wDAYDVQQKEwVHb0ZUUDCBnzANBgkqhkiG9w0BAQEF -AAOBjQAwgYkCgYEAzjBwZMaFYHpyai4pniJM7RBPR6d13CBO3JmSC6pDA7qNJBgF -VDL5XcqPrGKqKeOmj26G4qj8VN5J8qxySdh9LnBZ2+u5FB+SE53GKPIjXqYbYLbj -EQ9CSmJ3mhrSdGfpE1iCVd//X9hhSyvCJ2O4qaMWV94RpEniFcoeheugmeUCAwEA -AaOBlDCBkTAdBgNVHQ4EFgQUb/FNa79J/POe13rCUSA3eSJviGgwYgYDVR0jBFsw -WYAUb/FNa79J/POe13rCUSA3eSJviGihNqQ0MDIxCzAJBgNVBAYTAlVTMRMwEQYD -VQQIEwpDYWxpZm9ybmlhMQ4wDAYDVQQKEwVHb0ZUUIIJAJls3dJsiITyMAwGA1Ud -EwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADgYEAhUH0UnU46s2XbAGq6RpmKuONjgJX -X4qKrpmBhSg6KS4WkgnLr8+YrvvcFhhPGf9xLpCS1o+RC0W6BuwqiAtM+ckqDnI5 -pb3vMhAhXTjg1bLWDNFn98iI5tSqGSjy9d7RfdC2yyFQsliq2b74yHxCOysC5OW0 -VpOorURz8ETlfAA= ------END CERTIFICATE----- -CERT - -curl -O https://download.pureftpd.org/pub/pure-ftpd/releases/obsolete/pure-ftpd-1.0.36.tar.gz -tar -xzf pure-ftpd-1.0.36.tar.gz -cd pure-ftpd-1.0.36 +# generate a key and certificate +openssl req -x509 -nodes -newkey rsa:4096 -keyout server.key -out server.cert -days 365 -subj '/C=US/ST=California/O=GoFTP' +cat server.key server.cert > pure-ftpd.pem + +pure_ftpd_version='1.0.49' + +test -f pure-ftpd-${pure_ftpd_version}.tar.gz || curl -O https://download.pureftpd.org/pub/pure-ftpd/releases/pure-ftpd-${pure_ftpd_version}.tar.gz +tar -xzf pure-ftpd-${pure_ftpd_version}.tar.gz +cd pure-ftpd-${pure_ftpd_version} # build normal binary with explicit tls support CFLAGS=$cflags LDFLAGS=$ldflags ./configure --with-nonroot --with-puredb --with-tls --with-certfile=$ftpd_dir/pure-ftpd.pem @@ -106,8 +71,6 @@ mv src/pure-ftpd ../pure-ftpd-implicittls cd .. -cat server.key server.cert > pure-ftpd.pem - # setup up a goftp user for ftp server if [ "$(uname)" == "Darwin" ]; then echo "goftp:_.../HVM0l1lcNKVtiKs:`id -u`:`id -g`::$goftp_dir/testroot/./::::::::::::" > users.txt @@ -121,4 +84,4 @@ fi chmod 600 users.txt # generate puredb user db file -pure-ftpd-1.0.36/src/pure-pw mkdb users.pdb -f users.txt +pure-ftpd-${pure_ftpd_version}/src/pure-pw mkdb users.pdb -f users.txt diff --git a/client.go b/client.go index 779cd5e..53c683b 100644 --- a/client.go +++ b/client.go @@ -5,6 +5,7 @@ package goftp import ( + "context" "crypto/tls" "errors" "fmt" @@ -109,6 +110,10 @@ type Config struct { // of data transfers. Defaults to 5 seconds. Timeout time.Duration + // DialContext specifies the dial function for creating unencrypted TCP connections. + // If DialContext is nil, then the client dials using package net. + DialContext func(ctx context.Context, network, addr string) (net.Conn, error) + // TLS Config used for FTPS. If provided, it will be an error if the server // does not support TLS. Both the control and data connection will use TLS. TLSConfig *tls.Config @@ -173,7 +178,6 @@ type Client struct { // Construct and return a new client Conn, setting default config // values as necessary. func newClient(config Config, hosts []string) *Client { - if config.ConnectionsPerHost <= 0 { config.ConnectionsPerHost = 5 } @@ -182,6 +186,12 @@ func newClient(config Config, hosts []string) *Client { config.Timeout = 5 * time.Second } + if config.DialContext == nil { + config.DialContext = (&net.Dialer{ + Timeout: config.Timeout, + }).DialContext + } + if config.User == "" { config.User = "anonymous" } @@ -240,7 +250,7 @@ func (c *Client) debug(f string, args ...interface{}) { } fmt.Fprintf(c.config.Logger, "goftp: %.3f %s\n", - time.Now().Sub(c.t0).Seconds(), + time.Since(c.t0).Seconds(), fmt.Sprintf(f, args...), ) } @@ -367,18 +377,13 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro epsvNotSupported: c.config.DisableEPSV, } - var conn net.Conn + // FIXME: this method should accept a parent Context + ctx, cancel := context.WithTimeout(context.Background(), c.config.Timeout) + defer cancel() - if c.config.TLSConfig != nil && c.config.TLSMode == TLSImplicit { - pconn.debug("opening TLS control connection to %s", host) - dialer := &net.Dialer{ - Timeout: c.config.Timeout, - } - conn, err = tls.DialWithDialer(dialer, "tcp", host, pconn.config.TLSConfig) - } else { - pconn.debug("opening control connection to %s", host) - conn, err = net.DialTimeout("tcp", host, c.config.Timeout) - } + var conn net.Conn + pconn.debug("opening control connection to %s", host) + conn, err = c.config.DialContext(ctx, "tcp", host) var ( code int @@ -397,6 +402,11 @@ func (c *Client) openConn(idx int, host string) (pconn *persistentConn, err erro goto Error } + if c.config.TLSConfig != nil && c.config.TLSMode == TLSImplicit { + pconn.debug("configuring implicit TLS control connection to %s", host) + conn = tls.Client(conn, pconn.config.TLSConfig) + } + pconn.setControlConn(conn) code, msg, err = pconn.readResponse() diff --git a/client_test.go b/client_test.go index ff14c14..d9aa380 100644 --- a/client_test.go +++ b/client_test.go @@ -6,7 +6,9 @@ package goftp import ( "bytes" + "context" "crypto/tls" + "net" "sync" "testing" "time" @@ -19,7 +21,7 @@ func TestTimeoutConnect(t *testing.T) { t0 := time.Now() _, err = c.ReadDir("") - delta := time.Now().Sub(t0) + delta := time.Since(t0) if err == nil || !err.(Error).Temporary() { t.Error("Expected a timeout error") @@ -40,9 +42,16 @@ func TestTimeoutConnect(t *testing.T) { func TestExplicitTLS(t *testing.T) { for _, addr := range ftpdAddrs { + var gotAddr string config := Config{ User: "goftp", Password: "rocks", + DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if gotAddr == "" { + gotAddr = addr + } + return (&net.Dialer{}).DialContext(ctx, network, addr) + }, TLSConfig: &tls.Config{ InsecureSkipVerify: true, }, @@ -64,6 +73,10 @@ func TestExplicitTLS(t *testing.T) { t.Errorf("Got %v", buf.Bytes()) } + if gotAddr != addr { + t.Errorf("Expected dial to %s, got %s", addr, gotAddr) + } + if c.numOpenConns() != len(c.freeConnCh) { t.Error("Leaked a connection") } @@ -71,13 +84,6 @@ func TestExplicitTLS(t *testing.T) { } func TestImplicitTLS(t *testing.T) { - closer, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") - if err != nil { - t.Fatal(err) - } - - defer closer() - for _, addr := range implicitTLSAddrs { config := Config{ TLSConfig: &tls.Config{ diff --git a/file_system.go b/file_system.go index d80bf40..6661c36 100644 --- a/file_system.go +++ b/file_system.go @@ -212,7 +212,7 @@ func (c *Client) controlStringList(f string, args ...interface{}) ([]string, err cmd := fmt.Sprintf(f, args...) - code, msg, err := pconn.sendCommand(cmd) + code, msg, _ := pconn.sendCommand(cmd) if !positiveCompletionReply(code) { pconn.debug("unexpected response to %s: %d-%s", cmd, code, msg) diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..6bf9320 --- /dev/null +++ b/go.mod @@ -0,0 +1,3 @@ +module github.com/secsy/goftp + +go 1.14 diff --git a/main_test.go b/main_test.go index 22a1254..c57a7ec 100644 --- a/main_test.go +++ b/main_test.go @@ -32,6 +32,12 @@ var ( ) func TestMain(m *testing.M) { + implicitCloser, err := startPureFTPD(implicitTLSAddrs, "ftpd/pure-ftpd-implicittls") + + if err != nil { + log.Fatal(err) + } + pureCloser, err := startPureFTPD(pureAddrs, "ftpd/pure-ftpd") ftpdAddrs = append(ftpdAddrs, pureAddrs...) @@ -49,6 +55,7 @@ func TestMain(m *testing.M) { var ret int func() { + defer implicitCloser() defer pureCloser() defer proCloser() ret = m.Run() @@ -86,6 +93,8 @@ func startPureFTPD(addrs []string, binary string) (func(), error) { cmd.Env = []string{fmt.Sprintf("FTP_ANON_DIR=%s/testroot", cwd)} + // cmd.Stderr = os.Stderr + err = cmd.Start() if err != nil { return nil, fmt.Errorf("error starting pure-ftpd on %s: %s", addr, err) @@ -129,7 +138,6 @@ func startProFTPD() (func(), error) { // "--debug", "10", ) - // cmd.Stdout = os.Stdout // cmd.Stderr = os.Stderr err = cmd.Start() diff --git a/persistent_connection.go b/persistent_connection.go index 15f5d74..7d2be65 100644 --- a/persistent_connection.go +++ b/persistent_connection.go @@ -6,6 +6,7 @@ package goftp import ( "bufio" + "context" "crypto/tls" "fmt" "net" @@ -187,7 +188,7 @@ func (pconn *persistentConn) debug(f string, args ...interface{}) { } fmt.Fprintf(pconn.config.Logger, "goftp: %.3f #%d %s\n", - time.Now().Sub(pconn.t0).Seconds(), + time.Since(pconn.t0).Seconds(), pconn.idx, fmt.Sprintf(f, args...), ) @@ -310,7 +311,7 @@ func (pconn *persistentConn) requestPassive() (string, error) { goto PASV } - remoteHost, _, err = net.SplitHostPort(pconn.controlConn.RemoteAddr().String()) + remoteHost, _, err = net.SplitHostPort(pconn.host) if err != nil { pconn.debug("failed determining remote host: %s", err) goto PASV @@ -418,8 +419,12 @@ func (pconn *persistentConn) prepareDataConn() (func() (net.Conn, error), error) return nil, err } + // FIXME: this method should accept a parent Context + ctx, cancel := context.WithTimeout(context.Background(), pconn.config.Timeout) + defer cancel() + pconn.debug("opening data connection to %s", host) - dc, netErr := net.DialTimeout("tcp", host, pconn.config.Timeout) + dc, netErr := pconn.config.DialContext(ctx, "tcp", host) if netErr != nil { var isTemporary bool diff --git a/round_tripper.go b/round_tripper.go new file mode 100644 index 0000000..5784965 --- /dev/null +++ b/round_tripper.go @@ -0,0 +1,78 @@ +package goftp + +import ( + "bufio" + "crypto/tls" + "io" + "net/http" + "strings" +) + +// RoundTrip implements the http.RoundTripper interface to allow an http.Client +// to handle ftp:// or ftps:// URLs. If req.URL.User is nil, the user and password +// from config will be used instead. +// Typical usage would be to register a Config to handle +// ftp:// and/or ftps:// URLs with http.Transport.RegisterProtocol. +// The User and Password fields in Config will be used when connecting +// to the remote FTP server unless the http.Request’s URL.User is non-nil. +func (config Config) RoundTrip(req *http.Request) (*http.Response, error) { + switch req.URL.Scheme { + default: + return nil, http.ErrSkipAltProtocol + case "ftp": + case "ftps": + if config.TLSConfig == nil { + config.TLSConfig = &tls.Config{} + } + } + + // If req.URL.User is non-nil, username and password + // will override config even if empty. + if req.URL.User != nil { + config.User = req.URL.User.Username() + config.Password, _ = req.URL.User.Password() + } + + path := strings.TrimPrefix(req.URL.Path, "/") + + client, err := DialConfig(config, req.URL.Host) + if err != nil { + return nil, err + } + + res := &http.Response{} + switch req.Method { + default: + return nil, http.ErrNotSupported + case http.MethodGet: + // Pipe Client.Retrieve to res.Body so enable unbuffered reads + // of large files. + // Errors returned by Client.Retrieve (like the size check) + // will be returned by res.Body.Read(). + r, w := io.Pipe() + brc := &bufferedReadCloser{bufio.NewReader(r), r} + res.Body = brc + go func() { + w.CloseWithError(client.Retrieve(path, w)) + }() + _, err = brc.Peek(1) // Get error immediately on bad read + if err != nil { + res.Body.Close() + res = nil + } else { + // Simulate HTTP response semantics + res.StatusCode = 200 + res.Status = http.StatusText(res.StatusCode) + } + } + return res, err +} + +type bufferedReadCloser struct { + *bufio.Reader + rc io.ReadCloser +} + +func (rc *bufferedReadCloser) Close() error { + return rc.rc.Close() +} diff --git a/round_tripper_test.go b/round_tripper_test.go new file mode 100644 index 0000000..c8bf49d --- /dev/null +++ b/round_tripper_test.go @@ -0,0 +1,157 @@ +package goftp + +import ( + "bytes" + "crypto/tls" + "io/ioutil" + "net/http" + "net/url" + "testing" + "time" +) + +func TestRoundTripperSkipAltProtocol(t *testing.T) { + config := Config{} + + req, err := http.NewRequest(http.MethodGet, "foo://localhost/foo.txt", nil) + if err != nil { + t.Fatal(err) + } + + _, err = config.RoundTrip(req) + if err != http.ErrSkipAltProtocol { + t.Errorf("Expected err = %v, got %v", http.ErrSkipAltProtocol, err) + } +} + +func TestRoundTripperTimeoutConnect(t *testing.T) { + config := Config{Timeout: 100 * time.Millisecond} + + req, err := http.NewRequest(http.MethodGet, "ftp://168.254.111.222:2121/subdir/1234.bin", nil) + if err != nil { + t.Fatal(err) + } + + t0 := time.Now() + _, err = config.RoundTrip(req) + delta := time.Since(t0) + if err == nil || !err.(Error).Temporary() { + t.Errorf("Expected a timeout error: %v", err) + } + + offBy := delta - config.Timeout + if offBy < 0 { + offBy = -offBy + } + if offBy > 50*time.Millisecond { + t.Errorf("Timeout of 100ms was off by %s", offBy) + } +} + +func TestRoundTripperExplicitTLS(t *testing.T) { + for _, addr := range ftpdAddrs { + config := Config{ + TLSConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + TLSMode: TLSExplicit, + } + + req, err := http.NewRequest(http.MethodGet, "ftp://"+addr+"/subdir/1234.bin", nil) + if err != nil { + t.Fatal(err) + } + + req.URL.User = url.UserPassword("goftp", "rocks") + + res, err := config.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + + if want, got := http.StatusOK, res.StatusCode; want != got { + t.Errorf("res.StatusCode: want: %v got: %v", want, got) + } + if want, got := http.StatusText(http.StatusOK), res.Status; want != got { + t.Errorf("res.Status: want: %v got: %v", want, got) + } + + b, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal([]byte{1, 2, 3, 4}, b) { + t.Errorf("Got %v", b) + } + + // Test nonexistent file + req, err = http.NewRequest(http.MethodGet, "ftp://"+addr+"/nonexistent.file", nil) + if err != nil { + t.Fatal(err) + } + req.URL.User = url.UserPassword("goftp", "rocks") + res, err = config.RoundTrip(req) + if res != nil { + t.Errorf("expected nil http.Response: %v", res) + } + if err == nil { + t.Errorf("expected non-nil error") + } + } +} + +func TestRoundTripperImplicitTLS(t *testing.T) { + for _, addr := range implicitTLSAddrs { + config := Config{ + TLSConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + TLSMode: TLSImplicit, + } + + req, err := http.NewRequest(http.MethodGet, "ftp://"+addr+"/subdir/1234.bin", nil) + if err != nil { + t.Fatal(err) + } + + req.URL.User = url.UserPassword("goftp", "rocks") + + res, err := config.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + + if want, got := http.StatusOK, res.StatusCode; want != got { + t.Errorf("res.StatusCode: want: %v got: %v", want, got) + } + if want, got := http.StatusText(http.StatusOK), res.Status; want != got { + t.Errorf("res.Status: want: %v got: %v", want, got) + } + + b, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal([]byte{1, 2, 3, 4}, b) { + t.Errorf("Got %v", b) + } + + // Test nonexistent file + req, err = http.NewRequest(http.MethodGet, "ftp://"+addr+"/nonexistent.file", nil) + if err != nil { + t.Fatal(err) + } + req.URL.User = url.UserPassword("goftp", "rocks") + res, err = config.RoundTrip(req) + if res != nil { + t.Errorf("expected nil http.Response: %v", res) + } + if err == nil { + t.Errorf("expected non-nil err") + } + } +} diff --git a/transfer.go b/transfer.go index b3ff8d3..032e990 100644 --- a/transfer.go +++ b/transfer.go @@ -7,7 +7,6 @@ package goftp import ( "fmt" "io" - "os" "strconv" ) @@ -90,7 +89,7 @@ func (c *Client) Store(path string, src io.Reader) error { } } - _, seekErr := seeker.Seek(size, os.SEEK_SET) + _, seekErr := seeker.Seek(size, io.SeekStart) if seekErr != nil { c.debug("failed seeking to %d while resuming upload to %s: %s", size,