implement tri-state verification in FTP detector (#1604)

This PR implements tri-state verification in the FTP detector. The verification timeout was made injectable to support a new test case. Some test cases that had already been broken have been fixed as well.

pkg/detectors/ftp/ftp.go

@@ -2,6 +2,8 @@ package ftp
 
 import (
 	"context"
+	"errors"
+	"net/textproto"
 	"net/url"
 	"regexp"
 	"strings"
@@ -13,7 +15,17 @@ import (
 	"github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb"
 )
 
-type Scanner struct{}
+const (
+	// https://datatracker.ietf.org/doc/html/rfc959
+	ftpNotLoggedIn = 530
+
+	defaultVerificationTimeout = 5 * time.Second
+)
+
+type Scanner struct {
+	// Verification timeout. Defaults to 5 seconds if unset.
+	verificationTimeout time.Duration
+}
 
 // Ensure the Scanner satisfies the interface at compile time.
 var _ detectors.Detector = (*Scanner)(nil)
@@ -58,48 +70,59 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result
 		rawURL.Path = ""
 		redact := strings.TrimSpace(strings.Replace(rawURL.String(), password, "********", -1))
 
-		s := detectors.Result{
+		r := detectors.Result{
 			DetectorType: detectorspb.DetectorType_FTP,
 			Raw:          []byte(rawURL.String()),
 			Redacted:     redact,
 		}
 
 		if verify {
-			s.Verified = verifyFTP(ctx, parsedURL)
+			timeout := s.verificationTimeout
+			if timeout == 0 {
+				timeout = defaultVerificationTimeout
+			}
+			verificationErr := verifyFTP(timeout, parsedURL)
+			r.Verified = verificationErr == nil
+			if !isErrDeterminate(verificationErr) {
+				r.VerificationError = verificationErr
+			}
 		}
 
-		if !s.Verified {
+		if !r.Verified {
 			// Skip unverified findings where the password starts with a `$` - it's almost certainly a variable.
 			if strings.HasPrefix(password, "$") {
 				continue
 			}
 		}
 
-		if detectors.IsKnownFalsePositive(string(s.Raw), []detectors.FalsePositive{"@ftp.freebsd.org"}, false) {
+		if detectors.IsKnownFalsePositive(string(r.Raw), []detectors.FalsePositive{"@ftp.freebsd.org"}, false) {
 			continue
 		}
 
-		results = append(results, s)
+		results = append(results, r)
 	}
 
 	return results, nil
 }
 
-func verifyFTP(ctx context.Context, u *url.URL) bool {
+func isErrDeterminate(e error) bool {
+	ftpErr := &textproto.Error{}
+	return errors.As(e, &ftpErr) && ftpErr.Code == ftpNotLoggedIn
+}
+
+func verifyFTP(timeout time.Duration, u *url.URL) error {
 	host := u.Host
 	if !strings.Contains(host, ":") {
 		host = host + ":21"
 	}
 
-	c, err := ftp.Dial(host, ftp.DialWithTimeout(5*time.Second))
+	c, err := ftp.Dial(host, ftp.DialWithTimeout(timeout))
 	if err != nil {
-		return false
+		return err
 	}
 
 	password, _ := u.User.Password()
-	err = c.Login(u.User.Username(), password)
-
-	return err == nil
+	return c.Login(u.User.Username(), password)
 }
 
 func (s Scanner) Type() detectorspb.DetectorType {

pkg/detectors/ftp/ftp_test.go

@@ -5,9 +5,11 @@ package ftp
 
 import (
 	"context"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"testing"
+	"time"
 
-	"github.com/kylelemons/godebug/pretty"
 	"github.com/trufflesecurity/trufflehog/v3/pkg/detectors"
 	"github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb"
 )
@@ -19,11 +21,12 @@ func TestFTP_FromChunk(t *testing.T) {
 		verify bool
 	}
 	tests := []struct {
-		name    string
-		s       Scanner
-		args    args
-		want    []detectors.Result
-		wantErr bool
+		name                string
+		s                   Scanner
+		args                args
+		want                []detectors.Result
+		wantErr             bool
+		wantVerificationErr bool
 	}{
 		{
 			name: "bad scheme",
@@ -48,7 +51,7 @@ func TestFTP_FromChunk(t *testing.T) {
 				{
 					DetectorType: detectorspb.DetectorType_FTP,
 					Verified:     true,
-					Redacted:     "ftp://dlpuser:*************************@ftp.dlptest.com",
+					Redacted:     "ftp://dlpuser:********@ftp.dlptest.com",
 				},
 			},
 			wantErr: false,
@@ -66,11 +69,49 @@ func TestFTP_FromChunk(t *testing.T) {
 				{
 					DetectorType: detectorspb.DetectorType_FTP,
 					Verified:     false,
-					Redacted:     "ftp://dlpuser:*******@ftp.dlptest.com",
+					Redacted:     "ftp://dlpuser:********@ftp.dlptest.com",
 				},
 			},
 			wantErr: false,
 		},
+		{
+			name: "bad host",
+			s:    Scanner{},
+			args: args{
+				ctx: context.Background(),
+				// https://dlptest.com/ftp-test/
+				data:   []byte("ftp://dlpuser:[email protected]"),
+				verify: true,
+			},
+			want: []detectors.Result{
+				{
+					DetectorType: detectorspb.DetectorType_FTP,
+					Verified:     false,
+					Redacted:     "ftp://dlpuser:********@ftp.dlptest.com.badhost",
+				},
+			},
+			wantErr:             false,
+			wantVerificationErr: true,
+		},
+		{
+			name: "timeout",
+			s:    Scanner{verificationTimeout: 1 * time.Microsecond},
+			args: args{
+				ctx: context.Background(),
+				// https://dlptest.com/ftp-test/
+				data:   []byte("ftp://dlpuser:[email protected]"),
+				verify: true,
+			},
+			want: []detectors.Result{
+				{
+					DetectorType: detectorspb.DetectorType_FTP,
+					Verified:     false,
+					Redacted:     "ftp://dlpuser:********@ftp.dlptest.com",
+				},
+			},
+			wantErr:             false,
+			wantVerificationErr: true,
+		},
 		{
 			name: "blocked FP",
 			s:    Scanner{},
@@ -84,8 +125,7 @@ func TestFTP_FromChunk(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s := Scanner{}
-			got, err := s.FromData(tt.args.ctx, tt.args.verify, tt.args.data)
+			got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("URI.FromData() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -95,9 +135,14 @@ func TestFTP_FromChunk(t *testing.T) {
 			// }
 			for i := range got {
 				got[i].Raw = nil
+
+				if (got[i].VerificationError != nil) != tt.wantVerificationErr {
+					t.Fatalf("wantVerificationError = %v, verification error = %v", tt.wantVerificationErr, got[i].VerificationError)
+				}
 			}
-			if diff := pretty.Compare(got, tt.want); diff != "" {
-				t.Errorf("URI.FromData() %s diff: (-got +want)\n%s", tt.name, diff)
+			opts := cmpopts.IgnoreFields(detectors.Result{}, "VerificationError")
+			if diff := cmp.Diff(got, tt.want, opts); diff != "" {
+				t.Errorf("FTP.FromData() %s diff: (-got +want)\n%s", tt.name, diff)
 			}
 		})
 	}