-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Description
Bug Description
The encryptClientIp function in encryption.ts has two failure paths that both return the raw, unencrypted client IP address. This plaintext IP is then sent in the mcp-client-ip HTTP header, defeating the purpose of the encryption feature.
Affected Code
File: packages/mcp/src/lib/encryption.ts, lines 14-27
function encryptClientIp(clientIp: string): string {
if (!validateEncryptionKey(ENCRYPTION_KEY)) {
console.error("Invalid encryption key format. Must be 64 hex characters.");
return clientIp; // PATH 1: returns raw IP
}
try {
const iv = randomBytes(16);
const cipher = createCipheriv(ALGORITHM, Buffer.from(ENCRYPTION_KEY, "hex"), iv);
let encrypted = cipher.update(clientIp, "utf8", "hex");
encrypted += cipher.final("hex");
return iv.toString("hex") + ":" + encrypted;
} catch (error) {
console.error("Error encrypting client IP:", error);
return clientIp; // PATH 2: returns raw IP
}
}Call site in generateHeaders:
if (context.clientIp) {
headers["mcp-client-ip"] = encryptClientIp(context.clientIp);
}Impact
- Path 1: Operator sets
CLIENT_IP_ENCRYPTION_KEYto a malformed value (not 64 hex chars). The console.error is logged, but the header is sent with the plaintext IP. The operator believes IPs are encrypted. - Path 2: Any runtime crypto error (corrupted key buffer, OpenSSL issue) causes the same plaintext fallback with only a console.error.
- The receiving server has no way to distinguish encrypted from plaintext values — both arrive in the same
mcp-client-ipheader.
This is a fail-open security behavior: encryption failure silently degrades to no encryption.
Expected Behavior
On encryption failure, the function should either:
- Return
undefinedso the header is not set at all (fail-closed), OR - Return a clearly-marked placeholder like
"ENCRYPTION_FAILED"that the server can detect
Proposed Solution
Option A (recommended — fail-closed):
function encryptClientIp(clientIp: string): string | undefined {
if (!validateEncryptionKey(ENCRYPTION_KEY)) {
console.error("Invalid encryption key format. Skipping IP header.");
return undefined;
}
try {
// ... encryption logic ...
} catch (error) {
console.error("Error encrypting client IP:", error);
return undefined;
}
}
// Call site:
const encryptedIp = encryptClientIp(context.clientIp);
if (encryptedIp) {
headers["mcp-client-ip"] = encryptedIp;
}Option B (fail-open with marker):
return "UNENCRYPTED"; // Server can detect and handle| Aspect | Option A | Option B |
|---|---|---|
| Privacy | No IP sent on failure | IP is sent but marked |
| Server compatibility | Server may lose IP data | Server can detect unencrypted |
Note: This is related to but distinct from the recently closed #1366 (which was a docs issue about the default key). This report is specifically about the code-level plaintext fallback behavior.
Happy to submit a PR for this fix.