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

Support extracting the http scheme from the requests #2278

Open
wants to merge 3 commits into
base: development/7.10
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion lib/policyEvaluator/requestUtils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import * as ipCheck from '../ipCheck'
import { IncomingMessage } from 'http'
import { TLSSocket } from 'tls'

export interface S3Config {
requests: {
trustedProxyCIDRs: string[],
extractClientIPFromHeader: string
extractClientIPFromHeader: string,
extractProtocolFromHeader: string,
}
}

Expand Down Expand Up @@ -37,3 +39,20 @@ export function getClientIp(request: IncomingMessage, s3config?: S3Config): stri
}
return clientIp;
}

/**
* getHttpProtocolSecurity - Dete²object
* @param s3config - s3 config
* @return {boolean} - returns true if the request is secure
*/
export function getHttpProtocolSecurity(request: IncomingMessage, s3config?: S3Config): boolean {
const requestConfig = s3config?.requests;
if (requestConfig) {
const { trustedProxyCIDRs } = requestConfig;
const clientIp = request.socket.remoteAddress?.toString() ?? '';
if (ipCheck.ipMatchCidrList(trustedProxyCIDRs, clientIp)) {
return request.headers[requestConfig.extractProtocolFromHeader.toLowerCase()] === 'https';
francoisferrand marked this conversation as resolved.
Show resolved Hide resolved
}
}
return request.socket instanceof TLSSocket && request.socket.encrypted;
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engines": {
"node": ">=16"
},
"version": "7.10.64",
"version": "7.10.65",
"description": "Common utilities for the S3 project components",
"main": "build/index.js",
"repository": {
Expand Down
79 changes: 79 additions & 0 deletions tests/unit/policyEvaluator/requestUtils.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const assert = require('assert');
const DummyRequest = require('../../utils/DummyRequest');
const requestUtils = require('../../../lib/policyEvaluator/requestUtils');
const { TLSSocket } = require('tls');

describe('requestUtils.getClientIp', () => {
// s3 config with 'requests.viaProxy` enabled
Expand Down Expand Up @@ -110,3 +111,81 @@ describe('requestUtils.getClientIp', () => {
assert.strictEqual(result, dummyRemoteIP);
});
});

describe('requestUtils.getHttpProtocolSecurity', () => {
const configWithProxy = require('../../utils/dummyS3ConfigProxy.json');
const configWithoutProxy = require('../../utils/dummyS3Config.json');
const testClientIp = '192.168.100.1';
const testProxyIp = '192.168.100.2';

it('should return true if request comes via trusted proxy with https proto header', () => {
const request = new DummyRequest({
headers: {
'x-forwarded-proto': 'https',
},
socket: {
remoteAddress: testProxyIp,
},
});
const result = requestUtils.getHttpProtocolSecurity(request, configWithProxy);
assert.strictEqual(result, true);
});

it('should return false if request comes via trusted proxy with http proto header', () => {
const request = new DummyRequest({
headers: {
'x-forwarded-proto': 'http',
},
socket: {
remoteAddress: testProxyIp,
},
});
const result = requestUtils.getHttpProtocolSecurity(request, configWithProxy);
assert.strictEqual(result, false);
});

it('should check TLS when request not from trusted proxy with http', () => {
const request = new DummyRequest({
headers: {
'x-forwarded-proto': 'http',
},
socket: new TLSSocket(null),
});
request.socket.encrypted = true;
const result = requestUtils.getHttpProtocolSecurity(request, configWithoutProxy);
assert.strictEqual(result, true);
});

it('should return false for non-TLS socket', () => {
const request = new DummyRequest({
headers: {
'x-forwarded-proto': 'https',
},
socket: {
remoteAddress: testClientIp,
},
});
const result = requestUtils.getHttpProtocolSecurity(request, configWithoutProxy);
assert.strictEqual(result, false);
});

it('should handle configured headers with uppercases', () => {
const request = new DummyRequest({
headers: {
'x-forwarded-proto': 'https',
},
socket: {
remoteAddress: testProxyIp,
},
});
const result = requestUtils.getHttpProtocolSecurity(request, {
requests: {
viaProxy: true,
trustedProxyCIDRs: ['192.168.100.0/22'],
extractClientIPFromHeader: 'X-Forwarded-For',
extractProtocolFromHeader: 'X-Forwarded-Proto',
},
});
assert.strictEqual(result, true);
});
});
3 changes: 2 additions & 1 deletion tests/utils/dummyS3ConfigProxy.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"requests": {
"viaProxy": true,
"trustedProxyCIDRs": ["192.168.100.0/22"],
"extractClientIPFromHeader": "x-forwarded-for"
"extractClientIPFromHeader": "x-forwarded-for",
"extractProtocolFromHeader": "x-forwarded-proto"
}
}
Loading