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

MDEV-33863: New mysqladmin command tls-info #3247

Open
wants to merge 7 commits into
base: 11.4
Choose a base branch
from
Open

Conversation

9EOR9
Copy link
Contributor

@9EOR9 9EOR9 commented May 8, 2024

  • The Jira issue number for this PR is: MDEV-3383

Description

Added a new command tls-info for mysqladmin client tool, which provides information for the TLS connection in use.

Release Notes

mysqladmin: Added a new command tls-info which returns the following information:

  • TLS ciphersuite
  • TLS protocol
  • Peer certificate information:
    • Version
    • Subject
    • Issuer
    • Valid not before/not after
    • SHA256 fingerprint

How can this PR be tested?

There is no test for, since the output depends on the certificate used by peer.
Feature can be tested by executing mysqladmin with command tls-info.

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.
  • This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced.

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.

Since TLSv1.0 is no longer supported in C/C 3.4, TLS1.0 tests which
were expected to fail are removed.
Added new command tls-info which provides the following
information:

Cipher suite in use
TLS protocol version
Peer certificate information:
  - Version
  - Subject
  - Issuer
  - Valid not before/after
  - SHA256 finger print
Since TLSv1.0 is no longer supported in C/C 3.4, TLS1.0 tests which
were expected to fail are removed.
Added new command tls-info which provides the following
information:

Cipher suite in use
TLS protocol version
Peer certificate information:
  - Version
  - Subject
  - Issuer
  - Valid not before/after
  - SHA256 finger print
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Member

@grooverdan grooverdan left a comment

Choose a reason for hiding this comment

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

Cosmetic changes but otherwise ok.
So as a new feature this will go into testing for the 11.6 release.

new_line=1;
char *version;

printf("Cipher suite:\t%s\n", mysql_get_ssl_cipher(mysql));
Copy link
Member

Choose a reason for hiding this comment

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

This is calling the same pure function a second time, but I guess its ok since the C/C function is so shallow.

info->not_after.tm_hour, info->not_after.tm_min);
printf("SHA256 fingerprint: %s\n", info->fingerprint);
} else {
my_printf_error(0, "Unable to retrieve peer certificate", 0);
Copy link
Member

Choose a reason for hiding this comment

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

indenting error. And style would say { on the next line.

Choose a reason for hiding this comment

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

Just for verbosity the CODING STANDARDS link in the PR template provides the coding style guideline and coding standards in general.

my_printf_error(0, "Unable to retrieve peer certificate", 0);
return 1;
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

same here.

@@ -1 +1 @@
--tls_version=TLSv1.0

Choose a reason for hiding this comment

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

Nice 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants