EAP-FAST PRF hooks and BoringSSL

[Jouni Malinen]

On Tue, May 10, 2016 at 01:28:22PM -0400, David Benjamin wrote:
> So, you all have this logic to reimplement the TLS PRF and key
> derivation in src/crypto/tls_openssl.c in openssl_get_keyblock_size
> and openssl_tls_prf. It appears these are used for key derivation in
> EAP-TLS, EAP-TTLS, and EAP-FAST. EAP-(T)TLS can just use RFC 5705
> exporters by way of SSL_export_keying_material, which you use. But
> EAP-FAST is a bit of a nightmare. Among other sins, it hooks into
> TLS's internal key derivation, so you all understandably need to do
> some invasive wrangling.
> 
> We took the EAP-FAST bits out of BoringSSL a long time ago. (The way
> it integrated into session resumption was problematic. I know how I'd
> redo it if I had to, but there hasn't been a need yet.) But to keep
> wpa_supplicant compiling, we have to keep some fields around
> (ssl->enc_read_ctx, ssl->read_hash, and
> ssl->s3->tmp.new_mac_secret_size) even though we never fill them in.

There is a lot of history behind this with EAP-FAST being supported with
OpenSSL 0.9.8 using a custom patch and OpenSSL not yet having
SSL_export_keying_material() available at the time.. And obviously the
EAP-FAST PRF being pretty horrible to implement outside the TLS library.

> 1. I can condition the EAP-FAST-only parts of the key derivation
> conditioned on EAP_FAST #defines. With
> df5bde83daaf3c820b6ac1f8d383b16a64ebd2db, we're finally no longer
> defining those symbols, so this would work. I would probably, to make
> this cleaner, split the tls_connection_prf entry-point into two
> functions, one for the reasonable RFC 5705 mechanisms and one for
> EAP-FAST's crazy thing. (Right now the API supports the RFC 5705
> style, but with skip_keyblock set, which is a little odd.)

Now that all supported OpenSSL versions (support for 0.9.8 and 1.0.0 was
dropped) do have SSL_export_keying_material(), doing the additional
cleanup here with a simple tls_connection_export_key() makes sense to
me.

> 2. For OpenSSL 1.1.x, you all use SSL_CIPHER_get_cipher_nid and
> SSL_CIPHER_get_digest_nid. BoringSSL doesn't have these, but I have no
> problems adding them. (I would not have used NIDs for
> SSL_CIPHER_get_kx_nid and SSL_CIPHER_get_auth_nid but whatever.) It's
> a little odd because the computation is wrong for TLS 1.1+ and we
> don't use EVP_CIPHER in our SSL code anymore. But the code will never
> run anyway, and if it were to later, I could make it match OpenSSL.

I didn't really like implementing key block calculations for OpenSSL
1.1.x (well, it is better than the older version, but still..), so I
don't see much need for BoringSSL to implement these.

> 3. For other reasons, we added a pair of functions,
> SSL_get_key_block_len and SSL_generate_key_block which are actually
> exactly what EAP-FAST wants here. I could make
> openssl_get_keyblock_size call SSL_get_key_block_len and even make
> openssl_tls_prf use SSL_generate_key_block. (Probably this would come
> with a similar tls_connection_prf split as in option #1.) But this
> would be BoringSSL-only code to support a feature that doesn't work in
> BoringSSL anyway, so this is of questionable use.
> https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_get_key_block_len

If OpenSSL were to add that function, I'd consider using it. With
BoringSSL, I would not bother changing anything here (other than
commenting this out from the build) before someone first decides that
there is need for supporting EAP-FAST. I know there are such requests
for certain Android devices, but as long as AOSP does not have that
support, vendors will likely need to do whatever patching is needed to
make this work.. Since I know this is happening, I'd kind of like to get
this implemented with BoringSSL in the main repository, if that can be
done easily, but obviously I'll leave it to you to prioritize such a
feature request.

> Do you all have a preference? #2 would be the least invasive option,
> but splitting tls_connection_prf in two for #1 might be preferable, at
> which point you may as well condition on EAP-FAST. In doing so, we can
> probably do away with the fallback from SSL_export_keying_material to
> openssl_tls_prf if SSL_export_keying_material fails (looks like that's
> a remnant of how the #ifdefs worked out while 1.0.0 was supported) and
> not even ship a custom TLS PRF in the no-EAP-FAST config. I'm happy to
> do a bit of cleanup for you along the way here.

Yeah, that's from the pre-SSL_export_keying_material() history.. I think
I like your patches in the newer email, so assuming they pass my test
cases, I'll apply them to address this.

-- 
Jouni Malinen                                            PGP id EFC895FA