Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou merged PR #46017: URL: https://github.com/apache/arrow/pull/46017 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3074075680 @github-actions crossbow submit -g cpp -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
github-actions[bot] commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3074091024 Revision: 04b15e2bfc94970670605e3fe21bc45680b29b67 Submitted crossbow builds: [ursacomputing/crossbow @ actions-12f2531100](https://github.com/ursacomputing/crossbow/branches/all?query=actions-12f2531100) |Task|Status| ||--| |example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258664/job/46022126266)| |example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258568/job/46022126311)| |example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258777/job/46022127196)| |test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258158/job/46022124809)| |test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258721/job/46022126449)| |test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258575/job/46022126172)| |test-cuda-cpp-ubuntu-22.04-cuda-11.7.1|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258545/job/46022125938)| |test-debian-12-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/16297259030/job/46022127573)| |test-debian-12-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258229/job/46022124978)| |test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258438/job/46022125509)| |test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258445/job/46022125596)| |test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258276/job/46022125324)| |test-ubuntu-22.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258154/job/46022124814)| |test-ubuntu-22.04-cpp-emscripten|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258787/job/46022126710)| |test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258720/job/46022126549)| |test-ubuntu-24.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/16297258152/job/46022124852)| |test-ubuntu-24.04-cpp-bundled-offline|[![GitHub
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2207780809
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -46,28 +48,56 @@ using ColumnPathToEncryptionPropertiesMap =
class PARQUET_EXPORT DecryptionKeyRetriever {
public:
- virtual std::string GetKey(const std::string& key_metadata) = 0;
+ /// \brief Retrieve a key.
+ /// \deprecated Deprecated since 21.0.0.
Review Comment:
Ahem. 21.0.0 is nearly released, so we should actually rename all these
deprecation notices to say 22.0.0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3058485182 :heavy_check_mark: All green. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3057082335 @EnricoMi We have two +1s after a meticulous review pass, so perhaps you can go ahead and revert the compatibility-breaking changes so that we get a clean CI and merge. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
wgtmac commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2195374415
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -46,28 +48,39 @@ using ColumnPathToEncryptionPropertiesMap =
class PARQUET_EXPORT DecryptionKeyRetriever {
public:
- virtual std::string GetKey(const std::string& key_metadata) = 0;
+ /// \brief Retrieve a key.
+ virtual ::arrow::util::SecureString GetKey(const std::string& key_id) = 0;
+
virtual ~DecryptionKeyRetriever() {}
};
/// Simple integer key retriever
class PARQUET_EXPORT IntegerKeyIdRetriever : public DecryptionKeyRetriever {
public:
- void PutKey(uint32_t key_id, const std::string& key);
- std::string GetKey(const std::string& key_metadata) override;
+ void PutKey(uint32_t key_id, ::arrow::util::SecureString key);
+
+ ::arrow::util::SecureString GetKey(const std::string& key_id_string)
override {
+// key_id_string is string but for IntegerKeyIdRetriever it encodes
Review Comment:
It is indeed surprising...
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2195224148
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -46,28 +48,39 @@ using ColumnPathToEncryptionPropertiesMap =
class PARQUET_EXPORT DecryptionKeyRetriever {
public:
- virtual std::string GetKey(const std::string& key_metadata) = 0;
+ /// \brief Retrieve a key.
+ virtual ::arrow::util::SecureString GetKey(const std::string& key_id) = 0;
+
virtual ~DecryptionKeyRetriever() {}
};
/// Simple integer key retriever
class PARQUET_EXPORT IntegerKeyIdRetriever : public DecryptionKeyRetriever {
public:
- void PutKey(uint32_t key_id, const std::string& key);
- std::string GetKey(const std::string& key_metadata) override;
+ void PutKey(uint32_t key_id, ::arrow::util::SecureString key);
+
+ ::arrow::util::SecureString GetKey(const std::string& key_id_string)
override {
+// key_id_string is string but for IntegerKeyIdRetriever it encodes
Review Comment:
Fair enough. I do find the implementation weird as well, and I'm not sure
this class is actually useful, but hey :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2195222379
##
cpp/src/parquet/encryption/encryption.cc:
##
@@ -26,31 +26,30 @@
#include "arrow/util/utf8.h"
#include "parquet/encryption/encryption_internal.h"
-namespace parquet {
+using ::arrow::util::SecureString;
-// integer key retriever
-void IntegerKeyIdRetriever::PutKey(uint32_t key_id, const std::string& key) {
- key_map_.insert({key_id, key});
-}
+namespace parquet {
-std::string IntegerKeyIdRetriever::GetKey(const std::string& key_metadata) {
- uint32_t key_id;
- memcpy(reinterpret_cast(&key_id), key_metadata.c_str(), 4);
+// any empty SecureString key is interpreted as if no key is given
+// this instance is used when a SecureString reference is returned
+const SecureString no_key_ = SecureString();
Review Comment:
The naming is a bit off as the trailing underscore is used for member
variables. You could make it `kNoKey` for example.
(can also make it `static` by the way)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2111781121
##
cpp/src/parquet/encryption/secure_string.h:
##
@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include
+#include
+
+#include "arrow/util/span.h"
+#include "parquet/platform.h"
+
+namespace parquet::encryption {
Review Comment:
I have extracted the `SecureString` implementation into a separate PR:
#46626. That targets `arrow/util`.
This PR will focus in migrating parquet encryption to that `SecureString`
implementation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3052565463 Maybe @bkietz and @wgtmac want to make a final pass over the PR. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2194937368
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -46,28 +48,39 @@ using ColumnPathToEncryptionPropertiesMap =
class PARQUET_EXPORT DecryptionKeyRetriever {
public:
- virtual std::string GetKey(const std::string& key_metadata) = 0;
+ /// \brief Retrieve a key.
+ virtual ::arrow::util::SecureString GetKey(const std::string& key_id) = 0;
+
virtual ~DecryptionKeyRetriever() {}
};
/// Simple integer key retriever
class PARQUET_EXPORT IntegerKeyIdRetriever : public DecryptionKeyRetriever {
public:
- void PutKey(uint32_t key_id, const std::string& key);
- std::string GetKey(const std::string& key_metadata) override;
+ void PutKey(uint32_t key_id, ::arrow::util::SecureString key);
+
+ ::arrow::util::SecureString GetKey(const std::string& key_id_string)
override {
+// key_id_string is string but for IntegerKeyIdRetriever it encodes
Review Comment:
@bkietz suggested the implementation given the signature is somewhat
surprising: https://github.com/apache/arrow/pull/46017#discussion_r2029146076
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2194955749
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -310,18 +325,22 @@ class PARQUET_EXPORT FileDecryptionProperties {
}
private:
- std::string footer_key_;
+ ::arrow::util::SecureString footer_key_;
std::string aad_prefix_;
std::shared_ptr aad_prefix_verifier_;
+ // any empty SecureString key is interpreted as if no key is given
+ // this instance is used if a SecureString reference is returned
+ const ::arrow::util::SecureString no_key_ = ::arrow::util::SecureString();
Review Comment:
Done in 184afcfd6d.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2194937368
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -46,28 +48,39 @@ using ColumnPathToEncryptionPropertiesMap =
class PARQUET_EXPORT DecryptionKeyRetriever {
public:
- virtual std::string GetKey(const std::string& key_metadata) = 0;
+ /// \brief Retrieve a key.
+ virtual ::arrow::util::SecureString GetKey(const std::string& key_id) = 0;
+
virtual ~DecryptionKeyRetriever() {}
};
/// Simple integer key retriever
class PARQUET_EXPORT IntegerKeyIdRetriever : public DecryptionKeyRetriever {
public:
- void PutKey(uint32_t key_id, const std::string& key);
- std::string GetKey(const std::string& key_metadata) override;
+ void PutKey(uint32_t key_id, ::arrow::util::SecureString key);
+
+ ::arrow::util::SecureString GetKey(const std::string& key_id_string)
override {
+// key_id_string is string but for IntegerKeyIdRetriever it encodes
Review Comment:
@bkietz suggested this given the implementation given the signature is
somewhat surprising:
https://github.com/apache/arrow/pull/46017#discussion_r2029146076
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2194773454
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -310,18 +325,22 @@ class PARQUET_EXPORT FileDecryptionProperties {
}
private:
- std::string footer_key_;
+ ::arrow::util::SecureString footer_key_;
std::string aad_prefix_;
std::shared_ptr aad_prefix_verifier_;
+ // any empty SecureString key is interpreted as if no key is given
+ // this instance is used if a SecureString reference is returned
+ const ::arrow::util::SecureString no_key_ = ::arrow::util::SecureString();
Review Comment:
Does this need to be an instance variable? It could just be a private
constant inside `encryption.cc`
##
cpp/src/parquet/encryption/encryption.h:
##
@@ -46,28 +48,39 @@ using ColumnPathToEncryptionPropertiesMap =
class PARQUET_EXPORT DecryptionKeyRetriever {
public:
- virtual std::string GetKey(const std::string& key_metadata) = 0;
+ /// \brief Retrieve a key.
+ virtual ::arrow::util::SecureString GetKey(const std::string& key_id) = 0;
+
virtual ~DecryptionKeyRetriever() {}
};
/// Simple integer key retriever
class PARQUET_EXPORT IntegerKeyIdRetriever : public DecryptionKeyRetriever {
public:
- void PutKey(uint32_t key_id, const std::string& key);
- std::string GetKey(const std::string& key_metadata) override;
+ void PutKey(uint32_t key_id, ::arrow::util::SecureString key);
+
+ ::arrow::util::SecureString GetKey(const std::string& key_id_string)
override {
+// key_id_string is string but for IntegerKeyIdRetriever it encodes
Review Comment:
I'm not sure it's worth inlining this method, is it? Especially as it will
usually be called through the vtable, anyway.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3048893227 > > But this will require migrating the Python API as well: https://github.com/apache/arrow/actions/runs/16126071702/job/45503157743?pr=46017#step:6:3956 > > I would like to move touching the Python API to a separate PR, which would contain the revert [290449a](https://github.com/apache/arrow/pull/46017/commits/290449a36c4b45fc5e6be931a6a12b378d80cdf4). > > Ok, that sounds good to me. Alright, once you approve this PR, I will revert [290449a](https://github.com/apache/arrow/pull/46017/commits/290449a36c4b45fc5e6be931a6a12b378d80cdf4) and we are ready to merge. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3048599013 > [](https://github.com/apache/arrow/pull/46017/commits/290449a36c4b45fc5e6be931a6a12b378d80cdf4) > > But this will require migrating the Python API as well: https://github.com/apache/arrow/actions/runs/16126071702/job/45503157743?pr=46017#step:6:3956 > > I would like to move touching the Python API to a separate PR, which would contain the revert [290449a](https://github.com/apache/arrow/pull/46017/commits/290449a36c4b45fc5e6be931a6a12b378d80cdf4). Ok, that sounds good to me. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3046352212 @pitrou I have removed the deprecated methods to "encourage" users to migrate to the new methods: [290449a](https://github.com/apache/arrow/pull/46017/commits/290449a36c4b45fc5e6be931a6a12b378d80cdf4) But this will require migrating the Python API as well: https://github.com/apache/arrow/actions/runs/16126071702/job/45503157743?pr=46017#step:6:3956 I would like to move touching the Python API to a separate PR, which would contain the revert [290449a](https://github.com/apache/arrow/pull/46017/commits/290449a36c4b45fc5e6be931a6a12b378d80cdf4). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2182909723
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
Review Comment:
In this case, yes, since the new idioms are more secure.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2182908014
##
cpp/src/parquet/encryption/internal_file_decryptor.cc:
##
@@ -148,9 +150,9 @@ InternalFileDecryptor::GetColumnDecryptorFactory(
// The column is encrypted with its own key
const std::string& column_key_metadata = crypto_metadata->key_metadata();
const std::string column_path =
crypto_metadata->path_in_schema()->ToDotString();
- std::string column_key = GetColumnKey(column_path, column_key_metadata);
+ const SecureString& column_key = GetColumnKey(column_path,
column_key_metadata);
- return [this, aad, metadata, column_key = std::move(column_key)]() {
+ return [this, aad, metadata, column_key = column_key]() {
Review Comment:
Ok, I found the answer: it does a copy, so this is fine.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2182839492
##
cpp/src/parquet/encryption/internal_file_decryptor.cc:
##
@@ -148,9 +150,9 @@ InternalFileDecryptor::GetColumnDecryptorFactory(
// The column is encrypted with its own key
const std::string& column_key_metadata = crypto_metadata->key_metadata();
const std::string column_path =
crypto_metadata->path_in_schema()->ToDotString();
- std::string column_key = GetColumnKey(column_path, column_key_metadata);
+ const SecureString& column_key = GetColumnKey(column_path,
column_key_metadata);
- return [this, aad, metadata, column_key = std::move(column_key)]() {
+ return [this, aad, metadata, column_key = column_key]() {
Review Comment:
It's more of a question from me: is `column_key` being captured by value
here? I'm not enough of a C++ lawyer :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2182825901
##
cpp/src/parquet/encryption/internal_file_decryptor.cc:
##
@@ -148,9 +150,9 @@ InternalFileDecryptor::GetColumnDecryptorFactory(
// The column is encrypted with its own key
const std::string& column_key_metadata = crypto_metadata->key_metadata();
const std::string column_path =
crypto_metadata->path_in_schema()->ToDotString();
- std::string column_key = GetColumnKey(column_path, column_key_metadata);
+ const SecureString& column_key = GetColumnKey(column_path,
column_key_metadata);
- return [this, aad, metadata, column_key = std::move(column_key)]() {
+ return [this, aad, metadata, column_key = column_key]() {
Review Comment:
Any action item for me? Shall I leave a comment in the code on this?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2182823133
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
+ }
- /// Decrypts (unwraps) a key with the master key.
+ /// \brief Decrypts (unwraps) a key with the master key.
+ /// \deprecated Deprecated since 21.0.0. Implement UnWrapKey instead.
+ ARROW_DEPRECATED("Deprecated in 21.0.0. Implement UnWrapKey instead.")
virtual std::string UnwrapKey(const std::string& wrapped_key,
-const std::string& master_key_identifier) = 0;
+const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc UnwrapKey(const std::string&, const std::string&)
Review Comment:
Docu say:
> Note that \copydoc foo() is roughly equivalent to doing:
> \brief \copybrief foo()
> \details \copydetails foo()
https://www.doxygen.nl/manual/commands.html#cmdcopydoc
which sounds like this would not copy the deprecation.
But you are right, to be sure I can duplicate the simple doc.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2182810826
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
Review Comment:
So you prefer to force user code to be migrated in order to compile against
the new Arrow version? Happy to do that, simplifies my code changes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179597718
##
cpp/src/parquet/encryption/internal_file_decryptor.cc:
##
@@ -148,9 +150,9 @@ InternalFileDecryptor::GetColumnDecryptorFactory(
// The column is encrypted with its own key
const std::string& column_key_metadata = crypto_metadata->key_metadata();
const std::string column_path =
crypto_metadata->path_in_schema()->ToDotString();
- std::string column_key = GetColumnKey(column_path, column_key_metadata);
+ const SecureString& column_key = GetColumnKey(column_path,
column_key_metadata);
- return [this, aad, metadata, column_key = std::move(column_key)]() {
+ return [this, aad, metadata, column_key = column_key]() {
Review Comment:
Hmm, we must capture the column key by value here. It's not obvious that
it's what `column_key = column_key` does.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179579428
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
+ }
- /// Decrypts (unwraps) a key with the master key.
+ /// \brief Decrypts (unwraps) a key with the master key.
+ /// \deprecated Deprecated since 21.0.0. Implement UnWrapKey instead.
+ ARROW_DEPRECATED("Deprecated in 21.0.0. Implement UnWrapKey instead.")
virtual std::string UnwrapKey(const std::string& wrapped_key,
-const std::string& master_key_identifier) = 0;
+const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc UnwrapKey(const std::string&, const std::string&)
+ virtual ::arrow::util::SecureString UnWrapKey(
+ const std::string& wrapped_key, const std::string&
master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
Review Comment:
Same question here re. the default implementation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179578572
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
Review Comment:
Is this default implementation provided so that old KmsClient
implementations that don't override it can still compile?
It doesn't sound like a terrific idea to me. We want people to implement the
new method, IMHO. It will break compatibility slightly, but for the better.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179573785
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
+ }
- /// Decrypts (unwraps) a key with the master key.
+ /// \brief Decrypts (unwraps) a key with the master key.
+ /// \deprecated Deprecated since 21.0.0. Implement UnWrapKey instead.
+ ARROW_DEPRECATED("Deprecated in 21.0.0. Implement UnWrapKey instead.")
Review Comment:
Perhaps be more specific about the difference (i.e. it's not a name change).
For example "Implement SecureString-returning UnWrapKey instead".
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179571971
##
cpp/src/parquet/encryption/kms_client.h:
##
@@ -79,14 +80,47 @@ class PARQUET_EXPORT KmsClient {
static constexpr const char kKmsInstanceUrlDefault[] = "DEFAULT";
static constexpr const char kKeyAccessTokenDefault[] = "DEFAULT";
- /// Wraps a key - encrypts it with the master key, encodes the result
+ /// \brief Wraps a key.
+ ///
+ /// Encrypts it with the master key, encodes the result
/// and potentially adds a KMS-specific metadata.
+ ///
+ /// \deprecated Deprecated since 21.0.0. Implement
+ /// WrapKey(const SecureString&, const std::string&) instead.
+ ARROW_DEPRECATED(
+ "Deprecated in 21.0.0. "
+ "Implement WrapKey(const SecureString&, const std::string&) instead.")
virtual std::string WrapKey(const std::string& key_bytes,
- const std::string& master_key_identifier) = 0;
+ const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc WrapKey(const std::string&, const std::string&)
+ virtual std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
+ const std::string& master_key_identifier) {
+ARROW_SUPPRESS_DEPRECATION_WARNING
+auto key = WrapKey(std::string(key_bytes.as_view()),
master_key_identifier);
+ARROW_UNSUPPRESS_DEPRECATION_WARNING
+return key;
+ }
- /// Decrypts (unwraps) a key with the master key.
+ /// \brief Decrypts (unwraps) a key with the master key.
+ /// \deprecated Deprecated since 21.0.0. Implement UnWrapKey instead.
+ ARROW_DEPRECATED("Deprecated in 21.0.0. Implement UnWrapKey instead.")
virtual std::string UnwrapKey(const std::string& wrapped_key,
-const std::string& master_key_identifier) = 0;
+const std::string& master_key_identifier) {
+throw ParquetException("Not implemented");
+ }
+
+ /// \copydoc UnwrapKey(const std::string&, const std::string&)
Review Comment:
This might also copy the deprecation mention, no? The docstring is
simple-enough that it can be copy-pasted, IMHO.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179569730
##
cpp/src/parquet/encryption/local_wrap_kms_client.h:
##
@@ -35,16 +35,16 @@ class PARQUET_EXPORT LocalWrapKmsClient : public KmsClient {
explicit LocalWrapKmsClient(const KmsConnectionConfig&
kms_connection_config);
- std::string WrapKey(const std::string& key_bytes,
+ std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
const std::string& master_key_identifier) override;
- std::string UnwrapKey(const std::string& wrapped_key,
-const std::string& master_key_identifier) override;
+ ::arrow::util::SecureString UnWrapKey(
Review Comment:
Ah, sorry, I was reading the diff backwards. I understand the rationale now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179567415
##
cpp/src/parquet/encryption/local_wrap_kms_client.h:
##
@@ -35,16 +35,16 @@ class PARQUET_EXPORT LocalWrapKmsClient : public KmsClient {
explicit LocalWrapKmsClient(const KmsConnectionConfig&
kms_connection_config);
- std::string WrapKey(const std::string& key_bytes,
+ std::string WrapKey(const ::arrow::util::SecureString& key_bytes,
const std::string& master_key_identifier) override;
- std::string UnwrapKey(const std::string& wrapped_key,
-const std::string& master_key_identifier) override;
+ ::arrow::util::SecureString UnWrapKey(
Review Comment:
Isn't it a bit gratuitous to change the method name casing from `UnwrapKey`
to `UnWrapKey`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2179565092
##
cpp/src/parquet/encryption/test_encryption_util.h:
##
@@ -40,12 +41,13 @@ class ParquetFileReader;
namespace encryption::test {
using ::arrow::internal::TemporaryDir;
+using ::arrow::util::SecureString;
constexpr int kFixedLength = 10;
-const char kFooterEncryptionKey[] = "0123456789012345"; // 128bit/16
-const char kColumnEncryptionKey1[] = "1234567890123450";
-const char kColumnEncryptionKey2[] = "1234567890123451";
+inline SecureString kFooterEncryptionKey("0123456789012345");
+inline SecureString kColumnEncryptionKey1("1234567890123450");
+inline SecureString kColumnEncryptionKey2("1234567890123451");
Review Comment:
Nit, but perhaps we can make these `const`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3027108202 I agree with returning by value, I don't think it's a problem that it's being copied. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
kapoisu commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3027049211 > @kapoisu since you have touched `encryption.h|cc` recently, please have a look if my changes respect your code style introduced in #46202. I generally tend to return by value for getters to be safe. It reduces the possibility of the object being modified externally and ensures the return value won't dangle even if the original object doesn't outlive it. That said, if the return value is large and the getter is called frequently, returning a reference may be reasonable. Either way, this is just a small trade-off that depends on the use cases. Defining no_key_ as ```const``` unnecessarily prevents the object from being copy-assigned. If the object doesn't allocate heap memory, I usually just return an empty instance instead, or make it ```constexpr static```. Other than that, LGTM — thank you for your hard work! :) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3026742307 @kapoisu since you have touched `encryption.h|cc` recently, please have a look if my changes respect your code style introduced in #46202. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3026660908 > @pitrou yes, I do. I think I have addressed all comments. Ah, thank you. I was wondering, because this PR is still in the "draft" state. Do you want to mark it ready for review? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
EnricoMi commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3025489974 @pitrou yes, I do. I think I have addressed all comments. I have deprecated the string-based methods. What do you think about the new method names? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
github-actions[bot] commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3025482817 :warning: GitHub issue #31603 **has been automatically assigned in GitHub** to PR creator. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
pitrou commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-3024361423 @EnricoMi Are you planning to update this PR? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-31603: [C++] Wrap Parquet encryption keys in SecureString [arrow]
github-actions[bot] commented on PR #46017: URL: https://github.com/apache/arrow/pull/46017#issuecomment-2948949098 :warning: GitHub issue #31603 **has been automatically assigned in GitHub** to PR creator. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
