From c22acb099109d718880041f6cee79a39c5fdde4f Mon Sep 17 00:00:00 2001 From: Matthew McPherrin Date: Wed, 14 Mar 2018 21:30:13 +0000 Subject: [PATCH 1/2] More robust parsing of FTP protocol I found some servers don't always return a space after the three-digit code for example, certigo connect --start-tls ftp ftp.mirrorservice.org:21 error connecting: FTP server responded with status 220----------------------------------------------------------------------------- , was expecting 220 Instead, we always take the first three digits and parse them as an integer, rather than relying on the space. Makes the code look a little cleaner too, by returning it as an int. --- starttls/ftp.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/starttls/ftp.go b/starttls/ftp.go index b2bb066..856730d 100644 --- a/starttls/ftp.go +++ b/starttls/ftp.go @@ -19,10 +19,9 @@ package starttls import ( "bufio" "crypto/tls" - "errors" "fmt" "net" - "strings" + "strconv" ) func dumpAuthTLSFromFTP(dialer Dialer, address string, config *tls.Config) (*tls.ConnectionState, error) { @@ -36,8 +35,8 @@ func dumpAuthTLSFromFTP(dialer Dialer, address string, config *tls.Config) (*tls if err != nil { return nil, err } - if status != "220" { - return nil, fmt.Errorf("FTP server responded with status %s, was expecting 220", status) + if status != 220 { + return nil, fmt.Errorf("FTP server responded with status %d, was expecting 220", status) } fmt.Fprintf(conn, "AUTH TLS\r\n") @@ -45,8 +44,8 @@ func dumpAuthTLSFromFTP(dialer Dialer, address string, config *tls.Config) (*tls if err != nil { return nil, err } - if status != "234" { - return nil, fmt.Errorf("FTP server responded with status %s, was expecting 234", status) + if status != 234 { + return nil, fmt.Errorf("FTP server responded with status %d, was expecting 234", status) } tlsConn := tls.Client(conn, config) @@ -59,15 +58,14 @@ func dumpAuthTLSFromFTP(dialer Dialer, address string, config *tls.Config) (*tls return &state, nil } -func readFTP(conn *net.TCPConn) (string, error) { +func readFTP(conn *net.TCPConn) (int, error) { reader := bufio.NewReader(conn) response, err := reader.ReadString('\n') if err != nil { - return "", err + return 0, err } - status := strings.Split(response, " ") - if len(status) == 0 { - return "", errors.New("garbled response from FTP server after AUTH TLS command") + if len(response) <= 3 { + return 0, fmt.Errorf("Error parsing ftp protocol: Status code too short: '%s'", response) } - return status[0], nil + return strconv.Atoi(response[:3]) } From 58f10be303c45f20a8c0184c0ff2035e9afd9d86 Mon Sep 17 00:00:00 2001 From: Matthew McPherrin Date: Wed, 14 Mar 2018 21:48:13 +0000 Subject: [PATCH 2/2] Add an aliased --starttls. DRY the protocols by putting them in starttls.go Unfortunately kingpin doesn't support natively support aliased flag names, so you can supply both --start-tls and --starttls without an error, but that's just "holding it wrong". --- main.go | 5 ++++- starttls/starttls.go | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 7fd483d..661e951 100644 --- a/main.go +++ b/main.go @@ -49,7 +49,7 @@ var ( connectCaPath = connect.Flag("ca", "Path to CA bundle (system default if unspecified).").ExistingFile() connectCert = connect.Flag("cert", "Client certificate chain for connecting to server (PEM).").ExistingFile() connectKey = connect.Flag("key", "Private key for client certificate, if not in same file (PEM).").ExistingFile() - connectStartTLS = connect.Flag("start-tls", "Enable StartTLS protocol ('ldap', 'mysql', 'postgres', 'smtp' or 'ftp').").Short('t').PlaceHolder("PROTOCOL").Enum("mysql", "postgres", "psql", "smtp", "ldap", "ftp") + connectStartTLS = connect.Flag("start-tls", fmt.Sprintf("Enable StartTLS protocol; one of: %v.", starttls.Protocols)).Short('t').PlaceHolder("PROTOCOL").Enum(starttls.Protocols...) connectIdentity = connect.Flag("identity", "With --start-tls, sets the DB user or SMTP EHLO name").Default("certigo").String() connectProxy = connect.Flag("proxy", "Optional URI for HTTP(s) CONNECT proxy to dial connections with").URL() connectTimeout = connect.Flag("timeout", "Timeout for connecting to remote server (can be '5m', '1s', etc).").Default("5s").Duration() @@ -73,6 +73,9 @@ func main() { terminalWidth := determineTerminalWidth() + // Alias starttls to start-tls + connect.Flag("starttls", "").Hidden().EnumVar(connectStartTLS, starttls.Protocols...) + stdout := colorable.NewColorableStdout() result := simpleResult{} switch kingpin.MustParse(app.Parse(os.Args[1:])) { diff --git a/starttls/starttls.go b/starttls/starttls.go index e181520..0a167b6 100644 --- a/starttls/starttls.go +++ b/starttls/starttls.go @@ -34,6 +34,9 @@ import ( "github.com/mwitkow/go-http-dialer" ) +// Protocols are the names of supported protocols +var Protocols []string = []string{"mysql", "postgres", "psql", "smtp", "ldap", "ftp"} + type connectResult struct { state *tls.ConnectionState err error