Skip to content
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

Add new ENV config S3_SERVICE_REMOVE_X_AMZ_HEADERS #241

Merged
merged 10 commits into from
May 2, 2024
34 changes: 32 additions & 2 deletions common/etc/nginx/include/s3gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ const S3_STYLE = process.env['S3_STYLE'];
* @type {Array<String>}
* */
const ADDITIONAL_HEADER_PREFIXES_TO_STRIP = utils.parseArray(process.env['HEADER_PREFIXES_TO_STRIP']);

/**
* Additional header prefixes to allow from the response before sending to the
* client. This is opposite to HEADER_PREFIXES_TO_STRIP.
* @type {Array<String>}
* */
const ADDITIONAL_HEADER_PREFIXES_ALLOWED = utils.parseArray(process.env['HEADER_PREFIXES_ALLOWED']);

/**
* Default filename for index pages to be read off of the backing object store.
* @type {string}
Expand All @@ -100,15 +108,19 @@ function editHeaders(r) {
r.method === 'HEAD' &&
_isDirectory(decodeURIComponent(r.variables.uri_path));

/* Strips all x-amz- headers from the output HTTP headers so that the
/* Strips all x-amz- (if x-amz- is not in ADDITIONAL_HEADER_PREFIXES_ALLOWED) headers from the output HTTP headers so that the
* requesters to the gateway will not know you are proxying S3. */
if ('headersOut' in r) {
for (const key in r.headersOut) {
const headerName = key.toLowerCase()
/* We delete all headers when it is a directory head request because
* none of the information is relevant for passing on via a gateway. */
if (isDirectoryHeadRequest) {
delete r.headersOut[key];
} else if (_isHeaderToBeStripped(key.toLowerCase(), ADDITIONAL_HEADER_PREFIXES_TO_STRIP)) {
} else if (
!_isHeaderToBeAllowed(headerName, ADDITIONAL_HEADER_PREFIXES_ALLOWED)
&& _isHeaderToBeStripped(headerName, ADDITIONAL_HEADER_PREFIXES_TO_STRIP)
gawsoftpl marked this conversation as resolved.
Show resolved Hide resolved
) {
delete r.headersOut[key];
}
}
Expand Down Expand Up @@ -145,6 +157,24 @@ function _isHeaderToBeStripped(headerName, additionalHeadersToStrip) {
return false;
}

/**
* Determines if a given HTTP header should be force allowed from requesting client.
* @param headerName {string} Lowercase HTTP header name
* @param additionalHeadersToAllow {Array<string>} array of additional headers to allow
* @returns {boolean} true if header should be removed
*/
function _isHeaderToBeAllowed(headerName, additionalHeadersToAllow) {

for (let i = 0; i < additionalHeadersToAllow.length; i++) {
const headerToAllow = additionalHeadersToAllow[i];
if (headerName.indexOf(headerToAllow, 0) >= 0) {
return true;
}
}

return false;
}

/**
* Outputs the timestamp used to sign the request, so that it can be added to
* the 'Date' header and sent by NGINX.
Expand Down
5 changes: 4 additions & 1 deletion docs/getting_started.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,14 @@ running as a Container or as a Systemd service.
| `PROXY_CACHE_VALID_FORBIDDEN` | No | | `30s` | Sets caching time for response code 403 |
| `PROVIDE_INDEX_PAGE` | No | `true`, `false` | `false` | Flag which returns the index page if there is one when requesting a directory. |
| `JS_TRUSTED_CERT_PATH` | No | | | Enables the `js_fetch_trusted_certificate` directive when retrieving AWS credentials and sets the path (on the container) to the specified path |
| `HEADER_PREFIXES_TO_STRIP` | No | | | A list of HTTP header prefixes that exclude headers client responses. List should be specified in lower-case and a semicolon (;) should be used to as a deliminator between values. For example: `x-goog-;x-something-` |
| `HEADER_PREFIXES_TO_STRIP` | No | | | A list of HTTP header prefixes that exclude headers from client responses. List should be specified in lower-case and a semicolon (;) should be used to as a deliminator between values. For example: x-goog-;x-something-. Headers starting with x-amz- will be stripped by default for security reasons unless explicitly added in HEADER_PREFIXES_ALLOWED. |
| `HEADER_PREFIXES_ALLOWED` | No | | | A list of allowed prefixes for HTTP headers that are returned to the client in responses. List should be specified in lower-case and a semicolon (;) should be used to as a deliminator between values. For example: x-amz-;x-something-. It is NOT recommended to return x-amz- headers for security reasons. Think carefully about what is allowed here. |
| `CORS_ENABLED` | No | `true`, `false` | `false` | Flag that enables CORS headers on GET requests and enables pre-flight OPTIONS requests. If enabled, this will add CORS headers for "fully open" cross domain requests by default, meaning all domains are allowed, similar to the settings show in [this example](https://enable-cors.org/server_nginx.html). CORS settings can be fine-tuned by overwriting the [`cors.conf.template`](/common/etc/nginx/templates/gateway/cors.conf.template) file. |
| `CORS_ALLOWED_ORIGIN` | No | | | value to set to be returned from the CORS `Access-Control-Allow-Origin` header. This value is only used if CORS is enabled. (default: \*) |
| `STRIP_LEADING_DIRECTORY_PATH` | No | | | Removes a portion of the path in the requested URL (if configured). Useful when deploying to an ALB under a folder (eg. www.mysite.com/somepath). |
| `PREFIX_LEADING_DIRECTORY_PATH` | No | | | Prefix to prepend to all S3 object paths. Useful to serve only a subset of an S3 bucket. When used in combination with `STRIP_LEADING_DIRECTORY_PATH`, this allows the leading path to be replaced, rather than just removed. |
|


If you are using [AWS instance profile credentials](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2.html),
you will need to omit the `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY` and `AWS_SESSION_TOKEN` variables from
Expand Down
54 changes: 53 additions & 1 deletion test/unit/s3gateway_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,54 @@ function testEditHeaders() {
}

s3gateway.editHeaders(r);

for (const key in r.headersOut) {
if (key.toLowerCase().indexOf("x-amz", 0) >= 0) {
throw "x-amz header not stripped from headers correctly";
}
}
}

function testEditHeadersWithAllowedPrefixes() {
printHeader('testEditHeadersWithAllowedPrefixes');

process.env['HEADER_PREFIXES_ALLOWED'] = 'x-amz-'
const r = {
"headersOut": {
"Accept-Ranges": "bytes",
"Content-Length": 42,
"Content-Security-Policy": "block-all-mixed-content",
"Content-Type": "text/plain",
"X-Amz-Bucket-Region": "us-east-1",
"X-Amz-Request-Id": "166539E18A46500A",
"X-Xss-Protection": "1; mode=block"
},
"variables": {
"uri_path": "/a/c/ramen.jpg"
},
}

r.log = function(msg) {
console.log(msg);
}

s3gateway.editHeaders(r);

let found_headers_x_amz_ = 0
for (const key in r.headersOut) {
if (key.toLowerCase().indexOf("x-amz", 0) == 0) {
found_headers_x_amz_++;
}
}

if (found_headers_x_amz_ != 2)
throw "x-amz header stripped from headers, should allow those 2 headers";

delete process.env['HEADER_PREFIXES_ALLOWED']

}


function testEditHeadersHeadDirectory() {
printHeader('testEditHeadersHeadDirectory');
let r = {
Expand Down Expand Up @@ -192,6 +232,18 @@ function testIsHeaderToBeStripped() {
}
}

function testIsHeaderToBeAllowed() {
printHeader('testIsHeaderToBeAllowed');

if (!s3gateway._isHeaderToBeAllowed('x-amz-abc', ['x-amz-'])) {
throw "x-amz-abc header should be allowed";
}

if (s3gateway._isHeaderToBeAllowed('x-amz-xyz',['x-amz-abc'])) {
throw "x-amz-xyz header should be stripped";
}
}

function testEscapeURIPathPreservesDoubleSlashes() {
printHeader('testEscapeURIPathPreservesDoubleSlashes');
var doubleSlashed = '/testbucketer2/foo3//bar3/somedir/license';
Expand Down
Loading