server
server copied to clipboard
MariaDB
MDEV-33834 Add TLS version to audit plugin available variables
MDEV: https://jira.mariadb.org/browse/MDEV-33834
Description
Add tls_version and tls_version_length variables to the audit plugin so they can be logged. This is useful to help identify suspicious or malformed connections attempting to use unsupported TLS versions. A log with this information will allow to detect and block more malicious connection attempts.
Users with 'server_audit_events' empty will have these two new variables automatically visible in their logs, but if users don't want them, they can always configure what fields to include by listing the fields in 'server_audit_events'.
Release Notes
None. This just adds more possibilities for users.
How can this PR be tested?
Modified the server audit plugin to include this new info, and updated the server audit plugin MTR tests.
Basing the PR against the correct MariaDB version
- [X] This is a new feature and the PR is based against the latest MariaDB development branch.
PR quality check
- [X] I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
- [X] For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.
Copyright
All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
Requested changes have been made. The MDEV is assigned to @holyfoot to review, so I have assigned here too.
@Chupsy just to set the expectations: new features go into 10.6, we start working on 10.6 features in early May and finish it in min-June. Somewhere in that time @holyfoot should review and (if all good) merge your PR.
@LinuxJedi not sure I understand the changes that you have made to the PR, maybe another commit or modifying current commit message could help ?
@LinuxJedi not sure I understand the changes that you have made to the PR, maybe another commit or modifying current commit message could help ?
I did an automatic rebase onto the top of the branch. I have not altered the code. I also added the MDEV to the title of the PR, as this makes things much easier for us.
And yes, @vuvova would have meant 11.6.
Hello @LinuxJedi @vuvova, any update on the validation of this PR ? Do you need me to do some modifications ?
To repeat, what I wrote above (with a s/10/11/ typo fix):
@Chupsy just to set the expectations: new features go into 11.6, we start working on 11.6 features in early May and finish it in min-June. Somewhere in that time @holyfoot should review and (if all good) merge your PR.
So, if nothing happens now — it's normal, the PR is not forgotten, please, wait until May-June
Hi @vuvova, I'm going to rebase this to 11.6 since the jira ticket is now targetting 11.7 and there is no branch for 11.7
@Chupsy , I wouldn't worry about it, for the purpose of review it doesn't really matter whether it's 11.5 or 11.6 or 11.7, don't waste your time, let's get it approved first
@Chupsy a generic question, while I'm looking at the code. Why tls version? There are many parameters one might hypothetically want to know about a tls connection — version, certificate (fingerprint, issuer, subject, validity period), ciphers used, etc.
We cannot possibly add all that to the audit log. Why should it be a tls version, and not, say, what cipher was used? What should be the answer to a request to add more tls info, like the ciphers?
@Chupsy just a reminder. 11.7 windows closes in less than three weeks.
@vuvova this developer will no longer be contributing to MariaDB.
@vuvova @LinuxJedi I will take over this PR and address the feedback (generalize MTR test and output format) in a new PR, thanks for the review.
