server icon indicating copy to clipboard operation
server copied to clipboard

Fix(wsrep_sst_rsync.sh): get CN

Open sudo-Tiz opened this issue 1 year ago • 2 comments

Description

The script retrieves the subject of the certificate using the command openssl x509 -noout -subject -in "$SSTCERT" and then pipes it to extract the CN. However, the current method for extracting the CN encounters issues when the CN is the first field of the subject. This pull request aims to address this issue and ensure the successful extraction of the CN from the subject of the certificate, regardless of its position within the subject field.

Here are the lines (864-866) that extracts the CN:

CN=$("$OPENSSL_BINARY" x509 -noout -subject -in "$SSTCERT" | tr ',' '\n' | grep -F 'CN =' | cut -d '=' -f2 | sed s/^\ // | sed s/\ %//)

This method of extracting the CN works when the CN is not the first field of the subject (e.g., subject=OU = Example, CN = www.example.com, C = EX), but it fails when the CN is the first field (e.g., subject=CN = www.exemple.com).

As the ACME challenge can only validate the CN of the certificate, this is the only field in the subject of the certificates produced by certbot.

A simple fix is to remove "subject=" from this return, and the pipe works again.

Release Notes

Fix the command to get CN in wsrep_sst_rsync when the CN field is the first field of the subject of the certificate

How can this PR be tested?

Using a certificate which only contain the CN field in it's subject.

Basing the PR against the correct MariaDB version

  • [ ] This is a new feature and the PR is based against the latest MariaDB development branch.
  • [x] 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.

sudo-Tiz avatar Apr 15 '24 10:04 sudo-Tiz

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 15 '24 10:04 CLAassistant

@sudo-Tiz , just going through PRs and it looks like https://github.com/MariaDB/server/commit/288ea9e146a238872998d7089070e82f39272728#diff-4cced5819e96bfdff765ccc8d3002156f4d80473e76743d8760582dbcee79fceR1586 has been commit that fixes this also. Can you check it works correctly?

Sorry your PR didn't get noticed it seems by @sysprg who committed the change.

grooverdan avatar Apr 30 '24 01:04 grooverdan

Hey @grooverdan, I've tested the fix provided by @sysprg, and it works for the scenario I described.

@LinuxJedi, the fix from @sysprg is included in the 10.4, 10.5, 10.6, and the latest branches, which is exactly what I needed. I'm eagerly awaiting the release.

Since browsers no longer primarily use the CN (or only as a fallback) but the SAN to extract the Domain Name (RFC6125), MariaDB should do the same and search for the first dNSNames field in the SAN. However, this is for another PR.

I'm closing this.

sudo-Tiz avatar May 14 '24 08:05 sudo-Tiz