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

Regex for subject_alt_names fails on Windows due to \r\n as line break #264

Open
bastelflp opened this issue Aug 10, 2021 · 1 comment
Open

Comments

@bastelflp
Copy link

bastelflp commented Aug 10, 2021

Issue description

I am using acme-tiny on Windows to create new Let's Encrypt certificates with OpenSSL 1.1.1k 25 Mar 2021.

The certificate creation is working with one domain in the CSR, however it fails as soon as I added multiple domains as Subject Alternative Name (SAN) with the following error:

'detail': 'Error finalizing order :: Order includes different number of names than CSR specifies',
'status': 403

The log output for "Found domains" only shows domain, so the error is expected from there.

Cause

The cause is, that the SAN domains are not found by the regex (on my machine).

This is happening, as the openssl output contains \r\n as line breaks (on my machine) and the regex is only looking for \n as linebreaks.

Workaround / Fix

I could fix it (for me) by adding additional \r? to the regex.

Original code:

subject_alt_names = re.search(r"X509v3 Subject Alternative Name: (?:critical)?\n +([^\n]+)\n", out.decode('utf8'), re.MULTILINE|re.DOTALL)

subject_alt_names = re.search(r"X509v3 Subject Alternative Name: (?:critical)?\n +([^\n]+)\n", out.decode('utf8'), re.MULTILINE|re.DOTALL)

Workaround code (added \r? before every \n):

subject_alt_names = re.search(r"X509v3 Subject Alternative Name: (?:critical)?\r?\n +([^\r?\n]+)\r?\n", out.decode('utf8'), re.MULTILINE|re.DOTALL)

@diafygi
Copy link
Owner

diafygi commented Aug 21, 2021

@bastelflp Since we just moved to Github Actions for testing, it's now possible to add tests for Windows support.

https://github.com/diafygi/acme-tiny/blob/master/.github/workflows/full-tests-with-coverage.yml

Are you able to add a windows matrix entry to these tests?

If so, then this issue could get resolved. Otherwise (since I don't use windows), there's no way for me to test your proposed changes.

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

No branches or pull requests

2 participants