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

CertFP broken on macOS #214

Closed
ljrk0 opened this issue Nov 28, 2023 · 0 comments
Closed

CertFP broken on macOS #214

ljrk0 opened this issue Nov 28, 2023 · 0 comments

Comments

@ljrk0
Copy link
Contributor

ljrk0 commented Nov 28, 2023

Hi there,

thank you for the most awesome IRC client :-)

Problem Statement

I've run into an issue when trying to setup SASL External on macOS. The code in question is part of the internal IRC implementation:

let pkcs12_archive = Identity::from_pkcs8(&bytes, &bytes)?;

The function from_pkcs8 receives two arguments, a PEM-encoded pem certificate-chain and the PKCS#8 (also basically PEM encoding) key. The former should contain the full certificate chain from the CA down to the user/leaf certificate, the latter the key for the leaf certificate. Using the instructions from Libera linked in the Wiki yields one PEM file combining private key and self-signed cert.

The macOS implementation is rather strict:

https://github.com/sfackler/rust-native-tls/blob/5db1b7772604d255381d847db5dd3a5b501c30a6/src/imp/security_framework.rs#L91

  1. It requires -----BEGIN PRIVATE KEY to be the first bytes in key the file, otherwise it'll return errSecParam or "One or more parameters passed to a function were not valid."
  2. It doesn't accept duplicate items, you'll get "The specified item already exists in the keychain."

Rust Native TLS API

Especially the latter completely breaks this implementation, due to the same values being passed to key and pem. Additionally, however, since from_pkcs8 is used on files which do not only contain key material, this may break in other ways as well.

Before c6d7e66 instead of from_pkcs8 the function from_pkcs12 was called. The PKCS#12 standard indeed allows for "everything" to be combined in one file, as it embeds not only the certificate chain but also can embed a PKCS#8 key file.

In result, an "Identity" in native-tls terms can be either constructed from a

  1. PKCS#12 file containing both the certificate chain and the key for the leaf, or a
  2. PKCS#8 file for the key only plus a PEM file with the certificate chain.

Unfortunately, this API doesn't easily allow for the format described in the Libera API which a PEM file (thus not PKCS#12) containing all required data.

Going Forward

I suppose the ideal solution is to:

  1. Add another entry sasl.external.key which contains the PKCS#8 key and have sasl.external.cert only contain the user certificate + chain.
    • In order to stay backwards-compatible, perhaps set key = cert if key is lacking? But this'd be permanently broken on macOS
  2. Perhaps add an entry sasl.external.combined which can contain a PKCS#12 combined file and would conflict with .cert or .key being set.

Note that it's virtually impossible to load the format described by Libera using Native TLS portably since especially the macOS version has many sanity checkers that e.g., reject certificates which also contains keys: https://github.com/sfackler/rust-native-tls/blob/5db1b7772604d255381d847db5dd3a5b501c30a6/src/imp/security_framework.rs#L213

Ideally, Native TLS would be consistent across platforms here, either always discarding unused data (as done with the OpenSSL backend) or complaining if a certificate contains a key, a key contains a certificate or certs are duplicated.

ljrk0 added a commit to ljrk0/halloy that referenced this issue Nov 28, 2023
@tarkah tarkah closed this as completed in 5ded34e Feb 5, 2024
tarkah added a commit that referenced this issue Feb 5, 2024
[DRAFT] Fix #214: Separate user cert-chain from PKCS#8 key
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

1 participant