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

Fix certificates exporting with LibreSSL #427

Merged
merged 4 commits into from
Sep 16, 2024

Conversation

priitlatt
Copy link
Contributor

@priitlatt priitlatt commented Sep 16, 2024

Fixes #419

PR #411 introduced a regression when exporting Apple code signing certificates to PKCS#12 containers.

Just checking openssl version to decide whether -noenc option is supported for PKCS#12 export can yield misleading results as there are different implementations whose versioning schemes and APIs do not agree with each other.

For example OpenSSL 3.0.0+ has deprecated -nodes in favor of -noenc, while LibreSSL 3.3.6. still uses -nodes and doesn't even recognize -noenc.

To choose correct flag to skip private key encryption for openssl pkcs12, just check the help message and see if it contains -noenc, otherwise use the legacy -nodes.

Updated actions:

  • app-store-connect certificates create,
  • app-store-connect certificates get,
  • app-store-connect certificates list,
  • app-store-connect fetch-signing-files.

QA notes

Tested on systems with different openssl installations
% openssl version
LibreSSL 3.3.6
% openssl version
OpenSSL 3.3.1 4 Jun 2024 (Library: OpenSSL 3.3.1 4 Jun 2024)

@priitlatt priitlatt added the bug Something isn't working label Sep 16, 2024
@priitlatt priitlatt marked this pull request as ready for review September 16, 2024 12:32
Copy link
Contributor

@mikhail-tokarev mikhail-tokarev left a comment

Choose a reason for hiding this comment

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

LGTM

capture_output=True,
check=False,
)
self.__SUPPORTS_NOENC__ = b"-noenc" in completed_process.stdout or b"-noenc" in completed_process.stderr
Copy link
Contributor

Choose a reason for hiding this comment

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

just out of curiosity, what are the cases when help text goes to stderr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent question actually. LibreSSL doesn't support neither -help nor help and emits the usage to stderr:

% openssl version
LibreSSL 3.3.6
% openssl pkcs12 help
unknown option 'help'
usage: pkcs12 [-aes128 | -aes192 | -aes256 | -camellia128 |
    -camellia192 | -camellia256 | -des | -des3 | -idea]
    [-cacerts] [-CAfile file] [-caname name]
    [-CApath directory] [-certfile file] [-certpbe alg]
    [-chain] [-clcerts] [-CSP name] [-descert] [-export]
    [-in file] [-info] [-inkey file] [-keyex] [-keypbe alg]
    [-keysig] [-LMK] [-macalg alg] [-maciter] [-name name]
    [-nocerts] [-nodes] [-noiter] [-nokeys] [-nomac]
    [-nomaciter] [-nomacver] [-noout] [-out file]
    [-passin arg] [-passout arg] [-password arg] [-twopass]

 -aes128            Encrypt PEM output with CBC AES
 ...
 -nodes             Don't encrypt private keys
 ...

Perhaps this requires even a code comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment about it in 1af5984.

Copy link
Contributor

Choose a reason for hiding this comment

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

ha, that makes sense! and yes, commend in the code is totally helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

but you search for -noenc in stderr just because you expect that LibreSSL will support it at some point but still without support of -help, is that correct? otherwise we can just check the first condition and immeddiately return the result.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My hope (or assumtion?) is that maybe some day LibreSSL will follow OpenSSL and then both will support -noenc. As it is now, we'd get automatic support to the updated version.

@@ -54,6 +54,9 @@ def _is_noenc_flag_supported(self) -> bool:
capture_output=True,
check=False,
)
# Check both stdout and stderr because LibreSSL doesn't have help commands per-se.
# Execution fails, and it just outputs "unknown option '-help'" along with full
Copy link
Contributor

Choose a reason for hiding this comment

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

for the sake of accuracy, the output is unknown option 'help' (no dash)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the output is indeed "unknown option '-help'", because the openssl pkcs12 is called with -help argument that OpenSSL supports.

Copy link
Contributor

Choose a reason for hiding this comment

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

true

@priitlatt priitlatt merged commit ff87078 into master Sep 16, 2024
11 checks passed
@priitlatt priitlatt deleted the bugfix/export-certificates-using-libressl branch September 16, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: '-noenc' is an Unknown Option for OpenSSL PKCS12 on Self-Hosted MacOS Runner
3 participants