SecureSocketPort: Enable Server Name Indication (SNI) by default #1456
Conversation
When establishing TLS connections, the WPEFramework's SecureSocketPort implementation, based on OpenSSL, is not enabling the SNI (Server Name Indication [1]) extension by default. Most server implementations do require SNI, i.e., that the client, when doing the TLS handshake, does provide information about which server is handshaking to, so that it can then provide a suitable certificate. This is particularly true for cloud based services. To comply with these, we should enable SNI by default. Enabling this, does not create any problems to servers that do not support or require SNI, so no harm done in enabling SNI by default. Enabling the TLS SNI feature with OpenSSL can be done by calling the `SSL_set_tlsext_host_name()` ([2]) on the associated SSL context, with the host name we're trying to connect to, so that the host name is then set on the "server_name" extension in the "Client Hello" TLS message. This way, in `SecureSocketPort::Handler::Initialize()`, which is where the SSL context is created and prepared, make the required call to `SSL_set_tlsext_host_name()` with the host name that is configured on the remote node associated with the `SecureSocketPort` instance. For supporting this, extend the `Core::NodeId` class so that it "remembers" the original host name provided when creating nodes for representing IP connections to a remote host, which is the case of the TLS connections. For this, add a new `m_remoteHostName` member and an associated `RemoteHostName()` getter. There was already a `m_hostName` member and a `HostName()` getter which seem to suggest this feature was already present in `Core::NodeId`. However, further inspection and debugging shows that in fact this is not always the case and it is used for different purposes, hence the decision to use a new member/getter, to ensure the new needs and not breaking any current uses of `m_hostName`. [1] https://en.wikipedia.org/wiki/Server_Name_Indication [2] https://www.openssl.org/docs/man1.1.1/man3/SSL_get_servername_type.html Signed-off-by: Ricardo Silva <[email protected]>
|
Hi, any progress on reviewing this PR? |
|
@rjpdasilva as this is R2, I think you need to trigger @anand-ky or @karuna2git to get this merged, and potentially all other PR's pending for R2. |
|
Any chance to follow up on this please? |
|
Changes looks good & merging it. Could you please bring the change to master also.? So that future releases will automatically pick this up. |
|
Hi @karuna2git, Thanks for merging. Regarding:
Last time I've checked, the |
|
Got it.. Thanks 👍 |
When establishing TLS connections, the WPEFramework's SecureSocketPort implementation, based on OpenSSL, is not enabling the SNI (Server Name Indication [1]) extension by default. Most server implementations do require SNI, i.e., that the client, when doing the TLS handshake, does provide information about which server is handshaking to, so that it can then provide a suitable certificate. This is particularly true for cloud based services. To comply with these, we should enable SNI by default. Enabling this, does not create any problems to servers that do not support or require SNI, so no harm done in enabling SNI by default.
Enabling the TLS SNI feature with OpenSSL can be done by calling the
SSL_set_tlsext_host_name()([2]) on the associated SSL context, with the host name we're trying to connect to, so that the host name is then set on the "server_name" extension in the "Client Hello" TLS message.This way, in
SecureSocketPort::Handler::Initialize(), which is where the SSL context is created and prepared, make the required call toSSL_set_tlsext_host_name()with the host name that is configured on the remote node associated with theSecureSocketPortinstance.For supporting this, extend the
Core::NodeIdclass so that it "remembers" the original host name provided when creating nodes for representing IP connections to a remote host, which is the case of the TLS connections. For this, add a newm_remoteHostNamemember and an associatedRemoteHostName()getter. There was already am_hostNamemember and aHostName()getter which seem to suggest this feature was already present inCore::NodeId. However, further inspection and debugging shows that in fact this is not always the case and it is used for different purposes, hence the decision to use a new member/getter, to ensure the new needs and not breaking any current uses ofm_hostName.[1] https://en.wikipedia.org/wiki/Server_Name_Indication
[2] https://www.openssl.org/docs/man1.1.1/man3/SSL_get_servername_type.html
RDKDEV-886