Skip to content

Commit

Permalink
Merge branch 'w/8.1/bugfix/ARSN-456' into tmp/octopus/w/8.2/bugfix/AR…
Browse files Browse the repository at this point in the history
  • Loading branch information
bert-e committed Jan 10, 2025
2 parents 81ee00a + 05dbb67 commit 07efddd
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 3 deletions.
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';
}
}
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": "8.1.144",
"version": "8.1.145",
"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"
}
}

0 comments on commit 07efddd

Please sign in to comment.