Skip to content

Commit

Permalink
Parse the Forwarded header, for secure cookies WIN!
Browse files Browse the repository at this point in the history
  • Loading branch information
chriswilty committed Feb 13, 2024
1 parent c851851 commit 43f1d53
Show file tree
Hide file tree
Showing 11 changed files with 168 additions and 105 deletions.
4 changes: 2 additions & 2 deletions backend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ WORKDIR /usr/app
COPY package*.json ./
RUN npm ci
COPY . .
EXPOSE 3001
#EXPOSE 3001
RUN npm run build
CMD ["node", "--import=tsx", "./src/server.ts"]
CMD ["node", "--import=tsx", "./src/app/server.ts"]
23 changes: 14 additions & 9 deletions backend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"type": "module",
"scripts": {
"build": "tsc --noEmit",
"dev": "tsx watch -r dotenv/config src/server.ts",
"start": "tsx -r dotenv/config src/server.ts",
"dev": "tsx watch -r dotenv/config src/app/server.ts",
"start": "tsx -r dotenv/config src/app/server.ts",
"docker:start": "docker compose up -d",
"docker:logs": "docker compose logs -f",
"docker:stop": "docker compose down",
Expand All @@ -18,7 +18,7 @@
"d3-dsv": "^2.0.0",
"dotenv": "^16.3.1",
"express": "^4.18.2",
"express-session": "^1.17.3",
"express-session": "^1.18.0",
"langchain": "^0.0.113",
"memorystore": "^1.6.7",
"openai": "^4.19.0",
Expand Down
3 changes: 2 additions & 1 deletion backend/src/app.ts → backend/src/app/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import cors from 'cors';
import express from 'express';

import nonSessionRoutes from './nonSessionRoutes';
import { usingForwardedHeader } from './proxySetup';
import sessionRoutes from './sessionRoutes';

export default express()
export default usingForwardedHeader(express())
.use(express.json())
.use(
cors({
Expand Down
18 changes: 18 additions & 0 deletions backend/src/app/nonSessionRoutes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import express from 'express';
import { fileURLToPath } from 'node:url';

import { handleGetDocuments } from '../controller/documentController';
import { handleHealthCheck } from '../controller/healthController';
import { handleGetSystemRoles } from '../controller/systemRoleController';
import { importMetaUrl } from '../importMetaUtils';

export default express.Router()
.use(
'/documents',
express.static(
fileURLToPath(new URL('../resources/documents', importMetaUrl()))
)
)
.get('/documents', handleGetDocuments)
.get('/health', handleHealthCheck)
.get('/systemRoles', handleGetSystemRoles);
48 changes: 48 additions & 0 deletions backend/src/app/proxySetup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { Express, Request } from 'express';

/**
* Unfortunately, "trust proxy" is by default broken when Express is behind an
* AWS HTTP API Gateway:
* - https://github.com/expressjs/express/issues/5459
* - https://repost.aws/en/questions/QUtBHMaz7IQ6aM4RCBMnJvgw/why-does-apigw-http-api-use-forwarded-header-while-other-services-still-use-x-forwarded-headers
*
* Therefore we use Express API overrides to modify our Request IP and Protocol properties:
* - https://expressjs.com/en/guide/overriding-express-api.html
*/
export function usingForwardedHeader(app: Express): Express {
Object.defineProperties(app.request, {
'ip': {
configurable: true,
enumerable: true,
get() {
const proxies = parseForwardedHeader(this as Request);
return proxies?.['for']?.[0] ?? this.socket.remoteAddress;
},
},
'protocol': {
configurable: true,
enumerable: true,
get() {
const proxies = parseForwardedHeader(this as Request);
return proxies?.['proto']?.[0] ?? this.socket.encrypted ? 'https' : 'http';
},
},
});
return app;
}

function parseForwardedHeader(request: Request) {
return request.header('Forwarded')
?.split(",")
.flatMap((proxy) => proxy.split(';'))
.reduce(
(result, proxyProps) => {
const [key, value] = proxyProps.split('=');
if (key && value) {
result[key] = (result[key] || []).concat(value);
}
return result;
},
{} as Record<string, string[]>
);
}
4 changes: 2 additions & 2 deletions backend/src/server.ts → backend/src/app/server.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { env, exit } from 'node:process';

import app from './app';
import { initDocumentVectors } from './document';
import { getValidModelsFromOpenAI } from './openai';
import { initDocumentVectors } from '../document';
import { getValidModelsFromOpenAI } from '../openai';
// by default runs on port 3001
const port = env.PORT ?? String(3001);

Expand Down
104 changes: 48 additions & 56 deletions backend/src/sessionRoutes.ts → backend/src/app/sessionRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,38 @@ import {
handleGetChatHistory,
handleAddInfoToChatHistory,
handleClearChatHistory,
} from './controller/chatController';
} from '../controller/chatController';
import {
handleConfigureDefence,
handleDefenceActivation,
handleDefenceDeactivation,
handleGetDefenceStatus,
handleResetSingleDefence,
} from './controller/defenceController';
} from '../controller/defenceController';
import {
handleClearEmails,
handleGetEmails,
} from './controller/emailController';
} from '../controller/emailController';
import {
handleConfigureModel,
handleGetModel,
handleGetValidModels,
handleSetModel,
} from './controller/modelController';
import { handleResetProgress } from './controller/resetController';
import { ChatModel, defaultChatModel } from './models/chat';
import { LevelState, getInitialLevelStates } from './models/level';
} from '../controller/modelController';
import { handleResetProgress } from '../controller/resetController';
import { ChatModel, defaultChatModel } from '../models/chat';
import { LevelState, getInitialLevelStates } from '../models/level';

