From 1a002972dfe436032d01c1926e5fe3c786fe2006 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Thu, 18 Jan 2024 17:21:38 -0500 Subject: [PATCH 01/21] first draft --- pkg/detectors/postgres/postgres.go | 246 +++++++----------------- pkg/detectors/postgres/postgres_test.go | 139 +------------ 2 files changed, 76 insertions(+), 309 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index cb22536cd396..26e60c15d093 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -4,58 +4,81 @@ import ( "context" "database/sql" "fmt" - "net/url" "regexp" + "strconv" "strings" "time" - _ "github.com/lib/pq" // PostgreSQL driver + "github.com/lib/pq" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" ) const ( defaultPort = "5432" - defaultHost = "localhost" ) +// This detector currently only finds Postgres connection string URIs +// (https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING-URIS) When it finds one, it uses +// pq.ParseURI to normalize this into space-separated key-value pair Postgres connection string, and then uses a regular +// expression to transform this connection string into a parameters map. This parameters map is manipulated prior to +// verification, which operates by transforming the map back into a space-separated kvp connection string. This is kind +// of clunky overall, but it has the benefit of preserving the connection string as a map when it needs to be modified, +// which is much nicer than having to patch a space-separated string of kvps. + +// Multi-host connection string URIs are currently not supported because pq.ParseURI doesn't parse them correctly. If we +// happen to run into a case where this matters we can address it then. var ( - _ detectors.Detector = (*Scanner)(nil) - uriPattern = regexp.MustCompile(`\b(?i)postgresql://[\S]+\b`) - hostnamePattern = regexp.MustCompile(`(?i)(?:host|server|address).{0,40}?(\b[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b)`) - portPattern = regexp.MustCompile(`(?i)(?:port|p).{0,40}?(\b[0-9]{1,5}\b)`) - usernamePattern = regexp.MustCompile(`(?im)(?:user|usr)\S{0,40}?[:=\s]{1,3}[ '"=]{0,1}([^:'"\s]{4,40})`) - passwordPattern = regexp.MustCompile(`(?im)(?:pass)\S{0,40}?[:=\s]{1,3}[ '"=]{0,1}([^:'"\s]{4,40})`) + _ detectors.Detector = (*Scanner)(nil) + uriPattern = regexp.MustCompile(`\b(?i)postgres(?:ql)?://\S+\b`) + connStrPartPattern = regexp.MustCompile(`([[:alpha:]]+)='(.+?)' ?`) ) type Scanner struct{} func (s Scanner) Keywords() []string { - return []string{"postgres", "psql", "pghost"} + return []string{"postgres"} } func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]detectors.Result, error) { var results []detectors.Result - var pgURLs []url.URL - pgURLs = append(pgURLs, findUriMatches(string(data))) - pgURLs = append(pgURLs, findComponentMatches(verify, string(data))...) + candidateParamSets := findUriMatches(data) - for _, pgURL := range pgURLs { - if pgURL.User == nil { + for _, params := range candidateParamSets { + user, ok := params["user"] + if !ok { continue } - username := pgURL.User.Username() - password, _ := pgURL.User.Password() - hostport := pgURL.Host + + password, ok := params["password"] + if !ok { + continue + } + + hostport, ok := params["host"] + if !ok { + continue + } + + if port, ok := params["port"]; ok { + hostport = hostport + ":" + port + } + result := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, - Raw: []byte(hostport + username + password), - RawV2: []byte(hostport + username + password), + Raw: []byte(hostport + user + password), + RawV2: []byte(hostport + user + password), } if verify { - timeoutInSeconds := getDeadlineInSeconds(ctx) - isVerified, verificationErr := verifyPostgres(&pgURL, timeoutInSeconds) + if timeout := getDeadlineInSeconds(ctx); timeout != 0 { + params["connect_timeout"] = strconv.Itoa(timeout) + } + + // We'd like the 'allow' mode but pq doesn't support it (https://github.com/lib/pq/issues/776) + // To kludge it we first try with 'require' and then fall back to 'disable' if there's an SSL error + params["sslmode"] = "require" + isVerified, verificationErr := verifyPostgres(params) result.Verified = isVerified result.SetVerificationError(verificationErr, password) } @@ -69,186 +92,61 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete return results, nil } -func getDeadlineInSeconds(ctx context.Context) int { - deadline, ok := ctx.Deadline() - if !ok { - // Context does not have a deadline - return 0 - } - - duration := time.Until(deadline) - return int(duration.Seconds()) -} - -func findUriMatches(dataStr string) url.URL { - var pgURL url.URL - for _, uri := range uriPattern.FindAllString(dataStr, -1) { - pgURL, err := url.Parse(uri) +func findUriMatches(data []byte) []map[string]string { + var matches []map[string]string + for _, uri := range uriPattern.FindAll(data, -1) { + connStr, err := pq.ParseURL(string(uri)) if err != nil { continue } - if pgURL.User != nil { - return *pgURL - } - } - return pgURL -} - -// check if postgres is running -func postgresRunning(hostname, port string) bool { - connStr := fmt.Sprintf("host=%s port=%s sslmode=disable", hostname, port) - db, err := sql.Open("postgres", connStr) - if err != nil { - return false - } - defer db.Close() - return true -} - -func findComponentMatches(verify bool, dataStr string) []url.URL { - usernameMatches := usernamePattern.FindAllStringSubmatch(dataStr, -1) - passwordMatches := passwordPattern.FindAllStringSubmatch(dataStr, -1) - hostnameMatches := hostnamePattern.FindAllStringSubmatch(dataStr, -1) - portMatches := portPattern.FindAllStringSubmatch(dataStr, -1) - - var pgURLs []url.URL - - hosts := findHosts(verify, hostnameMatches, portMatches) - - for _, username := range dedupMatches(usernameMatches) { - for _, password := range dedupMatches(passwordMatches) { - for _, host := range hosts { - hostname, port := strings.Split(host, ":")[0], strings.Split(host, ":")[1] - if combinedLength := len(username) + len(password) + len(hostname); combinedLength > 255 { - continue - } - postgresURL := url.URL{ - Scheme: "postgresql", - User: url.UserPassword(username, password), - Host: fmt.Sprintf("%s:%s", hostname, port), - } - pgURLs = append(pgURLs, postgresURL) - } - } - } - return pgURLs -} - -// if verification is turned on, and we can confirm that postgres is running on at least one host, -// return only hosts where it's running. otherwise return all hosts. -func findHosts(verify bool, hostnameMatches, portMatches [][]string) []string { - hostnames := dedupMatches(hostnameMatches) - ports := dedupMatches(portMatches) - var hosts []string - - if len(hostnames) < 1 { - hostnames = append(hostnames, defaultHost) - } - - if len(ports) < 1 { - ports = append(ports, defaultPort) - } - - for _, hostname := range hostnames { - for _, port := range ports { - hosts = append(hosts, fmt.Sprintf("%s:%s", hostname, port)) - } - } - if verify { - var verifiedHosts []string - for _, host := range hosts { - parts := strings.Split(host, ":") - hostname, port := parts[0], parts[1] - if postgresRunning(hostname, port) { - verifiedHosts = append(verifiedHosts, host) - } - } - if len(verifiedHosts) > 0 { - return verifiedHosts + params := make(map[string]string) + parts := connStrPartPattern.FindAllStringSubmatch(connStr, -1) + for _, part := range parts { + params[part[1]] = part[2] } - } - return hosts -} - -// deduplicate matches in order to reduce the number of verification requests -func dedupMatches(matches [][]string) []string { - setOfMatches := make(map[string]struct{}) - for _, match := range matches { - if len(match) > 1 { - setOfMatches[match[1]] = struct{}{} - } + matches = append(matches, params) } - var results []string - for match := range setOfMatches { - results = append(results, match) - } - return results + return matches } -func verifyPostgres(pgURL *url.URL, timeoutInSeconds int) (bool, error) { - if pgURL.User == nil { - return false, nil - } - username := pgURL.User.Username() - password, _ := pgURL.User.Password() - - hostname, port := pgURL.Hostname(), pgURL.Port() - if hostname == "" { - hostname = defaultHost - } - if port == "" { - port = defaultPort +func getDeadlineInSeconds(ctx context.Context) int { + deadline, ok := ctx.Deadline() + if !ok { + // Context does not have a deadline + return 0 } - sslmode := determineSSLMode(pgURL) + duration := time.Until(deadline) + return int(duration.Seconds()) +} - connStr := fmt.Sprintf("user=%s password=%s host=%s port=%s sslmode=%s", username, password, hostname, port, sslmode) - if timeoutInSeconds > 0 { - connStr = fmt.Sprintf("%s connect_timeout=%d", connStr, timeoutInSeconds) +func verifyPostgres(params map[string]string) (bool, error) { + var connStr string + for key, value := range params { + connStr += fmt.Sprintf("%s='%s'", key, value) } db, err := sql.Open("postgres", connStr) if err != nil { - if strings.Contains(err.Error(), "connection refused") { - // inactive host - return false, nil - } return false, err } defer db.Close() - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - - err = db.PingContext(ctx) + err = db.Ping() if err == nil { return true, nil - } else if strings.Contains(err.Error(), "password authentication failed") || // incorrect username or password - strings.Contains(err.Error(), "connection refused") { // inactive host + } else if strings.Contains(err.Error(), "password authentication failed") { return false, nil + } else if strings.Contains(err.Error(), "SSL is not enabled on the server") { + params["sslmode"] = "disable" + return verifyPostgres(params) } - // if ssl is not enabled, manually fall-back to sslmode=disable - if strings.Contains(err.Error(), "SSL is not enabled on the server") { - pgURL.RawQuery = fmt.Sprintf("sslmode=%s", "disable") - return verifyPostgres(pgURL, timeoutInSeconds) - } return false, err } -func determineSSLMode(pgURL *url.URL) string { - // default ssl mode is "prefer" per https://www.postgresql.org/docs/current/libpq-ssl.html - // but is currently not implemented in the driver per https://github.com/lib/pq/issues/1006 - // default for the driver is "require". ideally we would use "allow" but that is also not supported by the driver. - sslmode := "require" - if sslQuery, ok := pgURL.Query()["sslmode"]; ok && len(sslQuery) > 0 { - sslmode = sslQuery[0] - } - return sslmode -} - func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_Postgres } diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 08da6dd2fabc..33b3fa659a22 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -26,7 +26,7 @@ const ( postgresUser = "postgres" postgresPass = "23201dabb56ca236f3dc6736c0f9afad" postgresHost = "localhost" - postgresPort = "5433" + postgresPort = "5434" // Do not use 5433, as local dev environments can use it for other things inactiveUser = "inactive" inactivePass = "inactive" @@ -62,28 +62,7 @@ func TestPostgres_FromChunk(t *testing.T) { wantErr: false, }, { - name: "found with seperated credentials, verified", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf(` - POSTGRES_USER=%s - POSTGRES_PASSWORD=%s - POSTGRES_ADDRESS=%s - POSTGRES_PORT=%s - `, postgresUser, postgresPass, postgresHost, postgresPort)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_Postgres, - Verified: true, - }, - }, - wantErr: false, - }, - { - name: "found with single line credentials, verified", + name: "found connection URI, verified", s: Scanner{}, args: args{ ctx: context.Background(), @@ -99,65 +78,7 @@ func TestPostgres_FromChunk(t *testing.T) { wantErr: false, }, { - name: "found with json credentials, verified", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf( - `DB_CONFIG={"user": "%s", "password": "%s", "host": "%s", "port": "%s", "database": "postgres"}`, postgresUser, postgresPass, postgresHost, postgresPort)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_Postgres, - Verified: true, - }, - }, - wantErr: false, - }, - { - name: "found with seperated credentials, unverified", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf(` - POSTGRES_USER=%s - POSTGRES_PASSWORD=%s - POSTGRES_ADDRESS=%s - POSTGRES_PORT=%s - `, postgresUser, inactivePass, postgresHost, postgresPort)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_Postgres, - Verified: false, - }, - }, - wantErr: false, - }, - { - name: "found with seperated credentials - no port, unverified", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf(` - POSTGRES_USER=%s - POSTGRES_PASSWORD=%s - POSTGRES_ADDRESS=%s - `, postgresUser, inactivePass, postgresHost)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_Postgres, - Verified: false, - }, - }, - wantErr: false, - }, - { - name: "found with single line credentials, unverified", + name: "found connection URI, unverified", s: Scanner{}, args: args{ ctx: context.Background(), @@ -173,59 +94,7 @@ func TestPostgres_FromChunk(t *testing.T) { wantErr: false, }, { - name: "found with json credentials, unverified - inactive password", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf( - `DB_CONFIG={"user": "%s", "password": "%s", "host": "%s", "port": "%s", "database": "postgres"}`, postgresUser, inactivePass, postgresHost, postgresPort)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_Postgres, - Verified: false, - }, - }, - wantErr: false, - }, - { - name: "found with json credentials, unverified - inactive user", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf( - `DB_CONFIG={"user": "%s", "password": "%s", "host": "%s", "port": "%s", "database": "postgres"}`, inactiveUser, postgresPass, postgresHost, postgresPort)), - verify: true, - }, - want: []detectors.Result{ - { - DetectorType: detectorspb.DetectorType_Postgres, - Verified: false, - }, - }, - wantErr: false, - }, - { - name: "found, unverified due to error - inactive port", - s: Scanner{}, - args: args{ - ctx: context.Background(), - data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, postgresHost, inactivePort)), - verify: true, - }, - want: func() []detectors.Result { - r := detectors.Result{ - DetectorType: detectorspb.DetectorType_Postgres, - Verified: false, - } - return []detectors.Result{r} - }(), - wantErr: false, - }, - // This test seems take a long time to run (70s+) even with the timeout set to 1s. It's not clear why. - { - name: "found, unverified due to error - inactive host", + name: "found connection URI, unverified due to error - inactive host", s: Scanner{}, args: func() args { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) From 8fd4342fa62e275d0c34acffdd54d6321b5041ad Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Thu, 18 Jan 2024 17:23:05 -0500 Subject: [PATCH 02/21] re-enable detector --- pkg/engine/defaults.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/engine/defaults.go b/pkg/engine/defaults.go index 0cd901ab947a..16682b867cdb 100644 --- a/pkg/engine/defaults.go +++ b/pkg/engine/defaults.go @@ -524,6 +524,7 @@ import ( "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/positionstack" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/postageapp" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/postbacks" + "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/postgres" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/posthog" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/postman" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/postmark" @@ -1496,8 +1497,7 @@ func DefaultDetectors() []detectors.Detector { speechtextai.Scanner{}, databox.Scanner{}, postbacks.Scanner{}, - // Too noisy, needs attention - // postgres.Scanner{}, + postgres.Scanner{}, collect2.Scanner{}, uclassify.Scanner{}, holistic.Scanner{}, From c0b7ac1381adcfe4eea64e48bcec4e89fea8ceb4 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Fri, 19 Jan 2024 13:54:05 -0500 Subject: [PATCH 03/21] ignore loopback --- pkg/detectors/postgres/postgres.go | 18 ++++++++++++--- pkg/detectors/postgres/postgres_test.go | 29 +++++++++++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 26e60c15d093..837590c02290 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "net" "regexp" "strconv" "strings" @@ -34,7 +35,9 @@ var ( connStrPartPattern = regexp.MustCompile(`([[:alpha:]]+)='(.+?)' ?`) ) -type Scanner struct{} +type Scanner struct { + detectLoopback bool // Automated tests run against localhost, but we don't consider those real results in the wild +} func (s Scanner) Keywords() []string { return []string{"postgres"} @@ -55,13 +58,22 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete continue } - hostport, ok := params["host"] + host, ok := params["host"] if !ok { continue } + if !s.detectLoopback { + if host == "localhost" { + continue + } + if ip := net.ParseIP(host); ip != nil && ip.IsLoopback() { + continue + } + } + hostport := host if port, ok := params["port"]; ok { - hostport = hostport + ":" + port + hostport = host + ":" + port } result := detectors.Result{ diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 33b3fa659a22..09166c59bf30 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -63,7 +63,7 @@ func TestPostgres_FromChunk(t *testing.T) { }, { name: "found connection URI, verified", - s: Scanner{}, + s: Scanner{detectLoopback: true}, args: args{ ctx: context.Background(), data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, postgresHost, postgresPort)), @@ -79,7 +79,7 @@ func TestPostgres_FromChunk(t *testing.T) { }, { name: "found connection URI, unverified", - s: Scanner{}, + s: Scanner{detectLoopback: true}, args: args{ ctx: context.Background(), data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, inactivePass, postgresHost, postgresPort)), @@ -93,6 +93,28 @@ func TestPostgres_FromChunk(t *testing.T) { }, wantErr: false, }, + { + name: "ignored localhost", + s: Scanner{}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, "localhost", postgresPort)), + verify: true, + }, + want: nil, + wantErr: false, + }, + { + name: "ignored 127.0.0.1", + s: Scanner{}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, "127.0.0.1", postgresPort)), + verify: true, + }, + want: nil, + wantErr: false, + }, { name: "found connection URI, unverified due to error - inactive host", s: Scanner{}, @@ -118,8 +140,7 @@ func TestPostgres_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("postgres.FromData() error = %v, wantErr %v", err, tt.wantErr) return From 2941f82ff9a3b0ca2b3410fa0b87926378fb5927 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Fri, 19 Jan 2024 14:10:53 -0500 Subject: [PATCH 04/21] report docker startup err --- pkg/detectors/postgres/postgres_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 09166c59bf30..349a8c36d27a 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -35,7 +35,13 @@ const ( ) func TestPostgres_FromChunk(t *testing.T) { - startPostgres() + if err := startPostgres(); err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + t.Fatalf("could not start local postgres: %v w/stderr:\n%s", err, string(exitErr.Stderr)) + } else { + t.Fatalf("could not start local postgres: %v", err) + } + } defer stopPostgres() type args struct { From d9f5c316566728541bf58db1b681013730b39622 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Fri, 19 Jan 2024 15:09:17 -0500 Subject: [PATCH 05/21] check for missing db --- pkg/detectors/postgres/postgres.go | 14 +++++++++++++- pkg/detectors/postgres/postgres_test.go | 22 +++++++++++++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 837590c02290..227ec17f73f6 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -3,6 +3,7 @@ package postgres import ( "context" "database/sql" + "errors" "fmt" "net" "regexp" @@ -134,6 +135,15 @@ func getDeadlineInSeconds(ctx context.Context) int { return int(duration.Seconds()) } +func isErrorDatabaseNotFound(err error, dbName string) bool { + if dbName == "" { + dbName = "postgres" + } + missingDbErrorText := fmt.Sprintf("database \"%s\" does not exist", dbName) + + return strings.Contains(err.Error(), missingDbErrorText) +} + func verifyPostgres(params map[string]string) (bool, error) { var connStr string for key, value := range params { @@ -151,9 +161,11 @@ func verifyPostgres(params map[string]string) (bool, error) { return true, nil } else if strings.Contains(err.Error(), "password authentication failed") { return false, nil - } else if strings.Contains(err.Error(), "SSL is not enabled on the server") { + } else if errors.Is(err, pq.ErrSSLNotSupported) { params["sslmode"] = "disable" return verifyPostgres(params) + } else if isErrorDatabaseNotFound(err, params["dbname"]) { + return true, nil // If we know this, we were able to authenticate } return false, err diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 349a8c36d27a..5c46abd4a4ca 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -44,6 +44,18 @@ func TestPostgres_FromChunk(t *testing.T) { } defer stopPostgres() + // The detector is written to connect to the database 'postgres' if no explicit database is found in the candidate + // secret (because pq uses 'postgres' as a default if no database is specified). If the target cluster doesn't + // actually have a database with this name, we should still be able to verify the candidate secret, because if we + // can authenticate, Postgres will tell us that the database is missing, and if we can't, it will just tell us that + // we can't authenticate. + // + // Unfortunately, directly validating this in the automated tests is awkward because the docker image's POSTGRES_DB + // environment variable doesn't appear to work: The database created is always named 'postgres', no matter what + // POSTGRES_DB is set to. This means that we can't replicate a cluster without a database named 'postgres', so we + // can't directly test what happens if we see one. To work around this, all the automated tests try to connect to + // the nonexistent database 'postgres2'. In this way, we test the logic of attempting to connect to a non-existent + // database, even though the test cases are the inverse of what we'd see in the wild. type args struct { ctx context.Context data []byte @@ -72,7 +84,7 @@ func TestPostgres_FromChunk(t *testing.T) { s: Scanner{detectLoopback: true}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, postgresHost, postgresPort)), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2`, postgresUser, postgresPass, postgresHost, postgresPort)), verify: true, }, want: []detectors.Result{ @@ -88,7 +100,7 @@ func TestPostgres_FromChunk(t *testing.T) { s: Scanner{detectLoopback: true}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, inactivePass, postgresHost, postgresPort)), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2`, postgresUser, inactivePass, postgresHost, postgresPort)), verify: true, }, want: []detectors.Result{ @@ -104,7 +116,7 @@ func TestPostgres_FromChunk(t *testing.T) { s: Scanner{}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, "localhost", postgresPort)), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2`, postgresUser, postgresPass, "localhost", postgresPort)), verify: true, }, want: nil, @@ -115,7 +127,7 @@ func TestPostgres_FromChunk(t *testing.T) { s: Scanner{}, args: args{ ctx: context.Background(), - data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, "127.0.0.1", postgresPort)), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2`, postgresUser, postgresPass, "127.0.0.1", postgresPort)), verify: true, }, want: nil, @@ -129,7 +141,7 @@ func TestPostgres_FromChunk(t *testing.T) { defer cancel() return args{ ctx: ctx, - data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres`, postgresUser, postgresPass, inactiveHost, postgresPort)), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2`, postgresUser, postgresPass, inactiveHost, postgresPort)), verify: true, } }(), From a09417b95d6d6fc184913ada84a746108b46600c Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Fri, 19 Jan 2024 15:28:53 -0500 Subject: [PATCH 06/21] kludge a little less --- pkg/detectors/postgres/postgres.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 227ec17f73f6..4af94c634473 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -88,9 +88,6 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete params["connect_timeout"] = strconv.Itoa(timeout) } - // We'd like the 'allow' mode but pq doesn't support it (https://github.com/lib/pq/issues/776) - // To kludge it we first try with 'require' and then fall back to 'disable' if there's an SSL error - params["sslmode"] = "require" isVerified, verificationErr := verifyPostgres(params) result.Verified = isVerified result.SetVerificationError(verificationErr, password) @@ -162,6 +159,7 @@ func verifyPostgres(params map[string]string) (bool, error) { } else if strings.Contains(err.Error(), "password authentication failed") { return false, nil } else if errors.Is(err, pq.ErrSSLNotSupported) { + // Do not merge until this kludge is collectively sanctioned! params["sslmode"] = "disable" return verifyPostgres(params) } else if isErrorDatabaseNotFound(err, params["dbname"]) { From 526dda95c3add5bc348de6a27857045b63241124 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 11:07:36 -0500 Subject: [PATCH 07/21] adjust comment --- pkg/detectors/postgres/postgres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 4af94c634473..2083cfac2301 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -37,7 +37,7 @@ var ( ) type Scanner struct { - detectLoopback bool // Automated tests run against localhost, but we don't consider those real results in the wild + detectLoopback bool // Automated tests run against localhost, but we want to ignore those results in the wild } func (s Scanner) Keywords() []string { From eeed4ba9dbd0ed1084970a0dfb8226c9b6eaeaff Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 13:00:24 -0500 Subject: [PATCH 08/21] handle sslmodes better --- pkg/detectors/postgres/postgres.go | 8 ++- pkg/detectors/postgres/postgres_test.go | 80 +++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 2083cfac2301..33c3527c6c5f 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -142,6 +142,12 @@ func isErrorDatabaseNotFound(err error, dbName string) bool { } func verifyPostgres(params map[string]string) (bool, error) { + if sslmode := params["sslmode"]; sslmode == "allow" || sslmode == "prefer" { + // pq doesn't support 'allow' or 'prefer'. If we find either of them, we'll just ignore it. This will trigger + // the same logic that is run if no sslmode is set at all (which mimics 'prefer', which is the default). + delete(params, "sslmode") + } + var connStr string for key, value := range params { connStr += fmt.Sprintf("%s='%s'", key, value) @@ -158,7 +164,7 @@ func verifyPostgres(params map[string]string) (bool, error) { return true, nil } else if strings.Contains(err.Error(), "password authentication failed") { return false, nil - } else if errors.Is(err, pq.ErrSSLNotSupported) { + } else if errors.Is(err, pq.ErrSSLNotSupported) && params["sslmode"] == "" { // Do not merge until this kludge is collectively sanctioned! params["sslmode"] = "disable" return verifyPostgres(params) diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 5c46abd4a4ca..e85066a7d44a 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -15,6 +15,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/lib/pq" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" @@ -46,13 +47,12 @@ func TestPostgres_FromChunk(t *testing.T) { // The detector is written to connect to the database 'postgres' if no explicit database is found in the candidate // secret (because pq uses 'postgres' as a default if no database is specified). If the target cluster doesn't - // actually have a database with this name, we should still be able to verify the candidate secret, because if we - // can authenticate, Postgres will tell us that the database is missing, and if we can't, it will just tell us that - // we can't authenticate. + // actually have a database with this name, but our credentials are good, then Postgres will give us a "missing + // database" error message instead of an authentication failure. // // Unfortunately, directly validating this in the automated tests is awkward because the docker image's POSTGRES_DB // environment variable doesn't appear to work: The database created is always named 'postgres', no matter what - // POSTGRES_DB is set to. This means that we can't replicate a cluster without a database named 'postgres', so we + // POSTGRES_DB is set to. This means that we can't replicate a cluster that has no database named 'postgres', so we // can't directly test what happens if we see one. To work around this, all the automated tests try to connect to // the nonexistent database 'postgres2'. In this way, we test the logic of attempting to connect to a non-existent // database, even though the test cases are the inverse of what we'd see in the wild. @@ -80,7 +80,7 @@ func TestPostgres_FromChunk(t *testing.T) { wantErr: false, }, { - name: "found connection URI, verified", + name: "found connection URI with ssl mode unset, verified", s: Scanner{detectLoopback: true}, args: args{ ctx: context.Background(), @@ -95,6 +95,54 @@ func TestPostgres_FromChunk(t *testing.T) { }, wantErr: false, }, + { + name: "found connection URI with ssl mode 'prefer', verified", + s: Scanner{detectLoopback: true}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2?sslmode=prefer`, postgresUser, postgresPass, postgresHost, postgresPort)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_Postgres, + Verified: true, + }, + }, + wantErr: false, + }, + { + name: "found connection URI with ssl mode 'allow', verified", + s: Scanner{detectLoopback: true}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2?sslmode=allow`, postgresUser, postgresPass, postgresHost, postgresPort)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_Postgres, + Verified: true, + }, + }, + wantErr: false, + }, + { + name: "found connection URI without database, verified", + s: Scanner{detectLoopback: true}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/`, postgresUser, postgresPass, postgresHost, postgresPort)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_Postgres, + Verified: true, + }, + }, + wantErr: false, + }, { name: "found connection URI, unverified", s: Scanner{detectLoopback: true}, @@ -155,6 +203,28 @@ func TestPostgres_FromChunk(t *testing.T) { }(), wantErr: false, }, + { + name: "found connection URI, unverified due to error - ssl not supported", + s: Scanner{detectLoopback: true}, + args: func() args { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + return args{ + ctx: ctx, + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2?sslmode=require`, postgresUser, postgresPass, postgresHost, postgresPort)), + verify: true, + } + }(), + want: func() []detectors.Result { + r := detectors.Result{ + DetectorType: detectorspb.DetectorType_Postgres, + Verified: false, + } + r.SetVerificationError(pq.ErrSSLNotSupported) + return []detectors.Result{r} + }(), + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From da4b787cab7deebf1a26459dfa6b30149a3cb96a Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 13:13:58 -0500 Subject: [PATCH 09/21] store in extradata --- pkg/detectors/postgres/postgres.go | 17 ++++++++++++++++- pkg/detectors/postgres/postgres_test.go | 7 +++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 33c3527c6c5f..15d7558076b5 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -91,6 +91,13 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete isVerified, verificationErr := verifyPostgres(params) result.Verified = isVerified result.SetVerificationError(verificationErr, password) + sslmode := params["sslmode"] + if sslmode == "" { + sslmode = "" + } + result.ExtraData = map[string]string{ + "sslmode": sslmode, + } } if !result.Verified && detectors.IsKnownFalsePositive(password, detectors.DefaultFalsePositives, true) { @@ -146,6 +153,11 @@ func verifyPostgres(params map[string]string) (bool, error) { // pq doesn't support 'allow' or 'prefer'. If we find either of them, we'll just ignore it. This will trigger // the same logic that is run if no sslmode is set at all (which mimics 'prefer', which is the default). delete(params, "sslmode") + + // We still want to save the original sslmode in ExtraData, so we'll re-add it before returning. + defer func() { + params["sslmode"] = sslmode + }() } var connStr string @@ -165,8 +177,11 @@ func verifyPostgres(params map[string]string) (bool, error) { } else if strings.Contains(err.Error(), "password authentication failed") { return false, nil } else if errors.Is(err, pq.ErrSSLNotSupported) && params["sslmode"] == "" { - // Do not merge until this kludge is collectively sanctioned! + // If the sslmode is unset, then either it was unset in the candidate secret, or we've intentionally unset it + // because it was specified as 'allow' or 'prefer', neither of which pq supports. In all of these cases, non-SSL + // connections are acceptable, so now we try a connection without SSL to see if it works. params["sslmode"] = "disable" + defer delete(params, "sslmode") // We want to return with the original params map intact (for ExtraData) return verifyPostgres(params) } else if isErrorDatabaseNotFound(err, params["dbname"]) { return true, nil // If we know this, we were able to authenticate diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index e85066a7d44a..063b28763775 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -91,6 +91,7 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + ExtraData: map[string]string{"sslmode": ""}, }, }, wantErr: false, @@ -107,6 +108,7 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + ExtraData: map[string]string{"sslmode": "prefer"}, }, }, wantErr: false, @@ -123,6 +125,7 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + ExtraData: map[string]string{"sslmode": "allow"}, }, }, wantErr: false, @@ -139,6 +142,7 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + ExtraData: map[string]string{"sslmode": ""}, }, }, wantErr: false, @@ -155,6 +159,7 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: false, + ExtraData: map[string]string{"sslmode": ""}, }, }, wantErr: false, @@ -197,6 +202,7 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, + ExtraData: map[string]string{"sslmode": ""}, } r.SetVerificationError(errors.New("i/o timeout")) return []detectors.Result{r} @@ -219,6 +225,7 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, + ExtraData: map[string]string{"sslmode": "require"}, } r.SetVerificationError(pq.ErrSSLNotSupported) return []detectors.Result{r} From 0d37e553688b65c74999a264261693037369110e Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 13:35:21 -0500 Subject: [PATCH 10/21] normalize as url --- pkg/detectors/postgres/postgres.go | 12 +++++++----- pkg/detectors/postgres/postgres_test.go | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 15d7558076b5..b0b2f9c9cc98 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -72,15 +72,17 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete } } - hostport := host - if port, ok := params["port"]; ok { - hostport = host + ":" + port + port, ok := params["port"] + if !ok { + params["port"] = "5432" } + raw := []byte(fmt.Sprintf("postgresql://%s:%s@%s:%s", host, port, user, password)) + result := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, - Raw: []byte(hostport + user + password), - RawV2: []byte(hostport + user + password), + Raw: raw, + RawV2: raw, } if verify { diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 063b28763775..ba2a0eb478d4 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -91,6 +91,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -108,6 +110,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "prefer"}, }, }, @@ -125,6 +129,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "allow"}, }, }, @@ -142,6 +148,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -159,6 +167,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: false, + Raw: []byte("postgresql://localhost:5434@postgres:inactive"), + RawV2: []byte("postgresql://localhost:5434@postgres:inactive"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -202,6 +212,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, + Raw: []byte("postgresql://192.0.2.0:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://192.0.2.0:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": ""}, } r.SetVerificationError(errors.New("i/o timeout")) @@ -225,6 +237,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "require"}, } r.SetVerificationError(pq.ErrSSLNotSupported) @@ -256,7 +270,7 @@ func TestPostgres_FromChunk(t *testing.T) { t.Fatalf("wantVerificationError = %v, verification error = %v", tt.want[i].VerificationError(), got[i].VerificationError()) } } - ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "Raw", "RawV2", "verificationError") + ignoreOpts := cmpopts.IgnoreFields(detectors.Result{}, "verificationError") if diff := cmp.Diff(got, tt.want, ignoreOpts); diff != "" { t.Errorf("Postgres.FromData() %s diff: (-got +want)\n%s", tt.name, diff) } From 156790d668a14f8041e08db4a4d2334e4f22ed57 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 13:36:16 -0500 Subject: [PATCH 11/21] add timeout comment --- pkg/detectors/postgres/postgres.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index b0b2f9c9cc98..fcd7cbcb7f99 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -86,6 +86,8 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete } if verify { + // pq appears to ignore the context deadline, so we copy any timeout that's been set into the connection + // parameters themselves. if timeout := getDeadlineInSeconds(ctx); timeout != 0 { params["connect_timeout"] = strconv.Itoa(timeout) } From ec4c296c1252ee92caeac61e008daf8db6829070 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 13:58:46 -0500 Subject: [PATCH 12/21] normalize earlier --- pkg/detectors/postgres/postgres.go | 26 ++++++++++---- pkg/detectors/postgres/postgres_test.go | 46 ++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index fcd7cbcb7f99..a25313e44e5c 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -85,6 +85,16 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete RawV2: raw, } + // We don't need to normalize the (deprecated) requiressl option into the (up-to-date) sslmode option - pq can + // do it for us - but we will do it anyway here so that when we later capture sslmode into ExtraData we will + // capture it post-normalization. (The detector's behavior is undefined for candidate secrets that have both + // requiressl and sslmode set.) + if requiressl := params["requiressl"]; requiressl == "0" { + params["sslmode"] = "prefer" + } else if requiressl == "1" { + params["sslmode"] = "require" + } + if verify { // pq appears to ignore the context deadline, so we copy any timeout that's been set into the connection // parameters themselves. @@ -95,13 +105,15 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete isVerified, verificationErr := verifyPostgres(params) result.Verified = isVerified result.SetVerificationError(verificationErr, password) - sslmode := params["sslmode"] - if sslmode == "" { - sslmode = "" - } - result.ExtraData = map[string]string{ - "sslmode": sslmode, - } + } + + // We gather SSL information into ExtraData in case it's useful for later reporting. + sslmode := params["sslmode"] + if sslmode == "" { + sslmode = "" + } + result.ExtraData = map[string]string{ + "sslmode": sslmode, } if !result.Verified && detectors.IsKnownFalsePositive(password, detectors.DefaultFalsePositives, true) { diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index ba2a0eb478d4..8913ce3cbff6 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -136,6 +136,25 @@ func TestPostgres_FromChunk(t *testing.T) { }, wantErr: false, }, + { + name: "found connection URI with requiressl=0, verified", + s: Scanner{detectLoopback: true}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2?requiressl=0`, postgresUser, postgresPass, postgresHost, postgresPort)), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_Postgres, + Verified: true, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + ExtraData: map[string]string{"sslmode": "prefer"}, + }, + }, + wantErr: false, + }, { name: "found connection URI without database, verified", s: Scanner{detectLoopback: true}, @@ -222,7 +241,7 @@ func TestPostgres_FromChunk(t *testing.T) { wantErr: false, }, { - name: "found connection URI, unverified due to error - ssl not supported", + name: "found connection URI, unverified due to error - ssl not supported (using sslmode)", s: Scanner{detectLoopback: true}, args: func() args { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) @@ -246,6 +265,31 @@ func TestPostgres_FromChunk(t *testing.T) { }(), wantErr: false, }, + { + name: "found connection URI, unverified due to error - ssl not supported (using requiressl)", + s: Scanner{detectLoopback: true}, + args: func() args { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + return args{ + ctx: ctx, + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s:%s/postgres2?requiressl=1`, postgresUser, postgresPass, postgresHost, postgresPort)), + verify: true, + } + }(), + want: func() []detectors.Result { + r := detectors.Result{ + DetectorType: detectorspb.DetectorType_Postgres, + Verified: false, + Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + ExtraData: map[string]string{"sslmode": "require"}, + } + r.SetVerificationError(pq.ErrSSLNotSupported) + return []detectors.Result{r} + }(), + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 570d21afddde1a2b705c159fbf4d544ac71def37 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 14:12:33 -0500 Subject: [PATCH 13/21] add special char to pw --- pkg/detectors/postgres/postgres_test.go | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 8913ce3cbff6..1d5eb8a4428e 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -25,7 +25,7 @@ var postgresDockerHash string const ( postgresUser = "postgres" - postgresPass = "23201dabb56ca236f3dc6736c0f9afad" + postgresPass = "23201da=b56ca236f3dc6736c0f9afad" postgresHost = "localhost" postgresPort = "5434" // Do not use 5433, as local dev environments can use it for other things @@ -91,8 +91,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -110,8 +110,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "prefer"}, }, }, @@ -129,8 +129,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "allow"}, }, }, @@ -148,8 +148,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "prefer"}, }, }, @@ -167,8 +167,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -231,8 +231,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://192.0.2.0:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://192.0.2.0:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://192.0.2.0:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://192.0.2.0:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": ""}, } r.SetVerificationError(errors.New("i/o timeout")) @@ -256,8 +256,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "require"}, } r.SetVerificationError(pq.ErrSSLNotSupported) @@ -281,8 +281,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201dabb56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), ExtraData: map[string]string{"sslmode": "require"}, } r.SetVerificationError(pq.ErrSSLNotSupported) From 544e9cfafef98dbc673f58d4e5357b5c2caa5509 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 14:24:16 -0500 Subject: [PATCH 14/21] fix order --- pkg/detectors/postgres/postgres.go | 2 +- pkg/detectors/postgres/postgres_test.go | 36 ++++++++++++------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index a25313e44e5c..e31d69c742f4 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -77,7 +77,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete params["port"] = "5432" } - raw := []byte(fmt.Sprintf("postgresql://%s:%s@%s:%s", host, port, user, password)) + raw := []byte(fmt.Sprintf("postgresql://%s:%s@%s:%s", user, password, host, port)) result := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 1d5eb8a4428e..02f4a25ee00d 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -91,8 +91,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -110,8 +110,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": "prefer"}, }, }, @@ -129,8 +129,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": "allow"}, }, }, @@ -148,8 +148,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": "prefer"}, }, }, @@ -167,8 +167,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: true, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -186,8 +186,8 @@ func TestPostgres_FromChunk(t *testing.T) { { DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://localhost:5434@postgres:inactive"), - RawV2: []byte("postgresql://localhost:5434@postgres:inactive"), + Raw: []byte("postgresql://postgres:inactive@localhost:5434"), + RawV2: []byte("postgresql://postgres:inactive@localhost:5434"), ExtraData: map[string]string{"sslmode": ""}, }, }, @@ -231,8 +231,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://192.0.2.0:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://192.0.2.0:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@192.0.2.0:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@192.0.2.0:5434"), ExtraData: map[string]string{"sslmode": ""}, } r.SetVerificationError(errors.New("i/o timeout")) @@ -256,8 +256,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": "require"}, } r.SetVerificationError(pq.ErrSSLNotSupported) @@ -281,8 +281,8 @@ func TestPostgres_FromChunk(t *testing.T) { r := detectors.Result{ DetectorType: detectorspb.DetectorType_Postgres, Verified: false, - Raw: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), - RawV2: []byte("postgresql://localhost:5434@postgres:23201da=b56ca236f3dc6736c0f9afad"), + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5434"), ExtraData: map[string]string{"sslmode": "require"}, } r.SetVerificationError(pq.ErrSSLNotSupported) From ef65b07c9b3994863cee149dc14896b02716314b Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 15:00:02 -0500 Subject: [PATCH 15/21] switch to consts --- pkg/detectors/postgres/postgres.go | 54 ++++++++++++++++--------- pkg/detectors/postgres/postgres_test.go | 25 ++++++++++++ 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index e31d69c742f4..d043506c924a 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -18,6 +18,21 @@ import ( const ( defaultPort = "5432" + + pg_connect_timeout = "connect_timeout" + pg_dbname = "dbname" + pg_host = "host" + pg_password = "password" + pg_port = "port" + pg_requiressl = "requiressl" + pg_sslmode = "sslmode" + pg_sslmode_allow = "allow" + pg_sslmode_disable = "disable" + pg_sslmode_prefer = "prefer" + pg_sslmode_require = "require" + pg_user = "user" + + sslmode_unset = "" ) // This detector currently only finds Postgres connection string URIs @@ -49,17 +64,17 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete candidateParamSets := findUriMatches(data) for _, params := range candidateParamSets { - user, ok := params["user"] + user, ok := params[pg_user] if !ok { continue } - password, ok := params["password"] + password, ok := params[pg_password] if !ok { continue } - host, ok := params["host"] + host, ok := params[pg_host] if !ok { continue } @@ -72,9 +87,10 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete } } - port, ok := params["port"] + port, ok := params[pg_port] if !ok { - params["port"] = "5432" + port = defaultPort + params[pg_port] = port } raw := []byte(fmt.Sprintf("postgresql://%s:%s@%s:%s", user, password, host, port)) @@ -89,17 +105,17 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete // do it for us - but we will do it anyway here so that when we later capture sslmode into ExtraData we will // capture it post-normalization. (The detector's behavior is undefined for candidate secrets that have both // requiressl and sslmode set.) - if requiressl := params["requiressl"]; requiressl == "0" { - params["sslmode"] = "prefer" + if requiressl := params[pg_requiressl]; requiressl == "0" { + params[pg_sslmode] = pg_sslmode_prefer } else if requiressl == "1" { - params["sslmode"] = "require" + params[pg_sslmode] = pg_sslmode_require } if verify { // pq appears to ignore the context deadline, so we copy any timeout that's been set into the connection // parameters themselves. if timeout := getDeadlineInSeconds(ctx); timeout != 0 { - params["connect_timeout"] = strconv.Itoa(timeout) + params[pg_connect_timeout] = strconv.Itoa(timeout) } isVerified, verificationErr := verifyPostgres(params) @@ -108,12 +124,12 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete } // We gather SSL information into ExtraData in case it's useful for later reporting. - sslmode := params["sslmode"] + sslmode := params[pg_sslmode] if sslmode == "" { - sslmode = "" + sslmode = sslmode_unset } result.ExtraData = map[string]string{ - "sslmode": sslmode, + pg_sslmode: sslmode, } if !result.Verified && detectors.IsKnownFalsePositive(password, detectors.DefaultFalsePositives, true) { @@ -165,14 +181,14 @@ func isErrorDatabaseNotFound(err error, dbName string) bool { } func verifyPostgres(params map[string]string) (bool, error) { - if sslmode := params["sslmode"]; sslmode == "allow" || sslmode == "prefer" { + if sslmode := params[pg_sslmode]; sslmode == pg_sslmode_allow || sslmode == pg_sslmode_prefer { // pq doesn't support 'allow' or 'prefer'. If we find either of them, we'll just ignore it. This will trigger // the same logic that is run if no sslmode is set at all (which mimics 'prefer', which is the default). - delete(params, "sslmode") + delete(params, pg_sslmode) // We still want to save the original sslmode in ExtraData, so we'll re-add it before returning. defer func() { - params["sslmode"] = sslmode + params[pg_sslmode] = sslmode }() } @@ -192,14 +208,14 @@ func verifyPostgres(params map[string]string) (bool, error) { return true, nil } else if strings.Contains(err.Error(), "password authentication failed") { return false, nil - } else if errors.Is(err, pq.ErrSSLNotSupported) && params["sslmode"] == "" { + } else if errors.Is(err, pq.ErrSSLNotSupported) && params[pg_sslmode] == "" { // If the sslmode is unset, then either it was unset in the candidate secret, or we've intentionally unset it // because it was specified as 'allow' or 'prefer', neither of which pq supports. In all of these cases, non-SSL // connections are acceptable, so now we try a connection without SSL to see if it works. - params["sslmode"] = "disable" - defer delete(params, "sslmode") // We want to return with the original params map intact (for ExtraData) + params[pg_sslmode] = pg_sslmode_disable + defer delete(params, pg_sslmode) // We want to return with the original params map intact (for ExtraData) return verifyPostgres(params) - } else if isErrorDatabaseNotFound(err, params["dbname"]) { + } else if isErrorDatabaseNotFound(err, params[pg_dbname]) { return true, nil // If we know this, we were able to authenticate } diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 02f4a25ee00d..9ae5a5a8f783 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -240,6 +240,31 @@ func TestPostgres_FromChunk(t *testing.T) { }(), wantErr: false, }, + { + name: "found connection URI, unverified due to error - wrong port", + s: Scanner{detectLoopback: true}, + args: func() args { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + return args{ + ctx: ctx, + data: []byte(fmt.Sprintf(`postgresql://%s:%s@%s/postgres2`, postgresUser, postgresPass, postgresHost)), + verify: true, + } + }(), + want: func() []detectors.Result { + r := detectors.Result{ + DetectorType: detectorspb.DetectorType_Postgres, + Verified: false, + Raw: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5432"), + RawV2: []byte("postgresql://postgres:23201da=b56ca236f3dc6736c0f9afad@localhost:5432"), + ExtraData: map[string]string{"sslmode": ""}, + } + r.SetVerificationError(errors.New("connection refused")) + return []detectors.Result{r} + }(), + wantErr: false, + }, { name: "found connection URI, unverified due to error - ssl not supported (using sslmode)", s: Scanner{detectLoopback: true}, From c88a2a392b19c9933d32b66d79af0f5153442397 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 15:00:37 -0500 Subject: [PATCH 16/21] tweak comment --- pkg/detectors/postgres/postgres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index d043506c924a..9d1f9aff36e7 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -211,7 +211,7 @@ func verifyPostgres(params map[string]string) (bool, error) { } else if errors.Is(err, pq.ErrSSLNotSupported) && params[pg_sslmode] == "" { // If the sslmode is unset, then either it was unset in the candidate secret, or we've intentionally unset it // because it was specified as 'allow' or 'prefer', neither of which pq supports. In all of these cases, non-SSL - // connections are acceptable, so now we try a connection without SSL to see if it works. + // connections are acceptable, so now we try a connection without SSL. params[pg_sslmode] = pg_sslmode_disable defer delete(params, pg_sslmode) // We want to return with the original params map intact (for ExtraData) return verifyPostgres(params) From 81395b0c5ac110cef9b7330af8f14154e944eee5 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 15:02:13 -0500 Subject: [PATCH 17/21] add comment --- pkg/detectors/postgres/postgres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 9d1f9aff36e7..6500b253c993 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -32,7 +32,7 @@ const ( pg_sslmode_require = "require" pg_user = "user" - sslmode_unset = "" + sslmode_unset = "" // Ths is how we report an unset sslmode in ExtraData (it's not a postgres string) ) // This detector currently only finds Postgres connection string URIs From 9aa8112dc8962df635a33fa1b88a52c58c47c8ab Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Mon, 22 Jan 2024 15:08:12 -0500 Subject: [PATCH 18/21] de-const thing --- pkg/detectors/postgres/postgres.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 6500b253c993..5e7b8187959b 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -31,8 +31,6 @@ const ( pg_sslmode_prefer = "prefer" pg_sslmode_require = "require" pg_user = "user" - - sslmode_unset = "" // Ths is how we report an unset sslmode in ExtraData (it's not a postgres string) ) // This detector currently only finds Postgres connection string URIs @@ -126,7 +124,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) ([]dete // We gather SSL information into ExtraData in case it's useful for later reporting. sslmode := params[pg_sslmode] if sslmode == "" { - sslmode = sslmode_unset + sslmode = "" } result.ExtraData = map[string]string{ pg_sslmode: sslmode, From 9fde821e2b01efd58f643f7827f1384e9e6ca512 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Tue, 23 Jan 2024 12:18:36 -0500 Subject: [PATCH 19/21] pre-allocate map --- pkg/detectors/postgres/postgres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 5e7b8187959b..9001e9b58aad 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -147,8 +147,8 @@ func findUriMatches(data []byte) []map[string]string { continue } - params := make(map[string]string) parts := connStrPartPattern.FindAllStringSubmatch(connStr, -1) + params := make(map[string]string, len(parts)) for _, part := range parts { params[part[1]] = part[2] } From fc9593f1f97e95a89d816f7faf345671ab9eedc8 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Tue, 23 Jan 2024 12:22:51 -0500 Subject: [PATCH 20/21] use switch --- pkg/detectors/postgres/postgres.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/detectors/postgres/postgres.go b/pkg/detectors/postgres/postgres.go index 9001e9b58aad..183b37d4c3b1 100644 --- a/pkg/detectors/postgres/postgres.go +++ b/pkg/detectors/postgres/postgres.go @@ -202,22 +202,23 @@ func verifyPostgres(params map[string]string) (bool, error) { defer db.Close() err = db.Ping() - if err == nil { + switch { + case err == nil: return true, nil - } else if strings.Contains(err.Error(), "password authentication failed") { + case strings.Contains(err.Error(), "password authentication failed"): return false, nil - } else if errors.Is(err, pq.ErrSSLNotSupported) && params[pg_sslmode] == "" { + case errors.Is(err, pq.ErrSSLNotSupported) && params[pg_sslmode] == "": // If the sslmode is unset, then either it was unset in the candidate secret, or we've intentionally unset it // because it was specified as 'allow' or 'prefer', neither of which pq supports. In all of these cases, non-SSL // connections are acceptable, so now we try a connection without SSL. params[pg_sslmode] = pg_sslmode_disable defer delete(params, pg_sslmode) // We want to return with the original params map intact (for ExtraData) return verifyPostgres(params) - } else if isErrorDatabaseNotFound(err, params[pg_dbname]) { + case isErrorDatabaseNotFound(err, params[pg_dbname]): return true, nil // If we know this, we were able to authenticate + default: + return false, err } - - return false, err } func (s Scanner) Type() detectorspb.DetectorType { From ff75b479f3d850825c7555359b2dd1d2ed1e1ca8 Mon Sep 17 00:00:00 2001 From: Cody Rose Date: Tue, 23 Jan 2024 12:24:50 -0500 Subject: [PATCH 21/21] remove unused vars --- pkg/detectors/postgres/postgres_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/detectors/postgres/postgres_test.go b/pkg/detectors/postgres/postgres_test.go index 9ae5a5a8f783..90c460b0aece 100644 --- a/pkg/detectors/postgres/postgres_test.go +++ b/pkg/detectors/postgres/postgres_test.go @@ -29,9 +29,7 @@ const ( postgresHost = "localhost" postgresPort = "5434" // Do not use 5433, as local dev environments can use it for other things - inactiveUser = "inactive" inactivePass = "inactive" - inactivePort = "61000" inactiveHost = "192.0.2.0" )