Experiment with how to make YAML tests (and SQL more generally) able to encrypt records at rest #3557
Conversation
MMcM
added
enhancement
MMcM
force-pushed
the
yaml-test-encrypted-records
branch
from
f723f72 to
ccc924e
Compare
| * A text password to be used to generate an encryption key. | ||
| * Since a fixed salt is used, this is <em>not secure at all</em>. | ||
| */ | ||
| ENCRYPTION_PASSWORD, |
There was a problem hiding this comment.
Should this be used for testing only? Maybe document it as such?
| @@ -86,7 +72,7 @@ public static class RecordLayerConfigBuilder { | |||
|
|
|||
| public RecordLayerConfigBuilder() { | |||
| this.userVersionChecker = (oldUserVersion, oldMetaDataVersion, metaData) -> CompletableFuture.completedFuture(oldUserVersion); | |||
| this.serializer = DEFAULT_RELATIONAL_SERIALIZER; | |||
| this.serializer = StoreConfig.DEFAULT_RELATIONAL_SERIALIZER; | |||
There was a problem hiding this comment.
This can probably be removed?
...al-core/src/main/java/com/apple/foundationdb/relational/recordlayer/storage/StoreConfig.java
Show resolved
Hide resolved
| key = new SecretKeySpec(Base64.getDecoder().decode(keyBase64), "AES"); | ||
| } else { | ||
| // TODO: Is there a way to make this only available in YAML test framework? | ||
| final String keyPassword = options.getOption(Options.Name.ENCRYPTION_PASSWORD); |
There was a problem hiding this comment.
Maybe a better testing strategy is to check in a key to the repo and have the test harness read the file and pass it in?
A concern here is that even if this is clearly marked as "test only" there is still room for abuse or inattentive use of the feature.
| throw new RelationalException("Key generator not found", ErrorCode.UNSUPPORTED_OPERATION, ex); | ||
| } | ||
| keyGen.init(128); | ||
| key = keyGen.generateKey(); |
There was a problem hiding this comment.
What is the end goal of this code path? A one-time key that cannot be reused?
...al-core/src/main/java/com/apple/foundationdb/relational/recordlayer/storage/StoreConfig.java
Show resolved
Hide resolved
| # limitations under the License. | ||
| --- | ||
| options: | ||
| supported_version: 4.5.6.0 |
There was a problem hiding this comment.
| supported_version: 4.5.6.0 | |
| supported_version: !current_version |
| ENCRYPTION_PASSWORD: SQL | ||
| tests: | ||
| - | ||
| - query: SELECT * FROM t |
There was a problem hiding this comment.
Because of the way that requests are load-balanced across the versions, a better chance of hitting all servers will be achieved by adding more queries to the test (more than one, that is).
So for example, one more write and then read, or just more reads.
| - query: SELECT * FROM t | ||
| - error: "25F01" | ||
| --- | ||
| test_block: |
There was a problem hiding this comment.
One more flow is the writing of unencrypted data and failing to read it without key
| - | ||
| - query: SELECT * FROM t | ||
| - error: "25F01" | ||
| ... |
There was a problem hiding this comment.
The code path of using an empty (random) key was not tested. I am not sure whether that flow is a valid production flow, but it could be tested.
No description provided.