Repository navigation
Add pgbouncer authentication failure log format to pgsql-logs (#1881) - #1951
Conversation
| @@ -1,4 +1,4 @@ | |||
| len(results["s01-parse"]["crowdsecurity/pgsql-logs"]) == 6 | |||
| len(results["s01-parse"]["crowdsecurity/pgsql-logs"]) == 8 | |||
There was a problem hiding this comment.
Looks like the assert file was only partially regenerated?
It's missing the full asserts for the 2 new lines
There was a problem hiding this comment.
Done! Added the missing assertions for both PgBouncer log lines and verified that all tests are passing locally.
sabban
left a comment
There was a problem hiding this comment.
hi,
THank you for your contribution. Here's a review and please add a piece of doc to explain how to configure the datasource
| - grok: | ||
| #pgbouncer SASL authentication failed | ||
| pattern: '%{TIMESTAMP_ISO8601:timestamp} %{WORD:zone} \[%{INT:PID}\] LOG C-%{NOTSPACE:connection_id}: %{DATA:pgsql_dbname}/%{DATA:pgsql_target_user}@%{IP:source_ip}:%{INT:source_port} closing because: %{WORD:auth_method} authentication failed' | ||
| apply_on: message |
There was a problem hiding this comment.
It seems that you are counting each failed login twice.
There was a problem hiding this comment.
Fixed in ca7572d. I removed the duplicate grok pattern for connection closing. Only the initial ERROR log line is now parsed to prevent duplicate counting, and I added a test assertion verifying that closing lines are ignored.
| apply_on: message | ||
| - grok: | ||
| #pgbouncer password authentication failed | ||
| pattern: '%{TIMESTAMP_ISO8601:timestamp} %{WORD:zone} \[%{INT:PID}\] ERROR C-%{NOTSPACE:connection_id}: %{DATA:pgsql_dbname}/%{DATA:pgsql_target_user}@%{IP:source_ip}:%{INT:source_port} %{WORD:auth_method} authentication failed' |
There was a problem hiding this comment.
I believe this is not ipv6 compatible. You can use [? and ]? to add optional brackets
There was a problem hiding this comment.
Fixed in ca7572d. I updated the pattern to @[?%{IP:source_ip}]?:%{INT:source_port} and added test cases verifying bracketed IPv6 parsing (@[2001:db8::1]:port).
| filter: "evt.Parsed.program == 'postgres'" | ||
| description: "Parse PgSQL and PgBouncer logs" | ||
| filter: "evt.Parsed.program in ['postgres', 'pgbouncer']" | ||
| nodes: |
There was a problem hiding this comment.
why adding pgbouncer here as there's the logs you added still have postgres ?
There was a problem hiding this comment.
I kept pgbouncer in the filter so both file acquisition (labels.type: pgbouncer) and syslog/journald (where program name is pgbouncer) are supported. I added a dedicated test suite under .tests/pgbouncer-logs/ with log_type: pgbouncer (testing IPv4, IPv6, ignoring close logs, and bruteforce scenario overflow), while restoring .tests/pgsql-logs/ to focus purely on PostgreSQL.
- Prevent double counting by removing duplicate closing because grok pattern - Support bracketed IPv6 addresses in PgBouncer grok pattern - Add dedicated pgbouncer-logs hub test with log_type pgbouncer - Add datasource configuration documentation in pgsql-logs.md
f11eea9 to
ca7572d
Compare
|
Hi @sabban, Thank you for the detailed review! All requested points have been addressed in commit ca7572d:
All hub tests ( |
sabban
left a comment
There was a problem hiding this comment.
one last question before merging
|
|
||
| ```yaml | ||
| journalctl_filter: | ||
| - _SYSTEMD_UNIT=pgbouncer.service |
There was a problem hiding this comment.
did you you actually test this setup ?
There was a problem hiding this comment.
Good catch, you are completely right. I had only validated the file-based configuration (type: pgbouncer on /var/log/postgresql/pgbouncer.log, which is tested under .tests/pgbouncer-logs), and added the journalctl snippet speculatively (which was also missing source: journalctl).
Since syslog/journald setups often format or strip application prefixes differently depending on daemon setup and haven't been validated end-to-end here, I removed the journalctl block so the documentation focuses strictly on the tested file-based acquisition, consistent with the PostgreSQL section.
Description
This PR adds parsing support for PgBouncer authentication failure log formats (both password authentication and SASL failures), closing #1881.
Details
filterto accept bothpostgresandpgbouncerprograms:evt.Parsed.program in ['postgres', 'pgbouncer'].ERROR C-... password authentication failed.LOG C-... closing because: SASL authentication failed.timestamp,source_ip,source_port,pgsql_target_user,pgsql_dbname, andauth_method.log_type: pgsql_failed_auth, allowing automatic correlation with the existingcrowdsecurity/pgsql-bfbrute-force detection scenario..tests/pgsql-logs/.Closes #1881
Checklist