Node certificate endorsement and TLS authentication changes #581
Conversation
|
|
|
|
Codecov Report
@@ Coverage Diff @@
## master #581 +/- ##
=========================================
- Coverage 78.32% 78.1% -0.22%
=========================================
Files 140 141 +1
Lines 10697 10754 +57
=========================================
+ Hits 8378 8399 +21
- Misses 2319 2355 +36
|
src/enclave/rpcsessions.h
Outdated
| hasCa ? tls::auth_required : tls::auth_optional); | ||
|
|
||
| certs.push_back(std::move(the_cert)); | ||
| tls::auth_optional); |
There was a problem hiding this comment.
Can we add a comment on this, to convince me that either we don't always to need to verify or we always check the verification later?
There was a problem hiding this comment.
I would have thought we needed tls::auth_required here? It's true we check the cert later, but do we even want to allow unauthed connections at all?
There was a problem hiding this comment.
tls::auth_required maps to MBEDTLS_SSL_VERIFY_REQUIRED. As per https://tls.mbed.org/api/ssl_8h.html#a5695285c9dbfefec295012b566290f37, the server does not have to verify the client certificate (default). Otherwise, we would have to have the clients certs signed by a trusted CA and replace nullptr by that CA here.
|
|
|
|
Resolves #552
Before
ServerNameduring the TLS handshake to be routed to the right server/frontend.curlwhen CCF is built with HTTP), it is necessary to tell the client (e.g.--resolve) "the host you connect to at , well assume that its real name is <sni_name>". This is because when doing a TLS handshake, the client verifies that the server it's talking to is the server it believes it is (by checking that theSubjectAlternativeName(orCommonName, if SAN does not exist)).After
SNIcode on the server side has been removed (seerpcsessions.h,tls/server.h, etc.).--domainto a new node which will be used for TLS host authentication (see next point). If no domain is set, the IP address for RPC connections is used (*).CN=CCF NodeandSAN: iPAddress:<node_ip>(orSAN: dNSName=<domain>).CN=CCF Node <node_id>andSAN: iPAddress:<node_ip>(orSAN: dNSName=<domain>).(*) mbedtls does not parse
IPAddress=in theSubjectAltName. See more details #552 (comment)Implementation Details
frontendis now open or closed, as opposed to the corresponding SNI certificate being installed or not.client,memberclientand for the join protocol, we no longer callmbedtls_ssl_set_hostname()on the client side. This would break any clients when usingIPAddressinSANon the server-side as it is not parsed bymbedtls. This is fine as we still check the endorsement of the server certificate with the network certificate.mbedtlshave also been documented in the code, as comments.