Skip to content

Commit

Permalink
feat: Add new ENV config HEADER_PREFIXES_ALLOWED (#241)
Browse files Browse the repository at this point in the history
Adds the ability to create a list of header prefixes that are allowed to be returned to the client using the new `HEADER_PREFIXES_ALLOWED` configuration option which is a companion configuration option to `HEADER_PREFIXES_TO_STRIP`.

If specified, this configuration value will take precedence over the default stripped header prefixes (`x-amz-`) as well as and prefixes specified in `HEADER_PREFIXES_TO_STRIP`.

Note that it is generally not advised to return any object store specific headers to the client and the default remains to enforce stripping of these headers.  Use this configuration option with caution.
  • Loading branch information
gawsoftpl authored May 2, 2024
1 parent de13fab commit f304994
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 4 deletions.
1 change: 1 addition & 0 deletions common/docker-entrypoint.d/00-check-for-required-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,5 @@ echo "Directory Listing Path Prefix: ${DIRECTORY_LISTING_PATH_PREFIX}"
echo "Provide Index Pages Enabled: ${PROVIDE_INDEX_PAGE}"
echo "Append slash for directory enabled: ${APPEND_SLASH_FOR_POSSIBLE_DIRECTORY}"
echo "Stripping the following headers from responses: x-amz-;${HEADER_PREFIXES_TO_STRIP}"
echo "Allow the following headers from responses (these take precendence over the above): ${HEADER_PREFIXES_ALLOWED}"
echo "CORS Enabled: ${CORS_ENABLED}"
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)
) {
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

0 comments on commit f304994

Please sign in to comment.