security: fix 7 vulnerabilities from Opus code review
- [A01] /export: mount after auth.middleware, filter by company for non-admin - [A02] OIDC: verify issuer, audience, select key by kid; preload JWKS at startup - [A07] session fixation: session.regenerate() in POST /login, GET /callback, POST /dev-login - [A10] open redirect: safeReturn() helper validates returnTo (blocks //evil.com, https:// etc) - [A05] CSRF: getSessionIdentifier uses req.sessionID instead of dead req.cookies.jwt - [A05] fail-fast: reject default SESSION_SECRET / CSRF_SECRET in NODE_ENV=production - [A05] DEV_MODE=true blocked in production Tests: 68 PASS, 0 FAIL (was 50, added 18 new security-focused tests)
This commit is contained in:
@@ -75,20 +75,28 @@ async function start() {
|
||||
|
||||
// ── Публичные маршруты (до auth.middleware) ──────────────────────────────────
|
||||
|
||||
// ── Fail-fast в production ────────────────────────────────────────────────
|
||||
// Если секреты не заданы — приложение в проде немедленно падает.
|
||||
// Дефолтные значения — только для локальной разработки.
|
||||
if (process.env.NODE_ENV === 'production') {
|
||||
const DEFAULT_SESSION = 'dev-session-secret-change-me';
|
||||
const DEFAULT_CSRF = 'dev-csrf-secret-change-in-prod';
|
||||
if (!process.env.SESSION_SECRET || process.env.SESSION_SECRET === DEFAULT_SESSION) {
|
||||
console.error('FATAL: SESSION_SECRET не задан или равен дефолту в production!');
|
||||
process.exit(1);
|
||||
}
|
||||
if (!process.env.CSRF_SECRET || process.env.CSRF_SECRET === DEFAULT_CSRF) {
|
||||
console.error('FATAL: CSRF_SECRET не задан или равен дефолту в production!');
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
// k8s liveness probe — простая проверка что сервер жив
|
||||
app.get('/healthz', (req, res) => res.send('OK'));
|
||||
|
||||
// JWKS endpoint для валидации токенов внешними сервисами (только в mock-режиме)
|
||||
if (auth.jwksHandler) app.get('/.well-known/jwks.json', auth.jwksHandler);
|
||||
|
||||
// GET /export: агрегированный список CIDR, без авторизации.
|
||||
// src/routes/export.js содержит логику и комментарии.
|
||||
app.use(require('./src/routes/export').createRouter({
|
||||
q,
|
||||
exportLimiter,
|
||||
aggregateCIDRs,
|
||||
}));
|
||||
|
||||
// GET /login, POST /login, GET /logout.
|
||||
// src/routes/auth.js содержит логику и комментарии.
|
||||
app.use(require('./src/routes/auth').createRouter({
|
||||
@@ -98,9 +106,18 @@ async function start() {
|
||||
MOCK_USERS,
|
||||
}));
|
||||
|
||||
// ── Все маршруты ниже требуют валидного JWT (проверяет auth.middleware) ───────
|
||||
// ── Все маршруты ниже требуют авторизации (auth.middleware) ────────────────
|
||||
app.use(auth.middleware);
|
||||
|
||||
// GET /export: CIDR текущего пользователя (или всех компаний для admin).
|
||||
// Перемещён за auth.middleware — изоляция данных по компании.
|
||||
// src/routes/export.js содержит логику и комментарии.
|
||||
app.use(require('./src/routes/export').createRouter({
|
||||
q,
|
||||
exportLimiter,
|
||||
aggregateCIDRs,
|
||||
}));
|
||||
|
||||
// GET /, POST /add, POST /edit/:id, POST /delete/:id.
|
||||
// src/routes/entries.js содержит логику и комментарии.
|
||||
app.use(require('./src/routes/entries').createRouter({
|
||||
|
||||
+39
-4
@@ -53,16 +53,29 @@ let mockJwks = null;
|
||||
let cachedJwks = null; // OIDC public keys
|
||||
|
||||
// ── initAuth ────────────────────────────────────────────────────────────────
|
||||
function initAuth() {
|
||||
async function initAuth() {
|
||||
// Читаем здесь — не на уровне модуля, чтобы тесты могли менять process.env
|
||||
// до вызова initAuth() и видеть актуальные значения.
|
||||
const devMode = process.env.DEV_MODE === 'true';
|
||||
const devSecret = process.env.DEV_SECRET || '';
|
||||
|
||||
// DEV_MODE=true недопустим в production — жёсткая защита от случайного деплоя.
|
||||
if (process.env.NODE_ENV === 'production' && devMode) {
|
||||
throw new Error('DEV_MODE=true запрещён в production (NODE_ENV=production)');
|
||||
}
|
||||
|
||||
const isOidc = !!(KC_CLIENT_ID && KC_CLIENT_SECRET);
|
||||
|
||||
if (isOidc) {
|
||||
console.log('[auth] OIDC — Keycloak', KC_BASE_URL);
|
||||
// Предзагрузка JWKS при старте — избегаем задержки на первом запросе.
|
||||
// При ошибке логируем предупреждение: JWKS будет загружен в exchangeCode().
|
||||
try {
|
||||
cachedJwks = JSON.parse(await httpGet(OIDC.certs));
|
||||
console.log('[auth] JWKS loaded:', cachedJwks.keys.length, 'ключей');
|
||||
} catch (e) {
|
||||
console.warn('[auth] JWKS preload failed:', e.message, '— будет загружен при первом запросе');
|
||||
}
|
||||
} else {
|
||||
console.log('[auth] MOCK — local RS256 key');
|
||||
mockKeyPair = crypto.generateKeyPairSync('rsa', {
|
||||
@@ -165,6 +178,7 @@ async function exchangeCode(code) {
|
||||
const data = JSON.parse(raw);
|
||||
if (!data.access_token) throw new Error('No access_token: ' + raw.slice(0, 200));
|
||||
|
||||
// cachedJwks загружается в initAuth(); при провале preload — подгружаем здесь.
|
||||
if (!cachedJwks) cachedJwks = JSON.parse(await httpGet(OIDC.certs));
|
||||
const payload = verifyOidcToken(data.access_token);
|
||||
|
||||
@@ -200,10 +214,31 @@ function verifyMockToken(token) {
|
||||
|
||||
function verifyOidcToken(token) {
|
||||
if (!cachedJwks) throw new Error('JWKS not loaded yet');
|
||||
const key = cachedJwks.keys.find(k => k.kty === 'RSA' && k.use === 'sig');
|
||||
if (!key) throw new Error('No RSA sig key in JWKS');
|
||||
|
||||
// Декодируем заголовок токена для выбора ключа по kid.
|
||||
// base64url безопаснее base64: обрабатывает '-' и '_' без паддинга.
|
||||
const headerRaw = token.split('.')[0];
|
||||
let header;
|
||||
try {
|
||||
header = JSON.parse(Buffer.from(headerRaw, 'base64url').toString());
|
||||
} catch (e) {
|
||||
throw new Error('Malformed JWT header');
|
||||
}
|
||||
const kid = header.kid;
|
||||
|
||||
// Ищем ключ по kid (точный match) или первый RSA-sig ключ (если kid не указан).
|
||||
const key = kid
|
||||
? cachedJwks.keys.find(k => k.kid === kid && k.kty === 'RSA' && k.use === 'sig')
|
||||
: cachedJwks.keys.find(k => k.kty === 'RSA' && k.use === 'sig');
|
||||
if (!key) throw new Error('No matching RSA sig key in JWKS' + (kid ? ` (kid=${kid})` : ''));
|
||||
|
||||
const pem = crypto.createPublicKey({ key, format: 'jwk' }).export({ type: 'spki', format: 'pem' });
|
||||
return jwt.verify(token, pem, { algorithms: ['RS256'] });
|
||||
// Проверяем issuer и audience — защита от токенов других realm/клиентов (A02).
|
||||
return jwt.verify(token, pem, {
|
||||
algorithms: ['RS256'],
|
||||
issuer: KC_BASE_URL,
|
||||
audience: KC_CLIENT_ID,
|
||||
});
|
||||
}
|
||||
|
||||
function userFromPayload(payload) {
|
||||
|
||||
@@ -37,10 +37,10 @@ function initCsrf() {
|
||||
// ⚠️ В продакшене задать CSRF_SECRET в .env или через k8s Secret.
|
||||
getSecret: () => process.env.CSRF_SECRET || 'dev-csrf-secret-change-in-prod',
|
||||
|
||||
// Идентификатор сессии — привязывает CSRF-токен к конкретному пользователю.
|
||||
// Используем JWT-cookie: если его нет (не залогинен) — пустая строка,
|
||||
// что допустимо для формы /login (она сама создаёт сессию).
|
||||
getSessionIdentifier: (req) => req.cookies.jwt || '',
|
||||
// Идентификатор сессии — привязывает CSRF-токен к конкретной сессии пользователя.
|
||||
// Используем req.sessionID (выставляется express-session до вызова CSRF middleware).
|
||||
// Если сессии нет — пустая строка (допустимо для /login до установки сессии).
|
||||
getSessionIdentifier: (req) => req.sessionID || '',
|
||||
|
||||
cookieOptions: {
|
||||
// secure: true → cookie передаётся только по HTTPS. Важно в продакшене.
|
||||
|
||||
+43
-5
@@ -25,6 +25,20 @@
|
||||
const crypto = require('crypto');
|
||||
const { Router } = require('express');
|
||||
|
||||
/**
|
||||
* Валидирует redirect-цель: разрешает только локальные пути (начинается с '/',
|
||||
* не начинается с '//').
|
||||
* Защита от open redirect: //evil.com и https://evil.com → '/'.
|
||||
* @param {string|undefined} target
|
||||
* @returns {string}
|
||||
*/
|
||||
function safeReturn(target) {
|
||||
if (typeof target === 'string' && target.startsWith('/') && !target.startsWith('//')) {
|
||||
return target;
|
||||
}
|
||||
return '/';
|
||||
}
|
||||
|
||||
function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, MOCK_USERS }) {
|
||||
const router = Router();
|
||||
|
||||
@@ -57,7 +71,7 @@ function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, MOCK_USER
|
||||
return res.redirect('/login?error=' + encodeURIComponent('Пользователь не найден'));
|
||||
}
|
||||
|
||||
req.session.user = {
|
||||
const userData = {
|
||||
email: user.email,
|
||||
clientId: user.clientId,
|
||||
companyId: user.companyId,
|
||||
@@ -65,7 +79,16 @@ function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, MOCK_USER
|
||||
isAdmin: user.role === 'admin',
|
||||
};
|
||||
|
||||
res.redirect(req.query.returnTo || '/');
|
||||
// session.regenerate() меняет идентификатор сессии — защита от session fixation (A07).
|
||||
// Данные пишем уже в новую сессию.
|
||||
req.session.regenerate((err) => {
|
||||
if (err) { console.error('[login] session.regenerate error:', err); return res.status(500).send('Session error'); }
|
||||
req.session.user = userData;
|
||||
req.session.save((saveErr) => {
|
||||
if (saveErr) { console.error('[login] session.save error:', saveErr); return res.status(500).send('Session error'); }
|
||||
res.redirect(safeReturn(req.query.returnTo));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── GET /callback (только OIDC-режим) ──────────────────────────────────────
|
||||
@@ -87,12 +110,19 @@ function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, MOCK_USER
|
||||
|
||||
try {
|
||||
const { user, idToken, refreshToken } = await auth.exchangeCode(code);
|
||||
// Сохраняем returnTo до regenerate, так как regenerate сбросит сессию.
|
||||
const returnTo = safeReturn(req.session.returnTo);
|
||||
// session.regenerate() — защита от session fixation (A07).
|
||||
req.session.regenerate((err) => {
|
||||
if (err) { console.error('[callback] session.regenerate error:', err); return res.status(500).send('Session error'); }
|
||||
req.session.user = user;
|
||||
req.session.idToken = idToken;
|
||||
req.session.refreshToken = refreshToken;
|
||||
const returnTo = req.session.returnTo || '/';
|
||||
delete req.session.returnTo;
|
||||
req.session.save((saveErr) => {
|
||||
if (saveErr) { console.error('[callback] session.save error:', saveErr); return res.status(500).send('Session error'); }
|
||||
res.redirect(returnTo);
|
||||
});
|
||||
});
|
||||
} catch (e) {
|
||||
console.error('[auth] exchangeCode error:', e.message);
|
||||
res.redirect('/login?error=' + encodeURIComponent('Ошибка авторизации: ' + e.message));
|
||||
@@ -159,9 +189,17 @@ function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, MOCK_USER
|
||||
};
|
||||
}
|
||||
|
||||
req.session.regenerate((err) => {
|
||||
if (err) { console.error('[dev-login] session.regenerate error:', err); return res.status(500).send('Session error'); }
|
||||
req.session.user = user;
|
||||
// Восстанавливаем devKey в новой сессии — иначе следующий запрос потребует повторного ?key=
|
||||
if (auth.DEV_SECRET) req.session.devKey = auth.DEV_SECRET;
|
||||
req.session.save((saveErr) => {
|
||||
if (saveErr) { console.error('[dev-login] session.save error:', saveErr); return res.status(500).send('Session error'); }
|
||||
res.redirect('/');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── GET /logout ────────────────────────────────────────────────────────────
|
||||
router.get('/logout', (req, res) => {
|
||||
@@ -182,4 +220,4 @@ function createRouter({ auth, doubleCsrfProtection, generateCsrfToken, MOCK_USER
|
||||
return router;
|
||||
}
|
||||
|
||||
module.exports = { createRouter };
|
||||
module.exports = { createRouter, safeReturn };
|
||||
|
||||
+29
-11
@@ -1,12 +1,14 @@
|
||||
/**
|
||||
* src/routes/export.js — публичный маршрут GET /export.
|
||||
* src/routes/export.js — маршрут GET /export (требует авторизации).
|
||||
*
|
||||
* Назначение: отдаёт агрегированный список всех активных CIDR (все компании)
|
||||
* в виде текстового файла (один адрес/подсеть на строку).
|
||||
* Назначение: отдаёт агрегированный список активных CIDR в виде текстового файла.
|
||||
*
|
||||
* Изоляция по ролям:
|
||||
* - Обычный пользователь → только CIDR своей компании.
|
||||
* - Администратор → все компании (или фильтр по ?company=<id>).
|
||||
*
|
||||
* Особенности:
|
||||
* - НЕ требует авторизации (ТЗ: «доступ ограничивается на сетевом уровне»).
|
||||
* - Регистрируется ДО auth.middleware в server.js.
|
||||
* - Требует авторизации: регистрируется ПОСЛЕ auth.middleware в server.js.
|
||||
* - Ограничен exportLimiter: не более 20 запросов/мин с одного IP.
|
||||
* - aggregateCIDRs объединяет перекрывающиеся диапазоны → минимальный набор.
|
||||
*
|
||||
@@ -25,15 +27,31 @@ const { Router } = require('express');
|
||||
function createRouter({ q, exportLimiter, aggregateCIDRs }) {
|
||||
const router = Router();
|
||||
|
||||
// GET /export — агрегированный whitelist всех компаний в текстовом виде.
|
||||
// exportLimiter ограничивает частоту запросов (20/мин) до проверки авторизации —
|
||||
// это важно, так как маршрут публичный.
|
||||
// GET /export — агрегированный whitelist CIDR в текстовом виде.
|
||||
// Требует авторизации (регистрируется ПОСЛЕ auth.middleware в server.js).
|
||||
// Изоляция по ролям:
|
||||
// - Обычный пользователь → только CIDR своей компании.
|
||||
// - Администратор → все компании, или фильтр по ?company=<id>.
|
||||
// exportLimiter ограничивает частоту запросов (20/мин).
|
||||
router.get('/export', exportLimiter, async (req, res) => {
|
||||
try {
|
||||
// Получаем все активные CIDR без фильтра по компании
|
||||
const cidrs = await q.getExportCIDRs();
|
||||
let companyId = null;
|
||||
|
||||
// Суммаризация: объединяем пересекающиеся и смежные диапазоны
|
||||
if (!req.user.isAdmin) {
|
||||
// Обычный пользователь: только его компания.
|
||||
// getOrCreateCompany — атомарная UPSERT, безопасна для параллельных вызовов.
|
||||
const company = await q.getOrCreateCompany(req.user.clientId, req.user.companyName);
|
||||
companyId = company.id;
|
||||
} else if (req.query.company) {
|
||||
// Администратор с необязательным фильтром по компании.
|
||||
const parsed = parseInt(req.query.company, 10);
|
||||
if (Number.isFinite(parsed) && parsed > 0) companyId = parsed;
|
||||
}
|
||||
// Администратор без ?company → companyId остаётся null → все компании.
|
||||
|
||||
const cidrs = await q.getExportCIDRs(companyId);
|
||||
|
||||
// Суммаризация: объединяем пересекающиеся и смежные диапазоны.
|
||||
// Например: 192.168.0.0/25 + 192.168.0.128/25 → 192.168.0.0/24
|
||||
const aggregated = aggregateCIDRs(cidrs);
|
||||
|
||||
|
||||
+102
-3
@@ -127,17 +127,22 @@ test('overlap exact match', overlaps('192.168.0.0/24', '192.168.0.0/24'), true);
|
||||
test('no overlap public', overlaps('8.8.8.0/24', '9.9.9.0/24'), false);
|
||||
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
// 4. src/auth.js — session middleware (без JWT, без БД)
|
||||
// 4. src/auth.js — session middleware (async, без JWT, без БД)
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
section('auth.js (session middleware)');
|
||||
|
||||
process.env.DEV_MODE = 'true';
|
||||
process.env.ADMIN_CLIENT_ID = 'WZ01112';
|
||||
|
||||
const { initAuth, requireAdmin } = require('../src/auth');
|
||||
const { safeReturn } = require('../src/routes/auth');
|
||||
const csrfMiddleware = require('../src/middleware/csrf');
|
||||
|
||||
(async () => {
|
||||
|
||||
section('auth.js (session middleware)');
|
||||
|
||||
try {
|
||||
const auth = initAuth();
|
||||
const auth = await initAuth();
|
||||
test('middleware is function', typeof auth.middleware, 'function');
|
||||
test('jwksHandler present (mock mode)', typeof auth.jwksHandler, 'function');
|
||||
test('devLoginEnabled when DEV_MODE', auth.devLoginEnabled, true);
|
||||
@@ -183,6 +188,95 @@ try {
|
||||
fail++;
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
// 5. safeReturn — защита от open redirect (A01)
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
section('safeReturn (open redirect protection)');
|
||||
|
||||
test('normal path allowed', safeReturn('/dashboard'), '/dashboard');
|
||||
test('root / allowed', safeReturn('/'), '/');
|
||||
test('path with query allowed', safeReturn('/edit?id=5'), '/edit?id=5');
|
||||
test('//evil.com blocked', safeReturn('//evil.com'), '/');
|
||||
test('https://evil.com blocked', safeReturn('https://evil.com'), '/');
|
||||
test('http://evil.com blocked', safeReturn('http://evil.com'), '/');
|
||||
test('undefined → /', safeReturn(undefined), '/');
|
||||
test('null → /', safeReturn(null), '/');
|
||||
test('empty string → /', safeReturn(''), '/');
|
||||
test('//ok.com/path blocked', safeReturn('//ok.com/path'), '/');
|
||||
test('nested path allowed', safeReturn('/admin/users'), '/admin/users');
|
||||
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
// 6. CSRF middleware — getSessionIdentifier использует req.sessionID
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
section('CSRF getSessionIdentifier');
|
||||
|
||||
// Проверяем что csrf.js экспортирует объект с initCsrf
|
||||
test('csrf module exports initCsrf', typeof csrfMiddleware.initCsrf, 'function');
|
||||
|
||||
// Проверяем поведение getSessionIdentifier через тестовые значения
|
||||
// Функция в модуле читает req.sessionID (не req.cookies.jwt)
|
||||
const { doubleCsrfProtection: dp, generateCsrfToken: gct } = csrfMiddleware.initCsrf();
|
||||
test('initCsrf returns doubleCsrfProtection fn', typeof dp, 'function');
|
||||
test('initCsrf returns generateCsrfToken fn', typeof gct, 'function');
|
||||
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
// 7. Export auth isolation — req.user влияет на фильтр компании
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
section('Export auth isolation');
|
||||
|
||||
// Тестируем логику без реального HTTP — проверяем что createRouter не бросает при require
|
||||
const exportModule = require('../src/routes/export');
|
||||
test('export module exports createRouter', typeof exportModule.createRouter, 'function');
|
||||
|
||||
// Логика изоляции: обычный пользователь получает companyId из q.getOrCreateCompany,
|
||||
// admin без ?company → companyId = null (все компании).
|
||||
// Проверяем через минимальный stub — без реальной БД.
|
||||
let exportCompanyIdCaptured = null;
|
||||
const stubQ = {
|
||||
getOrCreateCompany: async (clientId) => ({ id: 42, clientId }),
|
||||
getExportCIDRs: async (cid) => { exportCompanyIdCaptured = cid; return []; },
|
||||
};
|
||||
const exportRouter = exportModule.createRouter({
|
||||
q: stubQ,
|
||||
exportLimiter: (req, res, next) => next(),
|
||||
aggregateCIDRs: (x) => x,
|
||||
});
|
||||
|
||||
// Симулируем GET /export для обычного пользователя
|
||||
const mockReqUser = {
|
||||
user: { isAdmin: false, clientId: 'WZ01325', companyName: 'TestCo' },
|
||||
query: {},
|
||||
method: 'GET',
|
||||
path: '/export',
|
||||
};
|
||||
const mockResUser = {
|
||||
setHeader: () => {},
|
||||
send: () => {},
|
||||
};
|
||||
// Найдём обработчик /export в router.stack
|
||||
const exportLayer = exportRouter.stack.find(l => l.route && l.route.path === '/export');
|
||||
test('export route registered', !!exportLayer, true);
|
||||
|
||||
if (exportLayer) {
|
||||
// Запускаем handler напрямую
|
||||
const handlers = exportLayer.route.stack.map(l => l.handle);
|
||||
// exportLimiter (stub пропускает) + async handler
|
||||
exportCompanyIdCaptured = 'NOT_CALLED';
|
||||
await handlers[handlers.length - 1](mockReqUser, mockResUser, () => {});
|
||||
test('user export: companyId filtered (42)', exportCompanyIdCaptured, 42);
|
||||
|
||||
// Симулируем GET /export для admin без ?company
|
||||
exportCompanyIdCaptured = 'NOT_CALLED';
|
||||
const mockReqAdmin = {
|
||||
user: { isAdmin: true, clientId: 'WZ01112', companyName: 'Admin' },
|
||||
query: {},
|
||||
method: 'GET',
|
||||
path: '/export',
|
||||
};
|
||||
await handlers[handlers.length - 1](mockReqAdmin, mockResUser, () => {});
|
||||
test('admin export: no filter (null)', exportCompanyIdCaptured, null);
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
// Итог
|
||||
// ────────────────────────────────────────────────────────────────
|
||||
@@ -190,3 +284,8 @@ console.log('\n═════════════════════
|
||||
console.log(pass + ' PASS, ' + fail + ' FAIL');
|
||||
console.log('══════════════════════════════');
|
||||
process.exit(fail > 0 ? 1 : 0);
|
||||
|
||||
})().catch(e => {
|
||||
console.error('\nTest runner fatal error:', e.message);
|
||||
process.exit(1);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user