declare module 'express-session' {
interface Session {
initialised: boolean;
chatModel: ChatModel;
levelState: LevelState[];
}
interface CookieOptions {
// TODO express-session types are not yet up-to-date!
partitioned?: boolean;
}
}

const sessionSigningSecret = process.env.SESSION_SECRET;
Expand All @@ -46,52 +50,51 @@ if (!sessionSigningSecret) {
process.exit(1);
}

const router = express.Router();

const stage = process.env.NODE_ENV;
console.log(`env=${stage}`);
const isProd = stage === 'production';
const cookieStaleHours = isProd ? 2 : 8;
const oneHourInMillis = 60 * 60 * 1000;
const maxAge = oneHourInMillis * cookieStaleHours;

router.use(
session({
name: 'prompt-injection.sid',
resave: false,
saveUninitialized: true,
secret: sessionSigningSecret,
// Session storage: currently in-memory but could use Redis in AWS
store: new (memoryStoreFactory(session))({
checkPeriod: oneHourInMillis,
}),
proxy: isProd,
cookie: {
maxAge,
/*
https://developer.mozilla.org/en-US/blog/goodbye-third-party-cookies/
Now that browsers have begun clamping down on non-secure Cookies, we
need to set secure=true in prod, until we can put Route53 in front of both
UI and API and get rid of APIGateway entirely. The showstopper is that
APIGateway is not adding Forwarded headers correctly, so the (secure)
session Cookie is no longer working in Prod.
See
https://repost.aws/questions/QUtBHMaz7IQ6aM4RCBMnJvgw/why-does-apigw-http-api-use-forwarded-header-while-other-services-still-use-x-forwarded-headers
*/
sameSite: isProd ? 'none' : 'strict',
secure: isProd,
},
})
);
const router = express.Router()
.use(
session({
name: 'prompt-injection.sid',
resave: false,
saveUninitialized: true,
secret: sessionSigningSecret,
// Session storage: currently in-memory but could use Redis in AWS
store: new (memoryStoreFactory(session))({
checkPeriod: oneHourInMillis,
}),
cookie: {
maxAge,
partitioned: isProd,
sameSite: isProd ? 'none' : 'strict',
secure: isProd,
},
})
)
.use((req, _res, next) => {
if (!req.session.initialised) {
req.session.chatModel = defaultChatModel;
req.session.levelState = getInitialLevelStates();
req.session.initialised = true;
}
next();
});

router.use((req, _res, next) => {
if (!req.session.initialised) {
req.session.chatModel = defaultChatModel;
req.session.levelState = getInitialLevelStates();
req.session.initialised = true;
}
next();
});
// TODO: Remove this debug logging!
if (isProd) {
router.use('/openai', (req, res, next) => {
console.log('Request:', req.ip, req.path, `secure=${req.secure}`, req.headers);
res.on('finish', () => {
console.log('Response:', req.path, res.getHeaders());
});
next();
});
}

// defences
router.get('/defence/status', handleGetDefenceStatus);
Expand Down Expand Up @@ -119,15 +122,4 @@ router.post('/openai/model/configure', handleConfigureModel);
// reset progress for all levels
router.post('/reset', handleResetProgress);

// Debugging: log headers in prod for primary routes
if (isProd) {
router.use('/openai', (req, res, next) => {
console.log('Request:', req.path, `secure=${req.secure}`, req.headers);
res.on('finish', () => {
console.log('Response:', req.path, res.getHeaders());
});
next();
});
}

export default router;
21 changes: 0 additions & 21 deletions backend/src/nonSessionRoutes.ts

This file was deleted.

10 changes: 5 additions & 5 deletions cloud/cdk.context.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"availability-zones:account=600982866784:region=eu-west-1": [
"eu-west-1a",
"eu-west-1b",
"eu-west-1c"
]
"availability-zones:account=600982866784:region=eu-west-1": [
"eu-west-1a",
"eu-west-1b",
"eu-west-1c"
]
}
Loading

0 comments on commit 43f1d53

Please sign in to comment.