[DRAFT] Fix #214: Separate user cert-chain from PKCS#8 key

[ljrk0]

This adds a new config option sasl.external.key which is required to properly support SASL External using PKCS#8. PKCS#8 is a format that's used exclusively for keys and not user-certs. As such, when using from_pkcs8() from native-tls, both a user-cert (in PEM) and the PKCS#8 key must be provided. This was implemented before through one file which contained both the PKCS#8 key and the user-cert, concatenated. Strict implementations will either complain about the key-file containing certificates, vice-versa or duplicate certificates: since "both" the key-file and cert-file contain the same certificates due to being identical.

The code is probably not ideal, e.g.,

1. the external_cert and external_key functions could possibly be merged, or
2. the fields client_cert_path and client_key_path in Security be wrapped as a tuple (client_cert_path, client_key_path) with similar changes
3. there could be support for combined PKCS#8 key files + PEM cert chains, as well as
4. support for PKCS#12 combined identity files.

Further, the documentation in the Wiki linking to Libera may need to be updated.

[casperstorm]

@tarkah could you take a look at this? 👀
Relevant issue: #214.

[tarkah]

Sorry for the delay on this, thank you for the PR! After reviewing things further, the change makes sense.

I'd like to see us not break backwards compatibility however. See my other comment.

[tarkah]

Can we make key an Option and return cert path here if key is none? This way we preserve backwards compatibility.

[ljrk0]

Hey, sure can do! Just to make sure I understand correctly:

The function external_key is currently already returning an Option, so what you're suggesting is to do another fallback check like this?

- } else {
+ else if let Self::External { cert, .. } = self {
+    return Some(cert) //fallback
+ } else {
      return None
  }

I'm currently mostly worried about odd error cases arising from this, when people use the same config from Linux to macOS/Windows and getting weird error messages -- but I don't have any better idea.

[tarkah]

I've implemented this patch on top of your work and confirmed it:

• Works w/ previous config (only cert defined w/ cert+key combined file)
• Works w/ separate cert + key files / entries in config

I'm not sure how to advise people on this, other than maybe we update our wiki? At least we don't have breakage for users migrating to this newer patched version and now support separately defined cert / keys for "proper" usage, which ideally people migrate to.

[ljrk0]

Ah, sorry for being MIA for some time! Sounds great!

Yeah, I was wondering whether one could add some more checking, because it'd be a weird error-case to have a config that works on Linux but you use it on macOS and it breaks (with weird TLS errors, too). Ideally, maybe one could issue a warning for combined files, going forward?