mirror of
https://github.com/screentinker/screentinker.git
synced 2026-05-15 07:32:23 -06:00
fix: log real client IPs through Cloudflare instead of CF edge
Express's req.ip was resolving to a Cloudflare edge address (e.g. 172.70.x.x) for any request fronted by Cloudflare, because trust proxy was set to '1' — that trusts the immediate hop, which IS Cloudflare. All activity_log rows from API paths captured the proxy, not the client. The WebSocket path was unaffected and recorded the real IP. Two layers of defense: 1. trust proxy now lists Cloudflare's published v4 + v6 ranges plus loopback / linklocal / uniquelocal (config/cloudflareIps.js). With this list req.ip resolves to the original client when fronted by CF, and X-Forwarded-For from any non-trusted source is ignored — so the value can't be spoofed. 2. New getClientIp(req) helper in services/activity.js prefers the CF-Connecting-IP header but only honors it when the immediate TCP peer is itself a trusted address. Same gate as trust proxy, so a visitor who hits the origin directly with a forged header is logged at their real address. Routed all five activity-log call sites (auth login success/failure, admin password reset, generic activityLogger middleware, and the in-memory rate-limiter key) through the helper. Logging-only change. No schema changes. Existing rows are not modified — fix applies to new entries going forward. Verified locally: - Bare loopback hit logs 127.0.0.1 (not a proxy address). - Helper unit cases including an untrusted peer (203.0.113.7) sending a forged CF-Connecting-IP correctly fall back to the real peer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2068bc8833
commit
45a6800621
39
server/config/cloudflareIps.js
Normal file
39
server/config/cloudflareIps.js
Normal file
|
|
@ -0,0 +1,39 @@
|
|||
// Cloudflare published edge IP ranges.
|
||||
// Source: https://www.cloudflare.com/ips-v4 and https://www.cloudflare.com/ips-v6
|
||||
// Snapshot: 2026-05-07. Update by hand when Cloudflare publishes a new list.
|
||||
const cloudflareIpv4 = [
|
||||
'173.245.48.0/20',
|
||||
'103.21.244.0/22',
|
||||
'103.22.200.0/22',
|
||||
'103.31.4.0/22',
|
||||
'141.101.64.0/18',
|
||||
'108.162.192.0/18',
|
||||
'190.93.240.0/20',
|
||||
'188.114.96.0/20',
|
||||
'197.234.240.0/22',
|
||||
'198.41.128.0/17',
|
||||
'162.158.0.0/15',
|
||||
'104.16.0.0/13',
|
||||
'104.24.0.0/14',
|
||||
'172.64.0.0/13',
|
||||
'131.0.72.0/22',
|
||||
];
|
||||
|
||||
const cloudflareIpv6 = [
|
||||
'2400:cb00::/32',
|
||||
'2606:4700::/32',
|
||||
'2803:f800::/32',
|
||||
'2405:b500::/32',
|
||||
'2405:8100::/32',
|
||||
'2a06:98c0::/29',
|
||||
'2c0f:f248::/32',
|
||||
];
|
||||
|
||||
const cloudflareIps = [...cloudflareIpv4, ...cloudflareIpv6];
|
||||
|
||||
// What Express's trust-proxy and our CF-Connecting-IP gate both honor.
|
||||
// 'loopback', 'linklocal', 'uniquelocal' keep local dev and any LAN reverse
|
||||
// proxy working without further config.
|
||||
const trustedProxies = ['loopback', 'linklocal', 'uniquelocal', ...cloudflareIps];
|
||||
|
||||
module.exports = { cloudflareIpv4, cloudflareIpv6, cloudflareIps, trustedProxies };
|
||||
|
|
@ -6,7 +6,7 @@ const { v4: uuidv4 } = require('uuid');
|
|||
const { OAuth2Client } = require('google-auth-library');
|
||||
const { db } = require('../db/database');
|
||||
const { generateToken, requireAuth, requireAdmin, requireSuperAdmin } = require('../middleware/auth');
|
||||
const { logActivity } = require('../services/activity');
|
||||
const { logActivity, getClientIp } = require('../services/activity');
|
||||
const config = require('../config');
|
||||
|
||||
function logFailedLogin(email, ip, reason) {
|
||||
|
|
@ -74,16 +74,16 @@ router.post('/login', (req, res) => {
|
|||
|
||||
const user = db.prepare('SELECT * FROM users WHERE email = ? AND auth_provider = ?').get(email.toLowerCase(), 'local');
|
||||
if (!user) {
|
||||
logFailedLogin(email, req.ip, 'User not found');
|
||||
logFailedLogin(email, getClientIp(req), 'User not found');
|
||||
return res.status(401).json({ error: 'Invalid email or password' });
|
||||
}
|
||||
|
||||
if (!bcrypt.compareSync(password, user.password_hash)) {
|
||||
logFailedLogin(email, req.ip, 'Wrong password');
|
||||
logFailedLogin(email, getClientIp(req), 'Wrong password');
|
||||
return res.status(401).json({ error: 'Invalid email or password' });
|
||||
}
|
||||
|
||||
logSuccessfulLogin(user.id, email, req.ip);
|
||||
logSuccessfulLogin(user.id, email, getClientIp(req));
|
||||
const token = generateToken(user);
|
||||
const { password_hash, ...safeUser } = user;
|
||||
res.json({ token, user: safeUser });
|
||||
|
|
@ -348,7 +348,7 @@ router.put('/users/:id/password', requireAuth, requireAdmin, (req, res) => {
|
|||
// Explicit audit entry — the generic activity logger captures the route
|
||||
// and target id, but a labeled detail string makes the audit log readable.
|
||||
// Never include the password; just who reset whose password.
|
||||
logActivity(req.user.id, 'password_reset_for_user', `target: ${target.email}`, null, req.ip);
|
||||
logActivity(req.user.id, 'password_reset_for_user', `target: ${target.email}`, null, getClientIp(req));
|
||||
res.json({ success: true });
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -13,7 +13,13 @@ const config = require('./config');
|
|||
});
|
||||
|
||||
const app = express();
|
||||
app.set('trust proxy', 1);
|
||||
const { trustedProxies } = require('./config/cloudflareIps');
|
||||
const { getClientIp } = require('./services/activity');
|
||||
// Trust loopback / link-local / unique-local (local dev, LAN reverse proxies)
|
||||
// and Cloudflare's published edge ranges. With this list, req.ip resolves to
|
||||
// the original client when fronted by Cloudflare; X-Forwarded-For from any
|
||||
// non-trusted source is ignored, so the value can't be spoofed.
|
||||
app.set('trust proxy', trustedProxies);
|
||||
|
||||
// Determine if SSL certs are available
|
||||
const hasSsl = fs.existsSync(config.sslCert) && fs.existsSync(config.sslKey);
|
||||
|
|
@ -186,7 +192,7 @@ app.use('/socket.io-client', express.static(
|
|||
const rateLimits = new Map();
|
||||
function rateLimit(windowMs, maxRequests) {
|
||||
return (req, res, next) => {
|
||||
const key = req.ip + req.path;
|
||||
const key = getClientIp(req) + req.path;
|
||||
const now = Date.now();
|
||||
const windowStart = now - windowMs;
|
||||
let hits = rateLimits.get(key) || [];
|
||||
|
|
|
|||
|
|
@ -1,4 +1,31 @@
|
|||
const { db } = require('../db/database');
|
||||
const proxyaddr = require('proxy-addr');
|
||||
const { trustedProxies } = require('../config/cloudflareIps');
|
||||
|
||||
// Gate function: returns true when an immediate TCP peer is one we trust
|
||||
// to populate forwarding headers (Cloudflare edges, loopback, link-local,
|
||||
// unique-local). Mirrors what `app.set('trust proxy', trustedProxies)` does
|
||||
// for X-Forwarded-For so that CF-Connecting-IP is held to the same standard.
|
||||
const isTrustedPeer = proxyaddr.compile(trustedProxies);
|
||||
|
||||
// Resolve the real client IP for logging.
|
||||
//
|
||||
// Cloudflare always sets `CF-Connecting-IP` to the original client address
|
||||
// when it proxies a request. We prefer that header — but only when the
|
||||
// connection's immediate peer is a trusted CF/loopback address; otherwise
|
||||
// any random visitor could spoof the header by hitting the origin directly.
|
||||
//
|
||||
// Falls back to req.ip (which Express resolves via the trust-proxy table)
|
||||
// so local dev and any non-CF deployment keep working unchanged.
|
||||
function getClientIp(req) {
|
||||
if (!req) return null;
|
||||
const cf = req.headers && req.headers['cf-connecting-ip'];
|
||||
if (typeof cf === 'string' && cf.length > 0) {
|
||||
const peer = req.socket && req.socket.remoteAddress;
|
||||
if (peer && isTrustedPeer(peer, 0)) return cf;
|
||||
}
|
||||
return req.ip || null;
|
||||
}
|
||||
|
||||
function logActivity(userId, action, details = null, deviceId = null, ipAddress = null) {
|
||||
try {
|
||||
|
|
@ -40,7 +67,7 @@ function activityLogger(req, res, next) {
|
|||
const userId = req.user?.id;
|
||||
const deviceId = req.params?.id || req.params?.deviceId || req.body?.device_id;
|
||||
const details = summarizeAction(req);
|
||||
logActivity(userId, action, details, deviceId, req.ip);
|
||||
logActivity(userId, action, details, deviceId, getClientIp(req));
|
||||
}
|
||||
return originalJson(data);
|
||||
};
|
||||
|
|
@ -57,4 +84,4 @@ function summarizeAction(req) {
|
|||
return parts.join(', ') || null;
|
||||
}
|
||||
|
||||
module.exports = { logActivity, getActivity, pruneActivityLog, activityLogger };
|
||||
module.exports = { logActivity, getActivity, pruneActivityLog, activityLogger, getClientIp };
|
||||
|
|
|
|||
Loading…
Reference in a new issue