cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Brown (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-14222) Add hot reloading of SSL Certificates capability to Cassandra
Date Thu, 08 Feb 2018 15:10:03 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-14222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16357044#comment-16357044
] 

Jason Brown commented on CASSANDRA-14222:
-----------------------------------------

Review comments:

- In {{SSLFactory#initHotReloading()}}, the else block is unnecessary and potentially confusing,
if the method happens to be called twice, as the logging says {{"Disabling hot reloading SSLContext"}}.
We won't actually disable the logging if {{isHotReloadingInitialized}} is true.

- In {{SSLfactory#initHotReloadableFiles()}}, add an additional check for 'enabled' to both
server and client lists. That is, if {{server_encryption_options.enabled}} is false, don't
bother adding the {{HotReloadableFile}} for it. (same for client)

- We typically don't pass around the {{Config}} instance, so for the {{SSLfactory}} methods
please pass the relevant {{EncryptionOptions}} instances. In {{MessagingService#reloadSslCerts()}},
call the methods on {{DatabaseDescriptor}} for getting the {{EncryptionOptions}} instances
rather than passing the raw config.

- Instead of having two lists of {{List<HotReloadableFile>}} in {{SSLFactory}}, can
we merge it down to one? We can add a flag to {{HotReloadableFile}} to indicate if it's server
or client. Then you could have code like this in {{SSLFactory#checkCertFilesForHotReloading()}}
(suit to taste):

{noformat}
if (hotReloadableFilesForServerCtx.stream().anyMatch(f -> s.isServer() && f.shouldReload()))
{
    logger.info("Server ssl certificates have been updated. Reseting the context for new peer
connections.");
    serverSslContext.set(null);
}

if (hotReloadableFilesForServerCtx.stream().anyMatch(f -> s.isClient() && f.shouldReload()))
{
    logger.info("Client ssl certificates have been updated. Reseting the context for new client
connections.");
    clientSslContext.set(null);
}
{noformat}

nits:
- At the top of {{SSLFactory#getSslContext()}}, you can avoid fetching the opposite {{SslContext}}
from the one you want (server vs. client) by doing this:
{noformat}
SslContext sslContext
if (forServer && (sslContext = serverSslContext.get()) != null)
   return sslContext;
if (!forServer && (sslContext = clientSslContext.get()) != null)
    return sslContext;
{noformat}

- Logging levels in {{SSLFactory#checkCertFilesForHotReloading()}}: 
-- "Checking whether certificates have been updated" should be {{TRACE}}, at best
-- others should be {{DEBUG}}


> Add hot reloading of SSL Certificates capability to Cassandra
> -------------------------------------------------------------
>
>                 Key: CASSANDRA-14222
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14222
>             Project: Cassandra
>          Issue Type: New Feature
>            Reporter: Dinesh Joshi
>            Assignee: Dinesh Joshi
>            Priority: Major
>             Fix For: 4.x
>
>
> Cassandra does not currently hot reload SSL certificates. For ease of operation it would
be useful if we add this capability. This patch would watch changes to the truststore &
keystore and would reload them when they're changed.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message