adamreeve commented on code in PR #44990:
URL: https://github.com/apache/arrow/pull/44990#discussion_r1953636356
##########
cpp/src/parquet/encryption/encryption_internal.cc:
##########
@@ -52,25 +52,64 @@ constexpr int32_t kBufferSizeLength = 4;
throw ParquetException("Couldn't init ALG decryption"); \
}
-class AesEncryptor::AesEncryptorImpl {
+class AesCryptoContext {
+ public:
+ AesCryptoContext(ParquetCipher::type alg_id, int32_t key_len, bool metadata,
+ bool write_length) {
Review Comment:
This is used for reading and writing, so maybe `include_length` would be a
better name?
##########
cpp/src/parquet/encryption/encryption_internal.cc:
##########
@@ -52,25 +52,64 @@ constexpr int32_t kBufferSizeLength = 4;
throw ParquetException("Couldn't init ALG decryption"); \
}
-class AesEncryptor::AesEncryptorImpl {
+class AesCryptoContext {
+ public:
+ AesCryptoContext(ParquetCipher::type alg_id, int32_t key_len, bool metadata,
+ bool write_length) {
+ openssl::EnsureInitialized();
+
+ length_buffer_length_ = write_length ? kBufferSizeLength : 0;
+ ciphertext_size_delta_ = length_buffer_length_ + kNonceLength;
+ if (metadata || (ParquetCipher::AES_GCM_V1 == alg_id)) {
+ aes_mode_ = kGcmMode;
+ ciphertext_size_delta_ += kGcmTagLength;
+ } else {
+ aes_mode_ = kCtrMode;
+ }
+
+ if (16 != key_len && 24 != key_len && 32 != key_len) {
+ std::stringstream ss;
+ ss << "Wrong key length: " << key_len;
+ throw ParquetException(ss.str());
+ }
+
+ key_length_ = key_len;
+ }
+
+ virtual ~AesCryptoContext() = default;
+
+ protected:
+ static void DeleteCipherContext(EVP_CIPHER_CTX* ctx) {
EVP_CIPHER_CTX_free(ctx); }
+
+ using CipherContext = std::unique_ptr<EVP_CIPHER_CTX,
decltype(&DeleteCipherContext)>;
+
+ static CipherContext NewCipherContext() {
+ auto ctx = CipherContext(EVP_CIPHER_CTX_new(), DeleteCipherContext);
+ if (!ctx) {
+ throw ParquetException("Couldn't init cipher context");
+ }
+ return ctx;
+ }
+
+ int32_t aes_mode_;
+ int32_t key_length_;
+ int32_t ciphertext_size_delta_;
+ int32_t length_buffer_length_;
+};
+
+class AesEncryptor::AesEncryptorImpl : public AesCryptoContext {
public:
explicit AesEncryptorImpl(ParquetCipher::type alg_id, int32_t key_len, bool
metadata,
bool write_length);
- ~AesEncryptorImpl() { WipeOut(); }
+ ~AesEncryptorImpl() override = default;
Review Comment:
nit: This override is redundant, and similarly for `~AesDecryptorImpl`.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
