You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Nikita Amelchev <ns...@gmail.com> on 2019/10/17 14:12:39 UTC
Re: TDE Master key rotation (Phase-2)
Hi, Igniters!
I have implemented the master key change process [1] for TDE as
described in the design [2].
I have prepared PR [3] and created the Upsource review branch [4].
Could anyone take a look at my changes?
Can we include it into a 2.8 release scope?
[1] https://issues.apache.org/jira/browse/IGNITE-12186
[2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
[3] https://github.com/apache/ignite/pull/6937
[4] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1067
пн, 23 сент. 2019 г. в 17:13, Nikolay Izhikov <ni...@apache.org>:
>
> Hello, Nikita.
>
> > A node creates the ChangeMasterKeyMessage message and sent it by discovery as a custom event.
> > The goal is to verify that all nodes have the same master key.
> ...
> > The ChangeMasterKeyFinishMessage action message is sent by discovery as a custom event.
> > New master key id.
>
> 1. We should store locally new key id and new key hash after processing of ChangeMasterKeyMessage
> 2. We should send new key hash in ChangeMasterKeyFinishMessage
> 3. We should ensure that both ChangeMasterKeyMessage and ChangeMasterKeyFinishMessage contains the same key id and key hash before executing a change process.
>
> I think we should rename:
>
> > Node left during key rotation process(was not starting re-encrypt cache keys)
>
> Node was down during key rotation. ChangeMasterKeyRecord was not found.
>
> > Node left during key rotation process(was starting re-encrypt cache keys)
>
> Node was down during key rotation. ChangeMasterKeyRecord found.
>
> We should add a description of changes in the cluster join process.
> A node should not try to join to the cluster before the process of ChangeMasterKeyRecord.
>
> It's not clear for me how we differ two cases:
>
> 1. ChangeMasterKeyRecord found in WAL and key rotation was finished successfully.
> 2. ChangeMasterKeyRecord found in WAL and key rotation was NOT finished successfully.
>
> > Meta storage will store master key id.
>
> We should add that key id from metastorage has a higher priority to key id from IgniteConfiguration.
>
>
> В Пт, 20/09/2019 в 14:06 +0300, Nikita Amelchev пишет:
> > Nikolay,
> >
> > you are right in many ways. I updated the design on the wiki. [1]
> >
> > [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> >
> > пт, 20 сент. 2019 г. в 13:49, Nikolay Izhikov <ni...@apache.org>:
> > >
> > > Nikita
> > >
> > > > I suggested the implementation where the encryption manager is
> > > > responsible for storing the master key id.
> > >
> > > I don't think it's a right proposal.
> > >
> > > 1. EncryptionSpi implementation becomes more complicated. Developer of it should be aware of Ignite deployment scenarious, etc.
> > > Imagine implementation when EncryptionSpi send master key id to some external storage over network(it's happen in Discovery thread)
> > >
> > > 2. Implementation would be duplicate features(saving master key id)
> > >
> > > 3. We already store cache keys in metastore. For me it's a native approach to store master key id in the same place.
> > >
> > > What do you think?
> > >
> > >
> > > В Пт, 20/09/2019 в 13:39 +0300, Nikita Amelchev пишет:
> > > > Nikolay,
> > > >
> > > > because I suggested the implementation where the encryption manager is
> > > > responsible for storing the master key id.
> > > > To implement this logic in the EncryptionSpi, we will need to
> > > > introduce the methods look like this:
> > > >
> > > > setMasterKeyId(String masterKeyId) // Sets "current" master key id
> > > > String getMasterKeyId() // Gets "current" master key id
> > > >
> > > > Follow methods will work with master key that setted by previous method:
> > > >
> > > > byte[] masterKeyDigest()
> > > > byte[] encryptKey(Serializable key)
> > > > Serializable decryptKey(byte[] key)
> > > >
> > > > If such implementation is more reasonable, I will do so.
> > > >
> > > > пт, 20 сент. 2019 г. в 13:04, Nikolay Izhikov <ni...@apache.org>:
> > > > >
> > > > > Why do we need "defaultMasterKeyId" instead of *current* master key id that can be obtained with `KeystoreEncryptionSpi#getMasterKeyName()`?
> > > > >
> > > > > В Пт, 20/09/2019 в 12:56 +0300, Nikita Amelchev пишет:
> > > > > > Nikolay,
> > > > > >
> > > > > > Thanks for the proposal, I like it.
> > > > > >
> > > > > > The GridEncryptionManager will control the process of master key
> > > > > > rotation, so we should provide him master key id at startup. Seems we
> > > > > > should get it from some configuration for encryption.
> > > > > >
> > > > > > I suggest just adding the String defaultMasterKeyId() method into the
> > > > > > EncryptionSpi interface. This method gets default master key id used
> > > > > > on first cluster start.
> > > > > >
> > > > > > The specific implementation will be responsible for setting this value.
> > > > > >
> > > > > > What do you think?
> > > > > >
> > > > > > пт, 20 сент. 2019 г. в 10:44, Nikolay Izhikov <ni...@apache.org>:
> > > > > > >
> > > > > > > Hello, Nikita
> > > > > > >
> > > > > > > > IgniteConfiguration: New methods will be added to the IgniteConfiguration:
> > > > > > > > public IgniteConfiguration setEncryptionMasterKeyId(String masterKeyId) - sets master key id.
> > > > > > > > public String getEncryptionMasterKeyId()
> > > > > > >
> > > > > > > We don't need it in the IgniteConfiguration.
> > > > > > >
> > > > > > > As you may know, we already have KeystoreEncryptionSpi#setMasterKeyName.
> > > > > > > Seems, we should add it to the EncryptionSpi itself.
> > > > > > >
> > > > > > >
> > > > > > > В Ср, 18/09/2019 в 22:25 +0300, Nikita Amelchev пишет:
> > > > > > > > Nikolay, thanks for participating.
> > > > > > > >
> > > > > > > > I have supplemented the design and clarify these moments. [1]
> > > > > > > >
> > > > > > > > [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > >
> > > > > > > > ср, 18 сент. 2019 г. в 16:48, Nikolay Izhikov <ni...@apache.org>:
> > > > > > > > >
> > > > > > > > > Hello, Nikita.
> > > > > > > > >
> > > > > > > > > Thanks for starting this discussion.
> > > > > > > > >
> > > > > > > > > 1. We should add prerequisites for "master key rotation process" in design.
> > > > > > > > > Seems, it should be, "New master key available to EncryptionSPI for each server node".
> > > > > > > > >
> > > > > > > > > 2. Please, use code formatting in wiki. It's make reading easier.
> > > > > > > > >
> > > > > > > > > 3. Please, clarify java API proposal. What will be changed and how.
> > > > > > > > > AFAIK we need to change EncryptionSPI, this should be covered in design.
> > > > > > > > >
> > > > > > > > > 4. Please, clarify new CLI commands.
> > > > > > > > > AFAIK we should have 2 command:
> > > > > > > > >
> > > > > > > > > 1. Start regular master key rotation process.
> > > > > > > > > 2. Start local master key rotation process during node recovery(for the case when key changed while node was down).
> > > > > > > > >
> > > > > > > > > В Ср, 18/09/2019 в 16:09 +0300, Nikita Amelchev пишет:
> > > > > > > > > > Hi, Igniters.
> > > > > > > > > >
> > > > > > > > > > I'm going to implement the ability to rotate the master encryption key
> > > > > > > > > > (TDE Phase 2). [1]
> > > > > > > > > > Master key rotation required in case of it compromising or at the end
> > > > > > > > > > of crypto period(key validity period). I prepared the design. [2]
> > > > > > > > > >
> > > > > > > > > > In briefly, master keys will be identified by String masterKeyId. The
> > > > > > > > > > concept of the masterKeyId will be added to the cache keys encryption
> > > > > > > > > > process in EncryptionSpi.
> > > > > > > > > >
> > > > > > > > > > Users can configure master key id in IgniteConfiguration and will be
> > > > > > > > > > able to manage the key rotation process from java API, JMX, CLI:
> > > > > > > > > > - ignite.encryption().changeMasterKey(String masterKeyId) - starts
> > > > > > > > > > master key rotation process.
> > > > > > > > > > - String ignite.encryption().getMasterKeyId() - gets current master key id.
> > > > > > > > > >
> > > > > > > > > > Any thoughts?
> > > > > > > > > >
> > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12186
> > > > > > > > > > [2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
> >
> >
> >
--
Best wishes,
Amelchev Nikita
Re: TDE Master key rotation (Phase-2)
Posted by Nikita Amelchev <ns...@gmail.com>.
Hello Igniters!
Nikolay almost finished PR review. Does anyone else want to look at
the changes? [1]
I implemented master key change management through Java API and JMX. I
created the issue [2] to implement change through control.sh that I
will do after the merge first one.
[1] https://github.com/apache/ignite/pull/6937
[2] https://issues.apache.org/jira/browse/IGNITE-12475
пт, 18 окт. 2019 г. в 15:18, Nikolay Izhikov <ni...@apache.org>:
>
> Hello, Nikita.
>
> Thank you.
>
> I will take a look. shortly.
>
> чт, 17 окт. 2019 г. в 18:23, Maxim Muzafarov <mm...@apache.org>:
>
> > Nikita,
> >
> > > Can we include it into a 2.8 release scope?
> > I think it is possible since the release scope freeze date has not
> > happened yet.
> >
> > On Thu, 17 Oct 2019 at 17:36, Nikita Amelchev <ns...@gmail.com>
> > wrote:
> > >
> > > Hi, Igniters!
> > >
> > > I have implemented the master key change process [1] for TDE as
> > > described in the design [2].
> > >
> > > I have prepared PR [3] and created the Upsource review branch [4].
> > >
> > > Could anyone take a look at my changes?
> > >
> > > Can we include it into a 2.8 release scope?
> > >
> > > [1] https://issues.apache.org/jira/browse/IGNITE-12186
> > > [2]
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > [3] https://github.com/apache/ignite/pull/6937
> > > [4] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1067
> > >
> > > пн, 23 сент. 2019 г. в 17:13, Nikolay Izhikov <ni...@apache.org>:
> > > >
> > > > Hello, Nikita.
> > > >
> > > > > A node creates the ChangeMasterKeyMessage message and sent it by
> > discovery as a custom event.
> > > > > The goal is to verify that all nodes have the same master key.
> > > > ...
> > > > > The ChangeMasterKeyFinishMessage action message is sent by discovery
> > as a custom event.
> > > > > New master key id.
> > > >
> > > > 1. We should store locally new key id and new key hash after
> > processing of ChangeMasterKeyMessage
> > > > 2. We should send new key hash in ChangeMasterKeyFinishMessage
> > > > 3. We should ensure that both ChangeMasterKeyMessage and
> > ChangeMasterKeyFinishMessage contains the same key id and key hash before
> > executing a change process.
> > > >
> > > > I think we should rename:
> > > >
> > > > > Node left during key rotation process(was not starting re-encrypt
> > cache keys)
> > > >
> > > > Node was down during key rotation. ChangeMasterKeyRecord was not found.
> > > >
> > > > > Node left during key rotation process(was starting re-encrypt cache
> > keys)
> > > >
> > > > Node was down during key rotation. ChangeMasterKeyRecord found.
> > > >
> > > > We should add a description of changes in the cluster join process.
> > > > A node should not try to join to the cluster before the process of
> > ChangeMasterKeyRecord.
> > > >
> > > > It's not clear for me how we differ two cases:
> > > >
> > > > 1. ChangeMasterKeyRecord found in WAL and key rotation was finished
> > successfully.
> > > > 2. ChangeMasterKeyRecord found in WAL and key rotation was NOT
> > finished successfully.
> > > >
> > > > > Meta storage will store master key id.
> > > >
> > > > We should add that key id from metastorage has a higher priority to
> > key id from IgniteConfiguration.
> > > >
> > > >
> > > > В Пт, 20/09/2019 в 14:06 +0300, Nikita Amelchev пишет:
> > > > > Nikolay,
> > > > >
> > > > > you are right in many ways. I updated the design on the wiki. [1]
> > > > >
> > > > > [1]
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > >
> > > > > пт, 20 сент. 2019 г. в 13:49, Nikolay Izhikov <ni...@apache.org>:
> > > > > >
> > > > > > Nikita
> > > > > >
> > > > > > > I suggested the implementation where the encryption manager is
> > > > > > > responsible for storing the master key id.
> > > > > >
> > > > > > I don't think it's a right proposal.
> > > > > >
> > > > > > 1. EncryptionSpi implementation becomes more complicated.
> > Developer of it should be aware of Ignite deployment scenarious, etc.
> > > > > > Imagine implementation when EncryptionSpi send master key id to
> > some external storage over network(it's happen in Discovery thread)
> > > > > >
> > > > > > 2. Implementation would be duplicate features(saving master key id)
> > > > > >
> > > > > > 3. We already store cache keys in metastore. For me it's a native
> > approach to store master key id in the same place.
> > > > > >
> > > > > > What do you think?
> > > > > >
> > > > > >
> > > > > > В Пт, 20/09/2019 в 13:39 +0300, Nikita Amelchev пишет:
> > > > > > > Nikolay,
> > > > > > >
> > > > > > > because I suggested the implementation where the encryption
> > manager is
> > > > > > > responsible for storing the master key id.
> > > > > > > To implement this logic in the EncryptionSpi, we will need to
> > > > > > > introduce the methods look like this:
> > > > > > >
> > > > > > > setMasterKeyId(String masterKeyId) // Sets "current" master key
> > id
> > > > > > > String getMasterKeyId() // Gets "current" master key id
> > > > > > >
> > > > > > > Follow methods will work with master key that setted by previous
> > method:
> > > > > > >
> > > > > > > byte[] masterKeyDigest()
> > > > > > > byte[] encryptKey(Serializable key)
> > > > > > > Serializable decryptKey(byte[] key)
> > > > > > >
> > > > > > > If such implementation is more reasonable, I will do so.
> > > > > > >
> > > > > > > пт, 20 сент. 2019 г. в 13:04, Nikolay Izhikov <
> > nizhikov@apache.org>:
> > > > > > > >
> > > > > > > > Why do we need "defaultMasterKeyId" instead of *current*
> > master key id that can be obtained with
> > `KeystoreEncryptionSpi#getMasterKeyName()`?
> > > > > > > >
> > > > > > > > В Пт, 20/09/2019 в 12:56 +0300, Nikita Amelchev пишет:
> > > > > > > > > Nikolay,
> > > > > > > > >
> > > > > > > > > Thanks for the proposal, I like it.
> > > > > > > > >
> > > > > > > > > The GridEncryptionManager will control the process of master
> > key
> > > > > > > > > rotation, so we should provide him master key id at startup.
> > Seems we
> > > > > > > > > should get it from some configuration for encryption.
> > > > > > > > >
> > > > > > > > > I suggest just adding the String defaultMasterKeyId() method
> > into the
> > > > > > > > > EncryptionSpi interface. This method gets default master key
> > id used
> > > > > > > > > on first cluster start.
> > > > > > > > >
> > > > > > > > > The specific implementation will be responsible for setting
> > this value.
> > > > > > > > >
> > > > > > > > > What do you think?
> > > > > > > > >
> > > > > > > > > пт, 20 сент. 2019 г. в 10:44, Nikolay Izhikov <
> > nizhikov@apache.org>:
> > > > > > > > > >
> > > > > > > > > > Hello, Nikita
> > > > > > > > > >
> > > > > > > > > > > IgniteConfiguration: New methods will be added to the
> > IgniteConfiguration:
> > > > > > > > > > > public IgniteConfiguration
> > setEncryptionMasterKeyId(String masterKeyId) - sets master key id.
> > > > > > > > > > > public String getEncryptionMasterKeyId()
> > > > > > > > > >
> > > > > > > > > > We don't need it in the IgniteConfiguration.
> > > > > > > > > >
> > > > > > > > > > As you may know, we already have
> > KeystoreEncryptionSpi#setMasterKeyName.
> > > > > > > > > > Seems, we should add it to the EncryptionSpi itself.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > В Ср, 18/09/2019 в 22:25 +0300, Nikita Amelchev пишет:
> > > > > > > > > > > Nikolay, thanks for participating.
> > > > > > > > > > >
> > > > > > > > > > > I have supplemented the design and clarify these
> > moments. [1]
> > > > > > > > > > >
> > > > > > > > > > > [1]
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > > > >
> > > > > > > > > > > ср, 18 сент. 2019 г. в 16:48, Nikolay Izhikov <
> > nizhikov@apache.org>:
> > > > > > > > > > > >
> > > > > > > > > > > > Hello, Nikita.
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > > > >
> > > > > > > > > > > > 1. We should add prerequisites for "master key
> > rotation process" in design.
> > > > > > > > > > > > Seems, it should be, "New master key available to
> > EncryptionSPI for each server node".
> > > > > > > > > > > >
> > > > > > > > > > > > 2. Please, use code formatting in wiki. It's make
> > reading easier.
> > > > > > > > > > > >
> > > > > > > > > > > > 3. Please, clarify java API proposal. What will be
> > changed and how.
> > > > > > > > > > > > AFAIK we need to change EncryptionSPI, this should be
> > covered in design.
> > > > > > > > > > > >
> > > > > > > > > > > > 4. Please, clarify new CLI commands.
> > > > > > > > > > > > AFAIK we should have 2 command:
> > > > > > > > > > > >
> > > > > > > > > > > > 1. Start regular master key rotation process.
> > > > > > > > > > > > 2. Start local master key rotation process
> > during node recovery(for the case when key changed while node was down).
> > > > > > > > > > > >
> > > > > > > > > > > > В Ср, 18/09/2019 в 16:09 +0300, Nikita Amelchev пишет:
> > > > > > > > > > > > > Hi, Igniters.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I'm going to implement the ability to rotate the
> > master encryption key
> > > > > > > > > > > > > (TDE Phase 2). [1]
> > > > > > > > > > > > > Master key rotation required in case of it
> > compromising or at the end
> > > > > > > > > > > > > of crypto period(key validity period). I prepared
> > the design. [2]
> > > > > > > > > > > > >
> > > > > > > > > > > > > In briefly, master keys will be identified by String
> > masterKeyId. The
> > > > > > > > > > > > > concept of the masterKeyId will be added to the
> > cache keys encryption
> > > > > > > > > > > > > process in EncryptionSpi.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Users can configure master key id in
> > IgniteConfiguration and will be
> > > > > > > > > > > > > able to manage the key rotation process from java
> > API, JMX, CLI:
> > > > > > > > > > > > > - ignite.encryption().changeMasterKey(String
> > masterKeyId) - starts
> > > > > > > > > > > > > master key rotation process.
> > > > > > > > > > > > > - String ignite.encryption().getMasterKeyId() -
> > gets current master key id.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Any thoughts?
> > > > > > > > > > > > >
> > > > > > > > > > > > > [1]
> > https://issues.apache.org/jira/browse/IGNITE-12186
> > > > > > > > > > > > > [2]
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
> > > --
> > > Best wishes,
> > > Amelchev Nikita
> >
--
Best wishes,
Amelchev Nikita
Re: TDE Master key rotation (Phase-2)
Posted by Nikolay Izhikov <ni...@apache.org>.
Hello, Nikita.
Thank you.
I will take a look. shortly.
чт, 17 окт. 2019 г. в 18:23, Maxim Muzafarov <mm...@apache.org>:
> Nikita,
>
> > Can we include it into a 2.8 release scope?
> I think it is possible since the release scope freeze date has not
> happened yet.
>
> On Thu, 17 Oct 2019 at 17:36, Nikita Amelchev <ns...@gmail.com>
> wrote:
> >
> > Hi, Igniters!
> >
> > I have implemented the master key change process [1] for TDE as
> > described in the design [2].
> >
> > I have prepared PR [3] and created the Upsource review branch [4].
> >
> > Could anyone take a look at my changes?
> >
> > Can we include it into a 2.8 release scope?
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-12186
> > [2]
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > [3] https://github.com/apache/ignite/pull/6937
> > [4] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1067
> >
> > пн, 23 сент. 2019 г. в 17:13, Nikolay Izhikov <ni...@apache.org>:
> > >
> > > Hello, Nikita.
> > >
> > > > A node creates the ChangeMasterKeyMessage message and sent it by
> discovery as a custom event.
> > > > The goal is to verify that all nodes have the same master key.
> > > ...
> > > > The ChangeMasterKeyFinishMessage action message is sent by discovery
> as a custom event.
> > > > New master key id.
> > >
> > > 1. We should store locally new key id and new key hash after
> processing of ChangeMasterKeyMessage
> > > 2. We should send new key hash in ChangeMasterKeyFinishMessage
> > > 3. We should ensure that both ChangeMasterKeyMessage and
> ChangeMasterKeyFinishMessage contains the same key id and key hash before
> executing a change process.
> > >
> > > I think we should rename:
> > >
> > > > Node left during key rotation process(was not starting re-encrypt
> cache keys)
> > >
> > > Node was down during key rotation. ChangeMasterKeyRecord was not found.
> > >
> > > > Node left during key rotation process(was starting re-encrypt cache
> keys)
> > >
> > > Node was down during key rotation. ChangeMasterKeyRecord found.
> > >
> > > We should add a description of changes in the cluster join process.
> > > A node should not try to join to the cluster before the process of
> ChangeMasterKeyRecord.
> > >
> > > It's not clear for me how we differ two cases:
> > >
> > > 1. ChangeMasterKeyRecord found in WAL and key rotation was finished
> successfully.
> > > 2. ChangeMasterKeyRecord found in WAL and key rotation was NOT
> finished successfully.
> > >
> > > > Meta storage will store master key id.
> > >
> > > We should add that key id from metastorage has a higher priority to
> key id from IgniteConfiguration.
> > >
> > >
> > > В Пт, 20/09/2019 в 14:06 +0300, Nikita Amelchev пишет:
> > > > Nikolay,
> > > >
> > > > you are right in many ways. I updated the design on the wiki. [1]
> > > >
> > > > [1]
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > >
> > > > пт, 20 сент. 2019 г. в 13:49, Nikolay Izhikov <ni...@apache.org>:
> > > > >
> > > > > Nikita
> > > > >
> > > > > > I suggested the implementation where the encryption manager is
> > > > > > responsible for storing the master key id.
> > > > >
> > > > > I don't think it's a right proposal.
> > > > >
> > > > > 1. EncryptionSpi implementation becomes more complicated.
> Developer of it should be aware of Ignite deployment scenarious, etc.
> > > > > Imagine implementation when EncryptionSpi send master key id to
> some external storage over network(it's happen in Discovery thread)
> > > > >
> > > > > 2. Implementation would be duplicate features(saving master key id)
> > > > >
> > > > > 3. We already store cache keys in metastore. For me it's a native
> approach to store master key id in the same place.
> > > > >
> > > > > What do you think?
> > > > >
> > > > >
> > > > > В Пт, 20/09/2019 в 13:39 +0300, Nikita Amelchev пишет:
> > > > > > Nikolay,
> > > > > >
> > > > > > because I suggested the implementation where the encryption
> manager is
> > > > > > responsible for storing the master key id.
> > > > > > To implement this logic in the EncryptionSpi, we will need to
> > > > > > introduce the methods look like this:
> > > > > >
> > > > > > setMasterKeyId(String masterKeyId) // Sets "current" master key
> id
> > > > > > String getMasterKeyId() // Gets "current" master key id
> > > > > >
> > > > > > Follow methods will work with master key that setted by previous
> method:
> > > > > >
> > > > > > byte[] masterKeyDigest()
> > > > > > byte[] encryptKey(Serializable key)
> > > > > > Serializable decryptKey(byte[] key)
> > > > > >
> > > > > > If such implementation is more reasonable, I will do so.
> > > > > >
> > > > > > пт, 20 сент. 2019 г. в 13:04, Nikolay Izhikov <
> nizhikov@apache.org>:
> > > > > > >
> > > > > > > Why do we need "defaultMasterKeyId" instead of *current*
> master key id that can be obtained with
> `KeystoreEncryptionSpi#getMasterKeyName()`?
> > > > > > >
> > > > > > > В Пт, 20/09/2019 в 12:56 +0300, Nikita Amelchev пишет:
> > > > > > > > Nikolay,
> > > > > > > >
> > > > > > > > Thanks for the proposal, I like it.
> > > > > > > >
> > > > > > > > The GridEncryptionManager will control the process of master
> key
> > > > > > > > rotation, so we should provide him master key id at startup.
> Seems we
> > > > > > > > should get it from some configuration for encryption.
> > > > > > > >
> > > > > > > > I suggest just adding the String defaultMasterKeyId() method
> into the
> > > > > > > > EncryptionSpi interface. This method gets default master key
> id used
> > > > > > > > on first cluster start.
> > > > > > > >
> > > > > > > > The specific implementation will be responsible for setting
> this value.
> > > > > > > >
> > > > > > > > What do you think?
> > > > > > > >
> > > > > > > > пт, 20 сент. 2019 г. в 10:44, Nikolay Izhikov <
> nizhikov@apache.org>:
> > > > > > > > >
> > > > > > > > > Hello, Nikita
> > > > > > > > >
> > > > > > > > > > IgniteConfiguration: New methods will be added to the
> IgniteConfiguration:
> > > > > > > > > > public IgniteConfiguration
> setEncryptionMasterKeyId(String masterKeyId) - sets master key id.
> > > > > > > > > > public String getEncryptionMasterKeyId()
> > > > > > > > >
> > > > > > > > > We don't need it in the IgniteConfiguration.
> > > > > > > > >
> > > > > > > > > As you may know, we already have
> KeystoreEncryptionSpi#setMasterKeyName.
> > > > > > > > > Seems, we should add it to the EncryptionSpi itself.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > В Ср, 18/09/2019 в 22:25 +0300, Nikita Amelchev пишет:
> > > > > > > > > > Nikolay, thanks for participating.
> > > > > > > > > >
> > > > > > > > > > I have supplemented the design and clarify these
> moments. [1]
> > > > > > > > > >
> > > > > > > > > > [1]
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > > >
> > > > > > > > > > ср, 18 сент. 2019 г. в 16:48, Nikolay Izhikov <
> nizhikov@apache.org>:
> > > > > > > > > > >
> > > > > > > > > > > Hello, Nikita.
> > > > > > > > > > >
> > > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > > >
> > > > > > > > > > > 1. We should add prerequisites for "master key
> rotation process" in design.
> > > > > > > > > > > Seems, it should be, "New master key available to
> EncryptionSPI for each server node".
> > > > > > > > > > >
> > > > > > > > > > > 2. Please, use code formatting in wiki. It's make
> reading easier.
> > > > > > > > > > >
> > > > > > > > > > > 3. Please, clarify java API proposal. What will be
> changed and how.
> > > > > > > > > > > AFAIK we need to change EncryptionSPI, this should be
> covered in design.
> > > > > > > > > > >
> > > > > > > > > > > 4. Please, clarify new CLI commands.
> > > > > > > > > > > AFAIK we should have 2 command:
> > > > > > > > > > >
> > > > > > > > > > > 1. Start regular master key rotation process.
> > > > > > > > > > > 2. Start local master key rotation process
> during node recovery(for the case when key changed while node was down).
> > > > > > > > > > >
> > > > > > > > > > > В Ср, 18/09/2019 в 16:09 +0300, Nikita Amelchev пишет:
> > > > > > > > > > > > Hi, Igniters.
> > > > > > > > > > > >
> > > > > > > > > > > > I'm going to implement the ability to rotate the
> master encryption key
> > > > > > > > > > > > (TDE Phase 2). [1]
> > > > > > > > > > > > Master key rotation required in case of it
> compromising or at the end
> > > > > > > > > > > > of crypto period(key validity period). I prepared
> the design. [2]
> > > > > > > > > > > >
> > > > > > > > > > > > In briefly, master keys will be identified by String
> masterKeyId. The
> > > > > > > > > > > > concept of the masterKeyId will be added to the
> cache keys encryption
> > > > > > > > > > > > process in EncryptionSpi.
> > > > > > > > > > > >
> > > > > > > > > > > > Users can configure master key id in
> IgniteConfiguration and will be
> > > > > > > > > > > > able to manage the key rotation process from java
> API, JMX, CLI:
> > > > > > > > > > > > - ignite.encryption().changeMasterKey(String
> masterKeyId) - starts
> > > > > > > > > > > > master key rotation process.
> > > > > > > > > > > > - String ignite.encryption().getMasterKeyId() -
> gets current master key id.
> > > > > > > > > > > >
> > > > > > > > > > > > Any thoughts?
> > > > > > > > > > > >
> > > > > > > > > > > > [1]
> https://issues.apache.org/jira/browse/IGNITE-12186
> > > > > > > > > > > > [2]
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
> >
> >
> >
> > --
> > Best wishes,
> > Amelchev Nikita
>
Re: TDE Master key rotation (Phase-2)
Posted by Maxim Muzafarov <mm...@apache.org>.
Nikita,
> Can we include it into a 2.8 release scope?
I think it is possible since the release scope freeze date has not happened yet.
On Thu, 17 Oct 2019 at 17:36, Nikita Amelchev <ns...@gmail.com> wrote:
>
> Hi, Igniters!
>
> I have implemented the master key change process [1] for TDE as
> described in the design [2].
>
> I have prepared PR [3] and created the Upsource review branch [4].
>
> Could anyone take a look at my changes?
>
> Can we include it into a 2.8 release scope?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12186
> [2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> [3] https://github.com/apache/ignite/pull/6937
> [4] https://reviews.ignite.apache.org/ignite/review/IGNT-CR-1067
>
> пн, 23 сент. 2019 г. в 17:13, Nikolay Izhikov <ni...@apache.org>:
> >
> > Hello, Nikita.
> >
> > > A node creates the ChangeMasterKeyMessage message and sent it by discovery as a custom event.
> > > The goal is to verify that all nodes have the same master key.
> > ...
> > > The ChangeMasterKeyFinishMessage action message is sent by discovery as a custom event.
> > > New master key id.
> >
> > 1. We should store locally new key id and new key hash after processing of ChangeMasterKeyMessage
> > 2. We should send new key hash in ChangeMasterKeyFinishMessage
> > 3. We should ensure that both ChangeMasterKeyMessage and ChangeMasterKeyFinishMessage contains the same key id and key hash before executing a change process.
> >
> > I think we should rename:
> >
> > > Node left during key rotation process(was not starting re-encrypt cache keys)
> >
> > Node was down during key rotation. ChangeMasterKeyRecord was not found.
> >
> > > Node left during key rotation process(was starting re-encrypt cache keys)
> >
> > Node was down during key rotation. ChangeMasterKeyRecord found.
> >
> > We should add a description of changes in the cluster join process.
> > A node should not try to join to the cluster before the process of ChangeMasterKeyRecord.
> >
> > It's not clear for me how we differ two cases:
> >
> > 1. ChangeMasterKeyRecord found in WAL and key rotation was finished successfully.
> > 2. ChangeMasterKeyRecord found in WAL and key rotation was NOT finished successfully.
> >
> > > Meta storage will store master key id.
> >
> > We should add that key id from metastorage has a higher priority to key id from IgniteConfiguration.
> >
> >
> > В Пт, 20/09/2019 в 14:06 +0300, Nikita Amelchev пишет:
> > > Nikolay,
> > >
> > > you are right in many ways. I updated the design on the wiki. [1]
> > >
> > > [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > >
> > > пт, 20 сент. 2019 г. в 13:49, Nikolay Izhikov <ni...@apache.org>:
> > > >
> > > > Nikita
> > > >
> > > > > I suggested the implementation where the encryption manager is
> > > > > responsible for storing the master key id.
> > > >
> > > > I don't think it's a right proposal.
> > > >
> > > > 1. EncryptionSpi implementation becomes more complicated. Developer of it should be aware of Ignite deployment scenarious, etc.
> > > > Imagine implementation when EncryptionSpi send master key id to some external storage over network(it's happen in Discovery thread)
> > > >
> > > > 2. Implementation would be duplicate features(saving master key id)
> > > >
> > > > 3. We already store cache keys in metastore. For me it's a native approach to store master key id in the same place.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > > В Пт, 20/09/2019 в 13:39 +0300, Nikita Amelchev пишет:
> > > > > Nikolay,
> > > > >
> > > > > because I suggested the implementation where the encryption manager is
> > > > > responsible for storing the master key id.
> > > > > To implement this logic in the EncryptionSpi, we will need to
> > > > > introduce the methods look like this:
> > > > >
> > > > > setMasterKeyId(String masterKeyId) // Sets "current" master key id
> > > > > String getMasterKeyId() // Gets "current" master key id
> > > > >
> > > > > Follow methods will work with master key that setted by previous method:
> > > > >
> > > > > byte[] masterKeyDigest()
> > > > > byte[] encryptKey(Serializable key)
> > > > > Serializable decryptKey(byte[] key)
> > > > >
> > > > > If such implementation is more reasonable, I will do so.
> > > > >
> > > > > пт, 20 сент. 2019 г. в 13:04, Nikolay Izhikov <ni...@apache.org>:
> > > > > >
> > > > > > Why do we need "defaultMasterKeyId" instead of *current* master key id that can be obtained with `KeystoreEncryptionSpi#getMasterKeyName()`?
> > > > > >
> > > > > > В Пт, 20/09/2019 в 12:56 +0300, Nikita Amelchev пишет:
> > > > > > > Nikolay,
> > > > > > >
> > > > > > > Thanks for the proposal, I like it.
> > > > > > >
> > > > > > > The GridEncryptionManager will control the process of master key
> > > > > > > rotation, so we should provide him master key id at startup. Seems we
> > > > > > > should get it from some configuration for encryption.
> > > > > > >
> > > > > > > I suggest just adding the String defaultMasterKeyId() method into the
> > > > > > > EncryptionSpi interface. This method gets default master key id used
> > > > > > > on first cluster start.
> > > > > > >
> > > > > > > The specific implementation will be responsible for setting this value.
> > > > > > >
> > > > > > > What do you think?
> > > > > > >
> > > > > > > пт, 20 сент. 2019 г. в 10:44, Nikolay Izhikov <ni...@apache.org>:
> > > > > > > >
> > > > > > > > Hello, Nikita
> > > > > > > >
> > > > > > > > > IgniteConfiguration: New methods will be added to the IgniteConfiguration:
> > > > > > > > > public IgniteConfiguration setEncryptionMasterKeyId(String masterKeyId) - sets master key id.
> > > > > > > > > public String getEncryptionMasterKeyId()
> > > > > > > >
> > > > > > > > We don't need it in the IgniteConfiguration.
> > > > > > > >
> > > > > > > > As you may know, we already have KeystoreEncryptionSpi#setMasterKeyName.
> > > > > > > > Seems, we should add it to the EncryptionSpi itself.
> > > > > > > >
> > > > > > > >
> > > > > > > > В Ср, 18/09/2019 в 22:25 +0300, Nikita Amelchev пишет:
> > > > > > > > > Nikolay, thanks for participating.
> > > > > > > > >
> > > > > > > > > I have supplemented the design and clarify these moments. [1]
> > > > > > > > >
> > > > > > > > > [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > >
> > > > > > > > > ср, 18 сент. 2019 г. в 16:48, Nikolay Izhikov <ni...@apache.org>:
> > > > > > > > > >
> > > > > > > > > > Hello, Nikita.
> > > > > > > > > >
> > > > > > > > > > Thanks for starting this discussion.
> > > > > > > > > >
> > > > > > > > > > 1. We should add prerequisites for "master key rotation process" in design.
> > > > > > > > > > Seems, it should be, "New master key available to EncryptionSPI for each server node".
> > > > > > > > > >
> > > > > > > > > > 2. Please, use code formatting in wiki. It's make reading easier.
> > > > > > > > > >
> > > > > > > > > > 3. Please, clarify java API proposal. What will be changed and how.
> > > > > > > > > > AFAIK we need to change EncryptionSPI, this should be covered in design.
> > > > > > > > > >
> > > > > > > > > > 4. Please, clarify new CLI commands.
> > > > > > > > > > AFAIK we should have 2 command:
> > > > > > > > > >
> > > > > > > > > > 1. Start regular master key rotation process.
> > > > > > > > > > 2. Start local master key rotation process during node recovery(for the case when key changed while node was down).
> > > > > > > > > >
> > > > > > > > > > В Ср, 18/09/2019 в 16:09 +0300, Nikita Amelchev пишет:
> > > > > > > > > > > Hi, Igniters.
> > > > > > > > > > >
> > > > > > > > > > > I'm going to implement the ability to rotate the master encryption key
> > > > > > > > > > > (TDE Phase 2). [1]
> > > > > > > > > > > Master key rotation required in case of it compromising or at the end
> > > > > > > > > > > of crypto period(key validity period). I prepared the design. [2]
> > > > > > > > > > >
> > > > > > > > > > > In briefly, master keys will be identified by String masterKeyId. The
> > > > > > > > > > > concept of the masterKeyId will be added to the cache keys encryption
> > > > > > > > > > > process in EncryptionSpi.
> > > > > > > > > > >
> > > > > > > > > > > Users can configure master key id in IgniteConfiguration and will be
> > > > > > > > > > > able to manage the key rotation process from java API, JMX, CLI:
> > > > > > > > > > > - ignite.encryption().changeMasterKey(String masterKeyId) - starts
> > > > > > > > > > > master key rotation process.
> > > > > > > > > > > - String ignite.encryption().getMasterKeyId() - gets current master key id.
> > > > > > > > > > >
> > > > > > > > > > > Any thoughts?
> > > > > > > > > > >
> > > > > > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12186
> > > > > > > > > > > [2] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=95652381
> > > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
>
>
>
> --
> Best wishes,
> Amelchev Nikita