Skip to content

feat(proxy): allowlist AWS_SIGN_ACCEPT_ENCODING - #1615

Open
kinorai wants to merge 2 commits into
longhorn:masterfrom
kinorai:feat/allowlist-sign-accept-encoding
Open

feat(proxy): allowlist AWS_SIGN_ACCEPT_ENCODING#1615
kinorai wants to merge 2 commits into
longhorn:masterfrom
kinorai:feat/allowlist-sign-accept-encoding

Conversation

@kinorai

@kinorai kinorai commented Aug 16, 2026

Copy link
Copy Markdown

Which issue(s) this PR fixes:

Issue longhorn/longhorn#13756

What this PR does / why we need it:

backupstore gained AWS_SIGN_ACCEPT_ENCODING (longhorn/backupstore#316), which
excludes Accept-Encoding from the SigV4 SignedHeaders set for backup targets
reached through a proxy that alters the header in transit.

longhorn-manager sends the key through getBackupCredentialEnv
(longhorn/longhorn-manager#5100), which appends every S3 credential entry
unconditionally, whether or not the secret sets it. validateBackupEnv rejects the
whole request on a single key outside backupEnvAllowlist, so without this entry
every S3 backup and restore fails with:

env key "AWS_SIGN_ACCEPT_ENCODING" is not permitted by the backup env allowlist

This adds the key to backupEnvAllowlist, and to the allowlist enumerated by
TestSetEnv_AcceptsAllowlistedCredentials.

Special notes for your reviewer:

Merge order matters. This must ship before or with the longhorn-manager change.
The other direction breaks every S3 backup, not only the backups of users who set the
key.

On the security boundary. AWS_SIGN_ACCEPT_ENCODING selects a signing option. It
carries no credential and it is not a loader variable, so it does not widen the
exposure GHSA-wgh7-5vxp-4qr4 closed. Its value is consumed only by
strconv.ParseBool in backupstore, and any non-boolean value is rejected there.

Dependency. btypes.AWSSignAcceptEncoding needs the backupstore vendor bump, so
this does not compile until longhorn/backupstore#316 merges.

Additional documentation or context

I could not build ./pkg/proxy locally, because the vendored
longhorn-engine/pkg/qcow needs libqcow.h, which the Dockerfile builds from source.
Instead I type-checked env_allowlist.go against a copy of the post-bump
backupstore/types, and ran validateBackupEnv against it directly:
AWS_SIGN_ACCEPT_ENCODING=false is accepted, and LD_PRELOAD=/evil.so is still
rejected. The same check against current master fails, which is what this PR fixes.


Prepared with AI assistance (Claude Code). Every test and command reported above was run, not estimated.

backupstore gained AWS_SIGN_ACCEPT_ENCODING, which excludes Accept-Encoding
from the SigV4 SignedHeaders set for backup targets reached through a proxy
that alters the header in transit.

longhorn-manager sends the key through getBackupCredentialEnv, which appends
every S3 credential entry unconditionally, whether or not the secret sets it.
validateBackupEnv rejects the whole request on a single key outside
backupEnvAllowlist, so without this entry every S3 backup and restore fails
with:
"env key \"AWS_SIGN_ACCEPT_ENCODING\" is not permitted by the backup env
allowlist"

Add the key to backupEnvAllowlist. It selects a signing option rather than
carrying a credential, and it is not a loader variable, so it does not widen
the exposure GHSA-wgh7-5vxp-4qr4 closed.

This must ship before or with the longhorn-manager change that sends the key.

Longhorn 13756

Signed-off-by: kinorai <kinorai@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


// Selects whether Accept-Encoding is part of the SigV4 signature. It is a
// signing option, not a credential, and carries no secret.
btypes.AWSSignAcceptEncoding: {},

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't compile when I opened this. backupstore#316 merged as 12a1fc975d8f, so I bumped and re-vendored in 210d947.

make validate and the test stage both pass in the container, pkg/proxy included. Diff is go.mod, go.sum, vendor/modules.txt and the three backupstore files.

longhorn/backupstore#316 merged as 12a1fc975d8f and defines
types.AWSSignAcceptEncoding, which pkg/proxy/env_allowlist.go references.
Bump and re-vendor so the package compiles.

Longhorn 13756

Signed-off-by: kinorai <kinorai@users.noreply.github.com>
@kinorai
kinorai force-pushed the feat/allowlist-sign-accept-encoding branch from bfc83ed to 210d947 Compare August 20, 2026 13:57

@derekbit derekbit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@derekbit

Copy link
Copy Markdown
Member

@mergify backport v1.12.x

@mergify

mergify Bot commented Aug 20, 2026

Copy link
Copy Markdown

backport v1.12.x

🟠 Waiting for conditions to match

Details
  • merged [📌 backport requirement]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants