Skip to content

Conversation

creatorcary
Copy link
Contributor

@creatorcary creatorcary commented Oct 1, 2025

We ultimately need this to be in bytes form for the ACME package (see ticket).

This is part of the fallout from upgrading to trixie, specifically from acme upgrading from 2.11.0 to 4.0.0. This significantly changed the behavior of acme.client.ClientV2.new_order.

2.11.0 executed this helpful statement in the OpenSSL package:

    if isinstance(buffer, str):
        buffer = buffer.encode("ascii")

whereas 4.0.0 passes csr_pem directly to the cryptography module instead:

        csr = x509.load_pem_x509_csr(csr_pem)

However, even 2.11.0 specified that a bytes argument should have been passed instead of a string.

This will require changes in two other repositories:

@creatorcary creatorcary requested review from a team and sonicaj October 1, 2025 18:29
@bugclerk
Copy link

bugclerk commented Oct 1, 2025

@bugclerk bugclerk changed the title Return generated CSR as bytes NAS-137796 / 26.04 / Return generated CSR as bytes Oct 1, 2025
csr = add_extensions(csr, data.get('cert_extensions', {}), key, None)
csr = csr.sign(key, retrieve_signing_algorithm(data, key), default_backend())

return csr.public_bytes(serialization.Encoding.PEM).decode(), export_private_key_object(key)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Highlighting the main change for easier review

@sonicaj
Copy link
Member

sonicaj commented Oct 3, 2025

@creatorcary why cannot we just do the following ?

diff --git a/truenas_acme_utils/issue_cert.py b/truenas_acme_utils/issue_cert.py
index 664974d..f584284 100644
--- a/truenas_acme_utils/issue_cert.py
+++ b/truenas_acme_utils/issue_cert.py
@@ -21,7 +21,7 @@ def issue_certificate(
     acme_client, key = get_acme_client_and_key(acme_client_key_payload)
     try:
         # perform operations and have a cert issued
-        order = acme_client.new_order(csr)
+        order = acme_client.new_order(csr.encode())
     except messages.Error as e:
         raise CallError(f'Failed to issue a new order for Certificate : {e}')

@creatorcary creatorcary changed the title NAS-137796 / 26.04 / Return generated CSR as bytes NAS-137796 / 26.04 / Encode CSR before sending to OpenSSL Oct 6, 2025
@creatorcary creatorcary merged commit 8e89c55 into master Oct 7, 2025
2 checks passed
@creatorcary creatorcary deleted the NAS-137796 branch October 7, 2025 14:07
@bugclerk
Copy link

bugclerk commented Oct 7, 2025

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Oct 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants