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

Server selects certificate based on client's support #17

Closed
tegefaulkes opened this issue Apr 26, 2023 · 4 comments
Closed

Server selects certificate based on client's support #17

tegefaulkes opened this issue Apr 26, 2023 · 4 comments
Assignees
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices

Comments

@tegefaulkes
Copy link
Contributor

Specification

We need the ability to provide an alternative certificate if the ed25519 certificate is not supported by the client. We will need this if we want to serve web pages to web browsers.

This may be implemented using the following boring SSL config options.

  1. https://docs.rs/boring/latest/boring/ssl/struct.SslContextBuilder.html#method.set_verify_algorithm_prefs
  2. https://docs.rs/boring/latest/boring/ssl/struct.SslContextBuilder.html#method.set_select_certificate_callback

Some other options may be needed. I'll need to look into this more.

Additional context

Tasks

  1. Determine if it's possible to select server certificate based on client's support for it.
  2. Implement a method for providing an alternative certificate if ed25519 is not supported.
@tegefaulkes tegefaulkes added the development Standard development label Apr 26, 2023
@CMCDragonkai
Copy link
Member

@tegefaulkes
Copy link
Contributor Author

Should this be a part of the #1 Epic? We don't need this until we want to serve a web-page so it's far from critical for now.

@CMCDragonkai
Copy link
Member

Leaving some notes here as I have found that Rust's boring crate doesn't appears to support this.

Node's own TLS does support this.

The relevant API looks like this:

  /**
   * Private key as a PEM string or Uint8Array buffer containing PEM formatted
   * key. You can pass multiple keys. The number of keys must match the number
   * of certs. Each key must be associated to the the corresponding cert chain.
   */
  key?: string | Array<string> | Uint8Array | Array<Uint8Array>;

  /**
   * X.509 certificate chain in PEM format or Uint8Array buffer containing
   * PEM formatted certificate chain. Each string or Uint8Array is a
   * certificate chain in subject to issuer order. Multiple certificate chains
   * can be passed. The number of certificate chains must match the number of
   * keys. Each certificate chain must be associated to the corresponding key.
   */
  cert?: string | Array<string> | Uint8Array | Array<Uint8Array>;

This is how we combine them:

  // This is an array of private keys in PEM format
  let keyPEMBuffers: Array<Uint8Array> | undefined;
  if (config.key != null) {
    const keyPEMs: Array<string> = [];
    if (typeof config.key === 'string') {
      keyPEMs.push(config.key.trim() + '\n');
    } else if (config.key instanceof Uint8Array) {
      keyPEMs.push(textDecoder.decode(config.key).trim() + '\n');
    } else if (Array.isArray(config.key)) {
      for (const k of config.key) {
        if (typeof k === 'string') {
          keyPEMs.push(k.trim() + '\n');
        } else {
          keyPEMs.push(textDecoder.decode(k).trim() + '\n');
        }
      }
    }
    keyPEMBuffers = keyPEMs.map((k) => textEncoder.encode(k));
  }
  // This is an array of certificate chains in PEM format
  let certChainPEMBuffers: Array<Uint8Array> | undefined;
  if (config.cert != null) {
    const certChainPEMs: Array<string> = [];
    if (typeof config.cert === 'string') {
      certChainPEMs.push(config.cert.trim() + '\n');
    } else if (config.cert instanceof Uint8Array) {
      certChainPEMs.push(textDecoder.decode(config.cert).trim() + '\n');
    } else if (Array.isArray(config.cert)) {
      for (const c of config.cert) {
        if (typeof c === 'string') {
          certChainPEMs.push(c.trim() + '\n');
        } else {
          certChainPEMs.push(textDecoder.decode(c).trim() + '\n');
        }
      }
    }
    certChainPEMBuffers = certChainPEMs.map((c) => textEncoder.encode(c));
  }

On the rust code, I'm going to only select the first one. All the other ones we will ignore for now, as it is not possible until boring package itself starts supporting perhaps the SSL_CTX_set_cert_cb as expected by quiche.

Some example C code:

#include <openssl/ssl.h>

int select_certificate(SSL *ssl, int *ad, void *arg) {
    /* Here, arg should be a structure containing your certificates */
    MyCertStorage *storage = (MyCertStorage *)arg;
    const char *suitename = SSL_CIPHER_get_name(SSL_get_current_cipher(ssl));

    if (strstr(suitename, "ECDSA")) {
        SSL_set1_chain(ssl, storage->ecdsa_chain);
        SSL_use_PrivateKey(ssl, storage->ecdsa_private_key);
    } else if (strstr(suitename, "RSA")) {
        SSL_set1_chain(ssl, storage->rsa_chain);
        SSL_use_PrivateKey(ssl, storage->rsa_private_key);
    } else {
        /* Unsupported suite */
        return 0;
    }

    return 1;
}

int main() {
    SSL_CTX *ctx;

    OpenSSL_add_all_algorithms();
    ERR_load_BIO_strings();
    ERR_load_crypto_strings();
    SSL_load_error_strings();

    if (SSL_library_init() < 0)
        return 1;

    if ((ctx = SSL_CTX_new(SSLv23_server_method())) == NULL)
        return 1;

    /* Load your certificates into MyCertStorage structure here */
    MyCertStorage storage;
    /* Load the RSA and ECDSA keys into storage */

    SSL_CTX_set_cert_cb(ctx, select_certificate, &storage);

    /* ... */

    return 0;
}

@CMCDragonkai
Copy link
Member

The conversation from #26 (comment) to #26 (comment) shows that while boringssl the C++/C code supports it, and boring-sys crate should be exporting these functions, the boring wrapper crate itself does not support this.

Yea I'm pretty sure that the current boring crate does not provide a straight forward way to do this multiple certificate chain.

Also I did find a good resource that explains what all the boringssl functions do: https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html. It's much more clearer than all the other manpages.

In particular this https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_CTX_set_cert_cb is actually one of the main ways in which one could potentially select the correct certificate.

So for now I'm going to disable the ability to set multiple certificates on the Rust side.

So atm, without patching boring, this is not possible.

The only reason to have this would be to allow our QUIC system present multiple different certificates... which isn't really necessary atm. In PK, the only time we want to use something other the Ed25519 cert is for the HTTP status page or perhaps the websocket API if we expect browser integration there.

So for now, this will be a close as not planned. But can be a feature we revisit in the future.

@CMCDragonkai CMCDragonkai closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2023
@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices label Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 4 End to End Networking behind Consumer NAT Devices
Development

No branches or pull requests

2 participants