added parallel certificate verification

[avadacatavra]

I'm working on using webpki and rustls in Servo. This PR will allow that.

When evaluating performance, I found that verify_server_cert was slower than using openssl, so I created two parallel functions to improve performance.

Performance evaluation: I connected to 13 https sites using verify_server_cert, parallel_verify_server_cert, and the openssl code currently used in servo.

Average time to establish a connection using:

• verify_server_cert: 110.6259529
• parallel_verify_server_cert: 87.99443197
• openssl: 77.41738468 ms

I plan to continue to look for improvements.

[manuels]

Am I missing something or is this an exact copy of verify_common_cert?

[avadacatavra]

there's a minute change--I'm using par_iter instead.

When I was looking at performance, parallelizing wasn't improving the code at all, so parallel_verify_common_cert uses rayon to convert from rustls to webpki structures. After that, performance improved. I didn't necessarily want to make permanent changes to the serial code (but I can).

[manuels]

Well, it's just my two cents -- I am also just a curious wanna-be contributer:

I think it's probably better not to duplicate such a critical part of the code, so we might be better off find solution that is flexible in whether processing it in parallel or not.
I have no idea how @ctz thinks about adding new dependencies.

[manuels]

Is maybe possible to make verify_common_cert() take a presented_certs of some Iter trait? (I am not familiar with the details of rayon and its compatibility to std)

[manuels]

Hey @avadacatavra, good performance catch!
Did you figure out what subfunction in rustls is so slow? Just wondering what the reason for this perfomance gap might be...

[avadacatavra]

@manuels I can make my performance results readable and post them in a few hours :)

[ctz]

I think the performance you're seeing is likely explained by briansmith/ring#431

But doing cert checks in parallel against all the roots is a very interesting idea. @briansmith how would you feel about that at the webpki level?

[briansmith]

How would the Rustls user control the level of parallelism (how many threads are used)?

[briansmith]

I suspect that you don't want to parallelize the entire certificate chain verification, for efficiency (battery use) reasons. Many certificate will have multiple chains that are valid, and each chain requires verifying a bunch of signatures, so if you do the whole path building in parallel, you'll end up doing a bunch of extra signature verifications, which are very expensive.

I will explain the multiple ways you already parallelize certificate verification without increasing CPU consumption:

• When we receive the server's Certificate message, we can do certificate verification in parallel with doing the rest of the TLS handshake. In particular, we can process ServerKeyExchange (do the Diffie-Hellman computation) and send ClientKeyExchange* and Finished, without knowing whether the certificate is valid. We just can't send application data or process the peer's application data until we get notified of the certificate validation result. This optimization is already implemented in Firefox/NSS (I implemented it).

• You can further split out the verification of the ServerKeyExchange signature into a separate thread. Again, you don't need to know the ServerKeyExchange signature is valid to do the Diffie-Hellman computation or to send ClientKeyExchange* and Finished, but you do need to have the result of the verification before you send application data or process application data from the peer.

• webpki can be changed to do the signature verification in parallel; see Verify signatures in parallel briansmith/webpki#37 where I explain in detail how it would be done.

[briansmith]

Regarding sending CertificateVerify before we've verified the server's identity: If we're not doing client auth then it is clearly OK. If we're doing client auth (signing with our own private key and sending our own certificate) then it is debatable whether we should send a proof of our identity without having verified the server's identity first. In Firefox/NSS, I decided to not send the CertificateVerify message until the server's identity has been validated (all signatures verified).

[briansmith]

Performance evaluation: I connected to 13 https sites using verify_server_cert, parallel_verify_server_cert, and the openssl code currently used in servo.

Could you please list the 13 sites? And also which version of OpenSSL?

[avadacatavra]

@briansmith Those results used OpenSSL 1.0.2i. I've also run my tests with OpenSSL OpenSSL 1.0.2k, which was slightly faster (71ms vs 77ms for total connection time)

The sites are:

• duckduckgo
• github
• wikipedia
• arstechnica
• reddit
• hackernews
• servo
• rust
• google.co.uk
• cnn
• washingtonpost
• twitter
• stackoverflow

Servo is currently using rust-openssl v0.7.14. Part of the Servo PR is a bump to v.0.9.6. There's an OpensslClient in hyper-openssl, but that doesn't allow us to customize the verification method.

[briansmith]

@avadacatavra Great, thanks for the list.

Looking over that list, I see there is a mix of algorithms (RSA and ECDSA) and key sizes (P-256 and P-384, RSA 2048 and larger). Given TLS is complex and the crypto primitives used are also complex, there could be multiple performance issues, so we need to find a way to isolate the performance issues. I filed briansmith/crypto-bench#24 and briansmith/crypto-bench#25 for investigating the performance of signature verification between ring and OpenSSL. I also filed briansmith/webpki#38 for isolating the certificate chain verification from anything in Rustls.

[avadacatavra]

@briansmith there's more numbers here: https://avadacatavra.github.io/servo/2016/02/07/webpkiresults.html

[nox]

What's the status on this?

[avadacatavra]

@nox status is that I had to finish the hyper bump and XOW before returning to this :)

[nox]

This needs a rebase.

[ctz]

What's the status on this?

I think there are lower hanging fruit for making the performance competitive here.

Not that I dislike the idea of parallelising this; and if rustls had a (perhaps optional) dependency on rayon or something I would also use it in the record layer too.

[ctz]

cnn

Hey @avadacatavra I'm adding benchmarks for cert verification for all these sites, and can't find a CNN site with a valid certificate (they do TLS, but present a certificate for 'a.ssl.fastly.net'). What hostname were you using?

edit: missed the bit in your blog post about cnn being non-TLS. Sorry for the noise!

[avadacatavra]

@ctz thanks for working on the benchmarks--I'm wrapping up some other projects that took priority over the past few weeks, then hoping to get back to this and related projects

[briansmith]

As discussed above, I think there are opportunities to do some operations in parallel but the approach in this PR is really inefficient. The performance issue with RSA signature verification in ring was fixed a while back, and I think this PR was basically a workaround for that issue. So I think this should be closed.