-
Notifications
You must be signed in to change notification settings - Fork 0
Tighten up fallback to per-block encryption on Encryption sequencer #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d6f8e4c
- On Encryption Sequencer, declare explicitly the conditions to allow…
avalerio-tkd 3e267f6
- Update to a unittest.
avalerio-tkd 9ef7fc6
- Updating variable names for clarity.
avalerio-tkd ce799ae
- Remove DBPSUnsupportedException from unneeded method signatures.
avalerio-tkd 2ee0c46
- Updating TODO comments.
avalerio-tkd 331f8de
- Added a more complete comment on the try-catch for the per-block fa…
avalerio-tkd a7b99f0
- Updated spacing and formatting.
avalerio-tkd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,24 +140,49 @@ bool DataBatchEncryptionSequencer::ConvertAndEncrypt(const std::vector<uint8_t>& | |
| // Compress the joined encrypted bytes | ||
| encrypted_result_ = Compress(joined_encrypted_bytes, encrypted_compression_); | ||
|
|
||
| } catch (const DBPSUnsupportedException& e) { | ||
| // If any stage is as of yet unsupported, default to whole payload (per-block) encryption | ||
| // (as opposed to per-value) | ||
| } | ||
| // If the sequence was interrupted by a DBPSUnsupportedException, allow fallback to per-block encryption | ||
| // but only for explicitly unsupported conditions. | ||
| // Any conditions that are already supported should not fallback. In those cases, the exception is re-thrown. | ||
| catch (const DBPSUnsupportedException& e) { | ||
|
argmarco-tkd marked this conversation as resolved.
|
||
|
|
||
| // Compression: Only UNCOMPRESSED and SNAPPY are supported | ||
| // TODO(Issue #188): Add support for other compressions. | ||
| const bool is_compression_supported = (compression_ == CompressionCodec::UNCOMPRESSED || | ||
| compression_ == CompressionCodec::SNAPPY); | ||
|
|
||
| // Format: Only PLAIN is supported | ||
| // TODO(Issue #187): Add support for other encodings. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to above - let's keep the TODO only if it's work we plan to complete in the project. Else, let's move these as notes for the future.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| const bool is_format_supported = (format_ == Format::PLAIN); | ||
|
|
||
| // Page type: All are supported (DATA_PAGE_V1, DATA_PAGE_V2, DICTIONARY_PAGE) | ||
| const bool is_page_supported = true; | ||
|
|
||
| // Datatype: All datatypes are supported. | ||
| const bool is_datatype_supported = true; | ||
|
|
||
| if (is_compression_supported && is_format_supported && is_page_supported && is_datatype_supported) { | ||
| // All conditions are supported, therefore an DBPSUnsupportedException exception should not have happened. | ||
| // Re-throw the exception. | ||
| throw; | ||
| } | ||
|
|
||
| // Fallback: Use per-block encryption for unsupported combinations. | ||
| encrypted_result_ = encryptor_->EncryptBlock(plaintext); | ||
| if (encrypted_result_.empty()) { | ||
| error_stage_ = "encryption"; | ||
| error_message_ = "Failed to encrypt data"; | ||
| return false; | ||
| } | ||
| // If the sequence was interrupted by a DBPSUnsupportedException at any point, use per block encryption. | ||
| // Set the encryption type to per block. | ||
| encryption_metadata_[ENCRYPTION_MODE] = ENCRYPTION_PER_BLOCK; | ||
| encryption_metadata_[DBPS_VERSION_KEY] = DBPS_VERSION; | ||
| return true; | ||
|
|
||
| } catch (const InvalidInputException& e) { | ||
| // Throw the exception so it can be caught by the caller. | ||
| throw; | ||
| } | ||
|
|
||
| // If the sequencer got here, it means the encryption of the values in the typed list finished successfully. | ||
| // Set the encryption type to per value | ||
| encryption_metadata_[ENCRYPTION_MODE] = ENCRYPTION_PER_VALUE; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is something we want to tackle as part of the project, keep the TODO. if it's a future extension, I recommend removing the todo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.