Supporting CPK (Customer Provided Keys) in Azure Blob Storage requests by Braedon-Wooding-Displayr · Pull Request #742 · apache/arrow-rs-object-store

Braedon-Wooding-Displayr · 2026-06-09T11:26:13Z

Which issue does this PR close?

Closes Supporting CPKs for Azure Blob Requests #741

Add customer-provided key support to the Azure backend by wiring CPK headers through all blob reads, writes, heads, and deletes, with builder-level configuration and test coverage for ranged reads and real-account behavior.

Rationale for this change

I think this is the cleanest way to solve this, matches similar style for other azure configuration.

What changes are included in this PR?

New configuration key encryption_key.

Are there any user-facing changes?

New configuration key encryption_key but I've already updated the docs comment for it, so it should be okay.

Braedon-Wooding-Displayr · 2026-06-09T11:27:51Z

        }
    }

+    // Azurite doesn't support CPK (just ignores it)


I've run some tests against some real storage accounts (using this + a manual cmd line app), looks good.

It really is just passing 3 headers.

kevinjqliu

Generally LGTM.

One small suggestion: could we move the encryption header application into the request-builder extension layer, similar to with_azure_authorization? Something like with_azure_encryption_headers(...) would make the call sites a bit cleaner and keep Azure request-specific header handling in one place.

BTW, it looks like copy_request doesn’t utilize CPK. Is that intentional?

Here’s the Azure CPK documentation for reference:
https://learn.microsoft.com/en-us/azure/storage/blobs/encryption-customer-provided-keys#request-headers-for-specifying-customer-provided-keys

kevinjqliu · 2026-06-09T17:42:29Z

+                source: Box::new(source),
+            })?;
+
+        if decoded_key.len() != 32 {


nit: why 32 here?

is it because

An AES-256 key is 256 bits = 32 bytes raw.

Yes it is! I'll add a comment.

kevinjqliu · 2026-06-09T17:43:09Z

+                .header(&ENCRYPTION_KEY_HEADER, encryption_key)
+                .header(&ENCRYPTION_KEY_SHA256_HEADER, encryption_key_sha256)
+                // Azure only supports this algorithm header
+                .header(&ENCRYPTION_ALGORITHM_HEADER, "AES256"),


nit: move this string up to the class

nit: i still see the string here :P

Oops, yep, moved it up to a class constant. Happy to move it somewhere else too (since this class structure has changed).

kevinjqliu · 2026-06-09T17:59:14Z

+        let builder = self.config.encryption_headers.apply(self.builder);
+        let response = builder
            .header(CONTENT_LENGTH, self.payload.content_length())
            .with_azure_authorization(&credential, &self.config.account)


nit: this call feels awkward, could we move this into the request-builder extension layer, similar to with_azure_authorization?

Right now AzureEncryptionHeaders::apply(builder) feels a little detached from the rest of the request construction, and it also makes it easy to apply the CPK headers without remembering that the request now contains secret material. A fluent helper like with_azure_encryption_headers(&self.config.encryption_headers) would match the existing style better.

Ideally that helper would also mark the request as sensitive when x-ms-encryption-key is present, so callers do not need to separately remember to combine credential sensitivity with CPK sensitivity.

Yeh, this is fair.

I unified the 2 sensitive handling and moved the handling to a EncryptionHeadersExt to match CredentialExt.

kevinjqliu · 2026-06-09T18:03:46Z

    }
 }

+#[derive(Default, Clone, Debug)]


nit: Can we avoid deriving Debug here? This struct contains the raw customer-provided encryption key, so debug output could leak key material. I’d prefer a custom Debug impl that only reports whether a key is configured, and maybe the SHA-256 value if we consider that safe enough, but not the raw key.

Done, I decided not to leak the SHA_256 hash since that is pretty secure still.

kevinjqliu · 2026-06-09T18:14:15Z

+        assert_eq!(
+            builder.get_config_value(&AzureConfigKey::EncryptionKey).as_deref(),
+            Some(key.as_str())
+        );


could we also add tests for invalid values

malformed base64

base64 that decodes to fewer than 32 bytes

base64 that decodes to more than 32 bytes

The happy-path roundtrip test is useful, but the main correctness behavior here is that invalid keys fail at build time rather than producing bad Azure requests.

