[
https://issues.apache.org/jira/browse/CASSANDRA-9945?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14649918#comment-14649918
]
Jason Brown commented on CASSANDRA-9945:
----------------------------------------
bq. Using a Cipher-per-thread (ThreadLocal) ...
Well, your jmh numbers are rather convincing ;) We can try that, but there's a
couple of wrinkles: for encrypting (writing), we do *not* want to reuse an IVs
across files, in which case we still need to call {{Cipher#init}}, which is
where I suspect the cost is with new Cipher instances. On the decryprt
(reading) side, we would have to make sure the alg/key/iv combo from the
parameters matches with that in the thread local, so it might need some kind of
a thread local map (not sure). I can look into this further...
bq. we could provide bouncycastle JCE
True, although the performance was terrible compared to the JDK's encryption
implementation. In my tests, it was sometimes 10x slower on the read path.
IIRC, there was still some magic in registering BC as the encryption provider
(security SPI-type thinng), which you can either do in code or through messing
with the JVM, but then you're in the same boat as the current patch. I'm not
against supporting BC, per se, but I'd prefer to pass until somebody actually
requests it.
bq. Can you elaborate a bit how the {{Key cache is to be used}} in
{{EncryptionContext}}?
Do you mean {{CipherFactory}} instead of {{EncryptionContext}}? If so, the goal
is to avoid always needing to reading from the keystore when a key is
requested, then keep it memoized. THis alos expects that you might have
multiple keys (because you migrated keys and have files encrypted with the
previous key(s) that still need to be read).
bq. Will there be any way to change the encryption keys e.g. via ”nodetool
refresh_tde_keystore“?
I hadn't envisioned a nodetool, command necessarily, because that would
necessitate the new key already being deployed to keystore on the machine - and
since you need a bounce of the process to recognize the updated keystore (and
the update key_alias in the yaml), I figured just wait until a bounce to
actaully switch to a new key. This might be less trouble for an operator than
invoking a nodetool command, since swapping keys is probably a rather unique
occurance.
bq. A unit test covering especially the {{getEncryptor}}+{{getDecryptor}}
methods in {{EncryptionContext}} would be great.
Once again, I think you mean {{CipherFactory}} here. Either way, this patch was
previously part of the (soon-to-be-ready-for-review) patch for CASSANDRA-6018.
In that one, you will find more than enough "round trip" encrypt/decrypt
operations ;). Either way, I can add a simple one to test that flow.
bq. I assume SecureRandom is not thread-safe
It is :), or at least, {{SecureRandom#nextBytes}} is synchronized. As creating
new encrypting Ciphers should be relatively uncontended in cassandra's use
case, I think this is fine.
bq. DatabaseDescriptor: typo
D'oh! well, that's a bit emabrassing. thanks
bq. the check if (tdeOptions.enabled)
In {{EncryptionContext}}, you are correct. Fixed.
bq. Unused method {{EncryptionContext#toHeaderParameters}}
Sorry, meant to commit that on CASSANDRA-6018. can remove for this commit, as
well as {{EncryptionContext.createFromMap}}
bq. the assertion (or the message) is wrong
improved the assert logic
Will post an updated branch soon
> Add transparent data encryption core classes
> --------------------------------------------
>
> Key: CASSANDRA-9945
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9945
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Jason Brown
> Assignee: Jason Brown
> Labels: encryption
> Fix For: 3.0 beta 1
>
>
> This patch will add the core infrastructure classes necessary for transparent
> data encryption (file-level encryption), as required for CASSANDRA-6018 and
> CASSANDRA-9633. The phrase "transparent data encryption", while not the most
> aesthetically pleasing, seems to be used throughout the database industry
> (Oracle, SQLQServer, Datastax Enterprise) to describe file level encryption,
> so we'll go with that, as well.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
