You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/09/01 09:41:07 UTC
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9313: IGNITE-14999v3
Vladsz83 commented on a change in pull request #9313:
URL: https://github.com/apache/ignite/pull/9313#discussion_r699652560
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/StoredCacheData.java
##########
@@ -118,6 +122,20 @@ public StoredCacheData sql(boolean sql) {
return this;
}
+ /**
+ * @return Chipered encryption key for this cache or cache group. {@code Null} if not encrypted.
Review comment:
Fixed
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotMetadata.java
##########
@@ -170,6 +178,54 @@ public boolean sameSnapshot(SnapshotMetadata compare) {
Objects.equals(baselineNodes(), compare.baselineNodes());
}
+ /**
+ * @param grpId Cache id or cache group id.
+ * @return {@code True} if cache group is encrypted. {@code False} otherwise.
+ */
+ public boolean isCacheGroupEncrypted(int grpId) {
+ Set<Integer> encrGrpIds = this.encrGrpIds;
+
+ return encrGrpIds != null && encrGrpIds.contains(grpId);
+ }
+
+ /**
+ * @param masterKeyDigest Master key digest for encrypted caches.
+ * @return this meta.
+ */
+ public SnapshotMetadata masterKeyDigest(@Nullable byte[] masterKeyDigest) {
+ this.masterKeyDigest = masterKeyDigest == null ? null : masterKeyDigest.clone();
+
+ return this;
+ }
+
+ /**
+ * @return Master key digest for encrypted caches.
+ */
+ public byte[] masterKeyDigest() {
+ byte[] masterKeyDigest = this.masterKeyDigest;
+
+ return masterKeyDigest == null ? null : masterKeyDigest.clone();
+ }
+
+ /**
+ * Stores ids of encrypted cache groups.
+ *
+ * @return this meta.
+ */
+ public SnapshotMetadata encrGrpIds(Collection<Integer> encrGrpIds) {
+ // Might be null even if final due to deserialization of previous version the object;
+ if (this.encrGrpIds == null) {
+ synchronized (this) {
+ if (this.encrGrpIds == null)
+ this.encrGrpIds = new HashSet<>();
+ }
+ }
+
+ this.encrGrpIds.addAll(encrGrpIds);
Review comment:
Fixed
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotMetadata.java
##########
@@ -170,6 +178,54 @@ public boolean sameSnapshot(SnapshotMetadata compare) {
Objects.equals(baselineNodes(), compare.baselineNodes());
}
+ /**
+ * @param grpId Cache id or cache group id.
+ * @return {@code True} if cache group is encrypted. {@code False} otherwise.
+ */
+ public boolean isCacheGroupEncrypted(int grpId) {
+ Set<Integer> encrGrpIds = this.encrGrpIds;
Review comment:
to avoid NPE if class member has been nulled after `encrGrpIds != null`
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/SnapshotMetadata.java
##########
@@ -58,6 +60,9 @@
@GridToStringInclude
private final List<Integer> grpIds;
+ /** Encrypted group ids. */
+ private Set<Integer> encrGrpIds;
Review comment:
Fixed
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java
##########
@@ -993,8 +1008,19 @@ public void cancelLocalSnapshotTask(String name) {
grps.stream().collect(Collectors.toMap(CU::cacheId, v -> v));
for (List<SnapshotMetadata> nodeMetas : metas.values()) {
- for (SnapshotMetadata meta : nodeMetas)
+ for (SnapshotMetadata meta : nodeMetas) {
+ if (meta.masterKeyDigest() != null && !Arrays.equals(meta.masterKeyDigest(),
+ kctx0.config().getEncryptionSpi().masterKeyDigest())) {
Review comment:
"performs KeystoreEncryptionSpi#makeDigest operation on every" But verifying snapshot is slow operation which involves many disk reading, page reading one by one from the partition files. Is it worth?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java
##########
@@ -993,8 +1008,19 @@ public void cancelLocalSnapshotTask(String name) {
grps.stream().collect(Collectors.toMap(CU::cacheId, v -> v));
for (List<SnapshotMetadata> nodeMetas : metas.values()) {
- for (SnapshotMetadata meta : nodeMetas)
+ for (SnapshotMetadata meta : nodeMetas) {
+ if (meta.masterKeyDigest() != null && !Arrays.equals(meta.masterKeyDigest(),
+ kctx0.config().getEncryptionSpi().masterKeyDigest())) {
Review comment:
"It's possible to get NPE for kctx0.config().getEncryptionSpi().masterKeyDigest() for config with disabled encryption." Impossible. Toy can find `if (cfg.getEncryptionSpi() == null)
cfg.setEncryptionSpi(new NoopEncryptionSpi());` in IgniteEx.initializeDefaultSpi() and in `EncryptedSnapshotTest.testValidatingSnapshotFailsWithNoEncryption`
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java
##########
@@ -597,6 +606,12 @@ public File snapshotTmpDir() {
"prior to snapshot operation start: " + leftNodes));
}
+ if (!cctx.localNode().isClient() && cctx.kernalContext().encryption().isMasterKeyChangeInProgress()
Review comment:
If cctx is null for client, we fail at start: `if (cctx.kernalContext().clientNode()`. Also snapshots is disables for client nodes because clisents do not have data and have nothing to snapshot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org