Added these as well.

Braedon-Wooding-Displayr · 2026-06-10T01:36:53Z

I've fixed all of above @kevinjqliu / @kylebarron

I've extend it to support copy blobs (requires a different source- header). This means that both destination & source will share the same CPK. This is something only supported in 2026-02-06 so I've had to do a version upgrade when using this. I don't want to upgrade our base version required because azurite often trails behind and this is very recent, so instead I've just applied the version upgrade when this scenario is hit. Happy to simplify this down just upgrading our base version if this is too much.

Ready for re-review.

kevinjqliu

Thanks for working on this! Super excited for this feature for blob and OneLake.

Another round of comments, most are just on more documentations.

One important thing to point out about is_sensitive. It currently only suppress URL. CPK leaves in the headers and is not redacted by setting the flag. I added a comment explain this behavior. We should redact the headers but this is getting into the scope creep territory

kevinjqliu · 2026-06-10T18:43:29Z

+static SOURCE_ENCRYPTION_KEY_HEADER: HeaderName =
+    HeaderName::from_static("x-ms-source-encryption-key");
+static SOURCE_ENCRYPTION_KEY_SHA256_HEADER: HeaderName =
+    HeaderName::from_static("x-ms-source-encryption-key-sha256");
+static SOURCE_ENCRYPTION_ALGORITHM_HEADER: HeaderName =
+    HeaderName::from_static("x-ms-source-encryption-algorithm");
+// Put Blob From URL added source CPK headers in 2026-02-06.
+const PUT_BLOB_FROM_URL_SOURCE_CPK_VERSION: &str = "2026-02-06";


nit: took me a while to figure out the "source" headers and "2026-02-06"

could we add a comment on why this is necessary? Its used for copy_request and 2026-02-06 is the first supported version. Its newer and than the base version and thus the requires the override

https://learn.microsoft.com/en-us/rest/api/storageservices/version-2026-02-06

kevinjqliu · 2026-06-10T18:44:39Z

