This turned out to be trickier than I thought and I don't have a firm solution
yet.
Since SslFactory will handle reconfiguration, the idea is to make the
configuration immutable in the pluggable factory.
SslFactory would create a new pluggable factory every time the configuration
changes.
The pluggable factory creates its SSLContext when it is configured and never
change it.
It turns out SslFactory does not really need the SSLContext, so it can use the
new pluggable factory as an SSLEngine factory.
The ideal interface looks like:
public interface SslEngineFactory {
void configure(Map<String, ?> configs);
SSLEngine createSslEngine(String peerHost, int peerPort);
}
This assumes SslFactory copies the configs and overwrites ssl.client.auth when
there is a clientAuthConfigOverride.
I am told the right way to do this in Kafka is with extra parameters in
configure().
public interface SslEngineFactory {
void configure(Map<String, ?> configs, String
clientAuthConfigOverride);
SSLEngine createSslEngine(String peerHost, int peerPort);
}
With this interface, SslEngineFactory must load its keystore and truststore.
Meanwhile, SslFactory must also load the keystore and truststore to detect a
future change in configuration.
This slight inefficiency is not important, what truly matters is the lack of
atomicity.
The keystore and truststore might change between the time SslFactory inspects
them and the time SslEngineFactory loads them again.
There are multiple ways to address this issue:
1. We can ignore the issue. If we always make sure SslFactory inspects the
files before SslEngineFactory is configured,
SslEngineFactory might load a newer version of the files, and SslFactory
might unnecessarily recreate the SslEngineFactory
the next time a reconfiguration is called, but we will never miss an update.
2. We can load the keystore and truststore in SslFactory and pass them to
SslEngineFactory.configure().
The configs for the keystore and truststore are ignored by SslEngineFactory.
public interface SslEngineFactory {
void configure(Map<String, ?> configs, String
clientAuthConfigOverride, KeyStore keystore, KeyStore truststore);
SSLEngine createSslEngine(String peerHost, int peerPort);
}
Notice we pass the KeyStore and not the SecurityStore since
SslEngineFactory does not need the metadata.
I find this signature messy.
3. We could query the SslEngineFactory for the keystore and truststore it
loaded.
This is ugly because SslEngineFactory would need to keep metadata for these
files,
or we would have to compare the actual KeyStore entries.
4. We could create a new SslEngineFactory for every reconfiguration assuming
the configuration always changed.
This would create a new SSLContext every time and therefore lose the SSL
Session cache every time.
Maybe solution 1 is sufficient. Notice we need to load the actual keystore
early in SslFactory
since we need to fix the lastModified time and the CertificateEntries.
There is a bug currently in the comparison of the CertificateEntries because
the KeyStore is reloaded
at the time of the comparison, which might compare the new contents against the
same new contents.
Solution 4 brings the issue of what is a reconfiguration.
I was wondering why Kafka supports only reconfiguration of the keystore and
truststore?
Why not any of the other config settings?
To support reconfiguration of the keystore and truststore and nothing else,
SslFactory would have to pass the original configs to the new SslEngineFactory.
SslFactory would have to keep a copy of the original configs and if we are
using solution 1,
it must overwrite the settings for the keystore and truststore.
If we do that, we might as well overwrite the clientAuth also and use the ideal
interface instead.
Thoughts?
-----Original Message-----
From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
Sent: Tuesday, October 23, 2018 7:54 AM
To: dev
Subject: Re: [DISCUSS] KIP-383 Pluggable interface for SSL Factory
Thanks for the PR. A couple of comments:
1) For other public interfaces, we use parameters to configure instead of
internal config names going into the same list of configs. Kafka has public
config names and custom config names that don't conflict with public config
names. These internal config names fall into neither category.
An example of how we do this in other classes is Deserializer:
void configure(Map<String, ?> configs, boolean isKey);
2) I am not sure `SslFactory` as it stands today is the right class to turn
into a pluggable class. I can see why we want to have a custom class to
configure SSLEngines. But I don't think we want a custom class to decide
the logic for dynamic reconfiguration or decide whether to validate against
trust stores. It would perhaps be better to have a class to just create
SSLContext and possibly configure SSLEngine with additional options is
there is a good use case for that. But the non-pluggable class in Kafka
should deal with all the reconfiguration and validation logic.
Thoughts?
>
