Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugin: support logging rejected connection attempts in audit log #14594

Merged
merged 3 commits into from
Feb 4, 2020
Merged

plugin: support logging rejected connection attempts in audit log #14594

merged 3 commits into from
Feb 4, 2020

Conversation

lysu
Copy link
Contributor

@lysu lysu commented Feb 3, 2020

What problem does this PR solve?

fixes #14563

releated PR in plugin https://github.com/pingcap/enterprise-plugin/pull/25

What is changed and how it works?

  • add new reject audit type
  • pass reject reason via ctx

Check List

Tests

  • Manual test (add detailed scripts or steps below)
build plugin and load it with new audit plugin 
login fail
see 

[2020/02/03 12:33:54.921 +08:00] [INFO] [logger.go:70] [ID=15807044340] [TIMESTAMP=2020/02/03 12:33:54.921 +08:00] [EVENT_CLASS=CONNECTION] [EVENT_SUBCLASS=Reject] [STATUS_CODE=0] [COST_TIME=0] [HOST=127.0.0.1] [CLIENT_IP=127.0.0.1] [USER=root] [DATABASES="[test]"] [TABLES="[]"] [SQL_TEXT=] [ROWS=0] [CLIENT_PORT=34160] [CONNECTION_ID=2] [CONNECTION_TYPE=Socket] [SERVER_ID=1] [SERVER_PORT=4000] [DURATION=0] [SERVER_OS_LOGIN_USER=robi] [OS_VERSION="Linux 5.3.18-1-MANJARO.x86_64"] [CLIENT_VERSION=] [SERVER_VERSION=v4.0.0-beta-51-gc46044f02-dirty] [AUDIT_VERSION=] [SSL_VERSION=v1.2.0] [PID=30808] [Reason="[server:1045]Access denied for user 'root'@'127.0.0.1' (using password: YES)"]

in audit log

Code changes

  • Has exported function/method change

Side effects

  • need cherry-pick to keep 3.0, 2.1 CI works

Related changes

  • Need to cherry-pick to the release branch

Release note

  • support logging rejected connection attempts in audit log.

This change is Reviewable

@lysu lysu added component/plugin type/enhancement The issue or PR belongs to an enhancement. labels Feb 3, 2020
@lysu
Copy link
Contributor Author

lysu commented Feb 3, 2020

/rebuild

@lysu
Copy link
Contributor Author

lysu commented Feb 3, 2020

@kolbe Hi, I tried and found maybe we'd better just add new Event Type 'Reject' to manifest, so EventType will be

  • PreAuth
  • Connected
  • Reject
  • Disconnect
  • ChangeUser

maybe it's more uniform for log output.

and here I choose pass reject reason by context.Context, in mysql it often use threadlocal variable to do that, but golang doesn't have that, but context.Context maybe suitable for this? 😞 we are hard to predefine all params for plugins, pre-define a context param maybe ok?

PTAL if free 😄

@lysu lysu requested a review from kolbe February 3, 2020 05:11
Copy link
Contributor

@kolbe kolbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lysu lysu requested a review from jackysp February 4, 2020 01:52
@lysu lysu added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 4, 2020
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jackysp
Copy link
Member

jackysp commented Feb 4, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 4, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 4, 2020

/run-all-tests

@sre-bot sre-bot merged commit a644d67 into pingcap:master Feb 4, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 4, 2020

cherry pick to release-3.0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/plugin status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support logging rejected connection attempts in audit log
4 participants