facebookincubator/fizz

Handshake failed when ChaCha20 comes first in the cipher suite list but without ChaCha20 support

Nefurtity opened this issue · 1 comments

Precondition: A server deployed fizz compilation with no ChaCha20

Situation: A client send a Client Hello with Cipher Suits in the following order

  1. TLS_CHACHA20_POLY1305_SHA256 (0x1303)
  2. TLS_AES_128_GCM_SHA256 (0x1301)
  3. TLS_AES_256_GCM_SHA384 (0x1302)

The handshake failed because fizz still picks up TLS_CHACHA20_POLY1305_SHA256 even though ChaCha20 is not available, otherwise the next cipher TLS_AES_128_GCM_SHA256 should be chosen.

virtual std::unique_ptr<Aead> makeAead(CipherSuite cipher) const {
    switch (cipher) {
      case CipherSuite::TLS_CHACHA20_POLY1305_SHA256:
        return std::make_unique<OpenSSLEVPCipher<ChaCha20Poly1305>>();
      case CipherSuite::TLS_AES_128_GCM_SHA256:
        return std::make_unique<OpenSSLEVPCipher<AESGCM128>>();
      case CipherSuite::TLS_AES_256_GCM_SHA384:
        return std::make_unique<OpenSSLEVPCipher<AESGCM256>>();
      case CipherSuite::TLS_AES_128_OCB_SHA256_EXPERIMENTAL:
        return std::make_unique<OpenSSLEVPCipher<AESOCB128>>();
      default:
        throw std::runtime_error("aead: not implemented");
    }
  }

ChaCha20Poly1305 should not return if fizz compiled with no ChaCha20.

Can you clarify how you are configuring the fizz context, and how you are removing chacha20? The default configuration should not have chacha in the supported ciphers list if chacha is not available. If you are explicitly adding chacha as a supported cipher, while also compiling without chacha, that's more of a configuration error, though we can likely provide more config validation to prevent that case.