+    /// when customer-provided encryption keys are configured, since CPK requests
+    /// carry secret key material. Combining both here means callers only need to
+    /// remember a single source of truth for request sensitivity.
+    fn is_sensitive(&self, credential: &Option<Arc<AzureCredential>>) -> bool {


nit: i noticed a few places still calculate is_sensitive using previous method. can we update all for uniformity?
In

get_user_delegation_key

list_request

kevinjqliu · 2026-06-10T18:47:07Z

+                .header(&ENCRYPTION_KEY_HEADER, encryption_key)
+                .header(&ENCRYPTION_KEY_SHA256_HEADER, encryption_key_sha256)
+                // Azure only supports this algorithm header
+                .header(&ENCRYPTION_ALGORITHM_HEADER, "AES256"),


nit: i still see the string here :P

kevinjqliu · 2026-06-10T18:53:13Z

+///
+/// Mirrors [`CredentialExt`] so that CPK headers are applied as part of the
+/// fluent request-construction chain, keeping the secret key material close to
+/// the rest of the request building rather than as a detached side-call.


Suggested change

///

/// Mirrors [`CredentialExt`] so that CPK headers are applied as part of the

/// fluent request-construction chain, keeping the secret key material close to

/// the rest of the request building rather than as a detached side-call.

nit: i feel like we dont need this explanation

kevinjqliu · 2026-06-10T18:54:43Z

+/// fluent request-construction chain, keeping the secret key material close to
+/// the rest of the request building rather than as a detached side-call.
+pub(crate) trait EncryptionHeadersExt {
+    /// Apply the customer-provided encryption headers for the request target.


Suggested change

/// Apply the customer-provided encryption headers for the request target.

/// Apply the customer-provided encryption headers for the request target.

/// <https://learn.microsoft.com/en-us/azure/storage/blobs/encryption-customer-provided-keys>

kevinjqliu · 2026-06-11T17:58:37Z

+    ///
+    /// [`with_azure_authorization`](Self::with_azure_authorization) preserves an
+    /// explicit version header instead of replacing it with the backend default.
+    fn with_azure_version(self, version: &'static str) -> Self;


nit: agent suggested pulling this out into its own trait since its not tied to authorization

/// Override the Azure Blob service version used for a single request. /// /// [`with_azure_authorization`](CredentialExt::with_azure_authorization) preserves /// an explicit version header instead of replacing it with the backend default. pub(crate) trait RequestVersionExt { fn with_azure_version(self, version: &'static str) -> Self; }

impl RequestVersionExt for HttpRequestBuilder { fn with_azure_version(self, version: &'static str) -> Self { self.header(&VERSION, version) } }

Yeh I was being lazy here keeping it in credentials, moved out.

kevinjqliu · 2026-06-11T18:02:15Z

Suggested change

.retryable(&self.config.retry_config)

.sensitive(sensitive)

.send()

propagate sensitive on retry

kevinjqliu · 2026-06-11T18:02:49Z

+    /// Make an Azure copy request <https://docs.microsoft.com/en-us/rest/api/storageservices/copy-blob>.
+    ///
+    /// When customer-provided keys are enabled, this request uses the Put Blob
+    /// From URL header shape and must opt into the service version that added
+    /// source CPK headers.


Suggested change

/// Make an Azure copy request <https://docs.microsoft.com/en-us/rest/api/storageservices/copy-blob>.

///

/// When customer-provided keys are enabled, this request uses the Put Blob

/// From URL header shape and must opt into the service version that added

/// source CPK headers.

/// Make an Azure copy request <https://docs.microsoft.com/en-us/rest/api/storageservices/copy-blob>.

///

/// The classic `Copy Blob` API does not accept CPK headers, so when

/// customer-provided keys are enabled this falls back to

/// [Put Blob From URL][put-blob-from-url] and opts into the service version

/// that added source CPK headers. That changes the semantics: the operation

/// is synchronous, the source must be a block blob no larger than 5,000 MiB,

/// and uncommitted blocks / the source block list are not preserved.

///

/// [put-blob-from-url]: https://learn.microsoft.com/en-us/rest/api/storageservices/put-blob-from-url

kevinjqliu · 2026-06-11T18:03:28Z

+    /// This is true when the credential is carried in the URL (SAS tokens) or
+    /// when customer-provided encryption keys are configured, since CPK requests
+    /// carry secret key material. Combining both here means callers only need to
+    /// remember a single source of truth for request sensitivity.


Suggested change

/// This is true when the credential is carried in the URL (SAS tokens) or

/// when customer-provided encryption keys are configured, since CPK requests

/// carry secret key material. Combining both here means callers only need to

/// remember a single source of truth for request sensitivity.

/// The retry layer's `sensitive` flag suppresses the request URL from

/// error messages (see [`RetryableRequestBuilder::sensitive`]). For SAS

/// credentials this is load-bearing because the token is carried as URL

/// query parameters.

///

/// CPK material lives in request *headers* (`x-ms-encryption-key` etc.),

/// not in the URL, so today's URL-only redaction does not actively hide

/// it. The flag is still set for CPK requests so that any future

/// expansion of the redaction surface (headers, response bodies) covers

/// CPK without further changes here, and so that operators have a single

/// "this request touches secret material" signal for both auth modes.

///

/// [`RetryableRequestBuilder::sensitive`]: crate::client::retry::RetryableRequestBuilder

alamb

Thank you @Braedon-Wooding-Displayr
In general, I am not likely to have time to help research and review all Azure specific features.

I will defer to @kevinjqliu on the azure specifics.

I think we were trying to keep the object store specific feature configuration relatively small in this repo, and fallback to the cloud provider native SDKs for the more advanced (but less used) features. Is that relevant for this PR?

kevinjqliu · 2026-06-12T19:21:29Z

CI issue is unrelated to this PR, can rebase #746 to resolve it

Braedon-Wooding-Displayr · 2026-06-14T08:25:08Z

@kevinjqliu I've applied all of your comments above.

Secondarily, I've had a deep scan and we don't seem to log headers anywhere but what I've done is added a new function to client/builder.rs called sensitive_header that does what you are saying and we call that when adding the encryption-key/SHA256. I felt it's better to be safe now rather than expose accidentally later if someone adds header logging.

GCP had a special function to add bearer_auth which I simplified to use this sensitive header function too.

kevinjqliu

I tested this PR using my own azure account against all functions that supports CPK. Everything look good, found a few minor issues.

PR’s CPK copy path authenticates the destination request, but not the private copy source URL.
redact copy source when used
signed_url and signed_urls should return NotSupported when CPK is configured, since this crate’s signer returns only a URL and Azure CPK requires request headers.
Fixed some unit test cases

I was using Codex for review, and it generated this commit locally with the fix:
817f106
Feel free to use any parts of it! Verified that this fixed the copy_request auth issue, I was using az cli for auth

kevinjqliu · 2026-06-14T19:40:36Z

+    /// CPK material lives in request *headers* (`x-ms-encryption-key` etc.),
+    /// not in the URL, so today's URL-only redaction does not actively hide
+    /// it. The flag is still set for CPK requests so that any future
+    /// expansion of the redaction surface (headers, response bodies) covers
+    /// CPK without further changes here, and so that operators have a single
+    /// "this request touches secret material" signal for both auth modes.


Suggested change

/// CPK material lives in request *headers* (`x-ms-encryption-key` etc.),

/// not in the URL, so today's URL-only redaction does not actively hide

/// it. The flag is still set for CPK requests so that any future

/// expansion of the redaction surface (headers, response bodies) covers

/// CPK without further changes here, and so that operators have a single

/// "this request touches secret material" signal for both auth modes.

/// CPK material lives in request *headers* (`x-ms-encryption-key` etc.),

/// not in the URL. Those header values are marked sensitive when added to

/// the request, while this flag ensures retry/error formatting also treats

/// the whole request as sensitive.

update comment based on new improvements using sensitive_header

Braedon-Wooding-Displayr · 2026-06-15T01:01:53Z

@kevinjqliu I've included your commit with just 1 change, I've made it so that COPY_SOURCE is always counted as a sensitive header I felt doing the partial sensitive if it includes query params was not necessary and that it can always be sensitive is just simpler.

Included the other comment fix.

Looks like this is probably ready for merge?

kevinjqliu

LGTM!

one last thing, codex flagged a rebase conflict issue -- looks like we need to rebase and add #[cfg(feature = "reqwest")] to tests due to #750

Braedon-Wooding-Displayr · 2026-06-16T01:16:41Z

@kevinjqliu done! Rebased + added that feature gate for tests.

Feel free to merge!

kevinjqliu · 2026-06-17T04:44:23Z

---- local::tests::test_close_file_detects_error_unix stdout ----

looks like CI failed on something unrelated, might need to rerun this one

kevinjqliu · 2026-06-17T04:45:28Z

@alamb if you could take a look at this, esp the changes in src/client/builder.rs.
I've validated the azure specific changes using my own azure account

alamb

looks good to me -- thank you @Braedon-Wooding-Displayr and @kevinjqliu for the verification

The merged CPK support (apache#742) computed the encryption key SHA-256 via ring::digest directly, which fails to compile on this branch since azure-base no longer depends on ring. Route it through the CryptoProvider abstraction (DigestAlgorithm::Sha256) instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Pluggable Crypto * Add CI * Upgrade reqwest to 0.13 * Certificates * Switch to aws-lc-rs * Fix CI * Fix doc * Remove reqwest/rustls-no-provider feature dependency * Compile with reqwest/rustls feature in CI when not selecting a crypto backend * Hack around WASIp1 CI by downgrading reqwest * Update docs * Improve * Document how to use flags * fixes * Updates * typo * docs * Clarify base feature crypto and HTTP requirements * revert non fips * Improve comments, and fix clippy * Use pluggable crypto API for Azure CPK key digest instead of ring The merged CPK support (#742) computed the encryption key SHA-256 via ring::digest directly, which fails to compile on this branch since azure-base no longer depends on ring. Route it through the CryptoProvider abstraction (DigestAlgorithm::Sha256) instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Raphael Taylor-Davies <r.taylordavies@googlemail.com> Co-authored-by: Adam Gutglick <adam@spiraldb.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> Co-authored-by: Kevin Liu <kevin.jq.liu@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Braedon-Wooding-Displayr mentioned this pull request Jun 9, 2026

Supporting CPKs for Azure Blob Requests #741

Closed

Braedon-Wooding-Displayr force-pushed the support-cpk-in-azure branch from 9fc0317 to 797dd05 Compare June 9, 2026 11:26

Braedon-Wooding-Displayr commented Jun 9, 2026

View reviewed changes

kylebarron reviewed Jun 9, 2026

View reviewed changes

Comment thread src/azure/builder.rs

kevinjqliu reviewed Jun 9, 2026

View reviewed changes

Braedon-Wooding-Displayr force-pushed the support-cpk-in-azure branch 2 times, most recently from a5c182b to 0ded725 Compare June 10, 2026 01:36

Braedon-Wooding-Displayr requested review from kevinjqliu and kylebarron June 10, 2026 01:41

kevinjqliu reviewed Jun 11, 2026

View reviewed changes

alamb reviewed Jun 11, 2026

View reviewed changes

Braedon-Wooding-Displayr force-pushed the support-cpk-in-azure branch 2 times, most recently from c7345e2 to 5f4f174 Compare June 14, 2026 08:23

Braedon-Wooding-Displayr requested a review from kevinjqliu June 14, 2026 08:25

kevinjqliu reviewed Jun 14, 2026

View reviewed changes

Braedon-Wooding-Displayr requested a review from kevinjqliu June 15, 2026 01:01

kevinjqliu approved these changes Jun 15, 2026

View reviewed changes

Braedon-Wooding-Displayr and others added 3 commits June 16, 2026 11:16

Supporting CPK (Customer Provided Keys) in Azure Blob Storage requests

0aa35b1

Fix Azure CPK copy source authorization

e9d01b8

Apply #[cfg(feature = reqwest)] to tests

9ace4c7

Braedon-Wooding-Displayr force-pushed the support-cpk-in-azure branch from 1510fe7 to 9ace4c7 Compare June 16, 2026 01:16

kevinjqliu mentioned this pull request Jun 17, 2026

Release object store 0.14.0 (breaking) - Target April 2026 #657

Closed

18 tasks

This was referenced Jun 17, 2026

feat: add AzureConfigKey::CredentialType to select Azure credential method #710

Merged

Fix flaky local close-file tests #757

Merged

alamb approved these changes Jun 17, 2026

View reviewed changes

alamb merged commit 5dc1c5a into apache:main Jun 17, 2026
13 of 14 checks passed

This was referenced Jun 25, 2026

Tracking upstream updates to 0.14 developmentseed/obstore#730

Closed

feat: Support Azure customer provided keys developmentseed/obstore#735

Merged

	/// Apply the customer-provided encryption headers for the request target.
	/// Apply the customer-provided encryption headers for the request target.
	/// <https://learn.microsoft.com/en-us/azure/storage/blobs/encryption-customer-provided-keys>

+            .retryable(&self.config.retry_config)
+            .sensitive(sensitive)
+            .send()

-    /// Make an Azure copy request <https://docs.microsoft.com/en-us/rest/api/storageservices/copy-blob>.
-    ///
-    /// When customer-provided keys are enabled, this request uses the Put Blob
-    /// From URL header shape and must opt into the service version that added
-    /// source CPK headers.
+    /// Make an Azure copy request <https://docs.microsoft.com/en-us/rest/api/storageservices/copy-blob>.
+    ///
+    /// The classic `Copy Blob` API does not accept CPK headers, so when
+    /// customer-provided keys are enabled this falls back to
+    /// [Put Blob From URL][put-blob-from-url] and opts into the service version
+    /// that added source CPK headers. That changes the semantics: the operation
+    /// is synchronous, the source must be a block blob no larger than 5,000 MiB,
+    /// and uncommitted blocks / the source block list are not preserved.
+    ///
+    /// [put-blob-from-url]: https://learn.microsoft.com/en-us/rest/api/storageservices/put-blob-from-url

Uh oh!

Conversation

Braedon-Wooding-Displayr commented Jun 9, 2026 • edited by alamb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Braedon-Wooding-Displayr commented Jun 10, 2026

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

kevinjqliu commented Jun 12, 2026

Uh oh!

Braedon-Wooding-Displayr commented Jun 14, 2026

Uh oh!

kevinjqliu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Braedon-Wooding-Displayr commented Jun 9, 2026 •

edited by alamb

Loading

kevinjqliu left a comment •

edited

Loading

kevinjqliu Jun 14, 2026 •

edited

Loading

Braedon-Wooding-Displayr commented Jun 16, 2026 •

edited

Loading