Skip to content

Commit

Permalink
fix: sanitize URL to prevent leaking user credentials in logs (#6175)
Browse files Browse the repository at this point in the history
* fix: sanitize URL to prevent leaking user credentials in logs

* Best effort to sanitize if an invalid URL is provided

* Only log used builder URL

* Ensure URLs are logged after validation
nflaig authored Dec 12, 2023

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
1 parent cb8607e commit d7357fd
Showing 7 changed files with 20 additions and 11 deletions.
4 changes: 2 additions & 2 deletions packages/beacon-node/src/eth1/provider/eth1Provider.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {toHexString} from "@chainsafe/ssz";
import {phase0} from "@lodestar/types";
import {ChainConfig} from "@lodestar/config";
import {fromHex, isErrorAborted, createElapsedTimeTracker} from "@lodestar/utils";
import {fromHex, isErrorAborted, createElapsedTimeTracker, toSafePrintableUrl} from "@lodestar/utils";
import {Logger} from "@lodestar/logger";

import {FetchError, isFetchError} from "@lodestar/api";
@@ -75,7 +75,6 @@ export class Eth1Provider implements IEth1Provider {
this.depositContractAddress = toHexString(config.DEPOSIT_CONTRACT_ADDRESS);

const providerUrls = opts.providerUrls ?? DEFAULT_PROVIDER_URLS;
this.logger?.info("Eth1 provider", {urls: providerUrls.toString()});
this.rpc = new JsonRpcHttpClient(providerUrls, {
signal,
// Don't fallback with is truncated error. Throw early and let the retry on this class handle it
@@ -85,6 +84,7 @@ export class Eth1Provider implements IEth1Provider {
jwtVersion: opts.jwtVersion,
metrics: metrics,
});
this.logger?.info("Eth1 provider", {urls: providerUrls.map(toSafePrintableUrl).toString()});

this.rpc.emitter.on(JsonRpcHttpClientEvent.RESPONSE, () => {
const oldState = this.state;
4 changes: 2 additions & 2 deletions packages/beacon-node/src/execution/builder/http.ts
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ import {ChainForkConfig} from "@lodestar/config";
import {Logger} from "@lodestar/logger";
import {getClient, Api as BuilderApi} from "@lodestar/api/builder";
import {SLOTS_PER_EPOCH, ForkExecution} from "@lodestar/params";

import {toSafePrintableUrl} from "@lodestar/utils";
import {ApiError} from "@lodestar/api";
import {Metrics} from "../../metrics/metrics.js";
import {IExecutionBuilder} from "./interface.js";
@@ -50,7 +50,6 @@ export class ExecutionBuilderHttp implements IExecutionBuilder {
) {
const baseUrl = opts.urls[0];
if (!baseUrl) throw Error("No Url provided for executionBuilder");
logger?.info("External builder", {urls: opts.urls.toString()});
this.api = getClient(
{
baseUrl,
@@ -59,6 +58,7 @@ export class ExecutionBuilderHttp implements IExecutionBuilder {
},
{config, metrics: metrics?.builderHttpClient}
);
logger?.info("External builder", {url: toSafePrintableUrl(baseUrl)});
this.config = config;
this.issueLocalFcUWithFeeRecipient = opts.issueLocalFcUWithFeeRecipient;

4 changes: 2 additions & 2 deletions packages/beacon-node/src/execution/engine/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {fromHex} from "@lodestar/utils";
import {fromHex, toSafePrintableUrl} from "@lodestar/utils";
import {JsonRpcHttpClient} from "../../eth1/provider/jsonRpcHttpClient.js";
import {IExecutionEngine} from "./interface.js";
import {ExecutionEngineDisabled} from "./disabled.js";
@@ -31,7 +31,6 @@ export function getExecutionEngineHttp(
opts: ExecutionEngineHttpOpts,
modules: ExecutionEngineModules
): IExecutionEngine {
modules.logger.info("Execution client", {urls: opts.urls.toString()});
const rpc = new JsonRpcHttpClient(opts.urls, {
...opts,
signal: modules.signal,
@@ -40,6 +39,7 @@ export function getExecutionEngineHttp(
jwtId: opts.jwtId,
jwtVersion: opts.jwtVersion,
});
modules.logger.info("Execution client", {urls: opts.urls.map(toSafePrintableUrl).toString()});
return new ExecutionEngineHttp(rpc, modules);
}

4 changes: 2 additions & 2 deletions packages/cli/src/cmds/validator/signers/logSigners.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Signer, SignerLocal, SignerRemote, SignerType} from "@lodestar/validator";
import {LogLevel, Logger} from "@lodestar/utils";
import {LogLevel, Logger, toSafePrintableUrl} from "@lodestar/utils";

/**
* Log each pubkeys for auditing out keys are loaded from the logs
@@ -27,7 +27,7 @@ export function logSigners(logger: Pick<Logger, LogLevel.info>, signers: Signer[
}

for (const {url, pubkeys} of groupExternalSignersByUrl(remoteSigners)) {
logger.info(`External signers on URL: ${url}`);
logger.info(`External signers on URL: ${toSafePrintableUrl(url)}`);
for (const pubkey of pubkeys) {
logger.info(pubkey);
}
2 changes: 1 addition & 1 deletion packages/utils/src/index.ts
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ export * from "./sleep.js";
export * from "./sort.js";
export * from "./timeout.js";
export {type RecursivePartial, bnToNum} from "./types.js";
export * from "./validation.js";
export * from "./url.js";
export * from "./verifyMerkleBranch.js";
export * from "./promise.js";
export * from "./waitFor.js";
Original file line number Diff line number Diff line change
@@ -18,3 +18,12 @@ export function isValidHttpUrl(urlStr: string): boolean {

return url.protocol === "http:" || url.protocol === "https:";
}

/**
* Sanitize URL to prevent leaking user credentials in logs
*
* Note: `urlStr` must be a valid URL
*/
export function toSafePrintableUrl(urlStr: string): string {
return new URL(urlStr).origin;
}
4 changes: 2 additions & 2 deletions packages/validator/src/validator.ts
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import {toHexString} from "@chainsafe/ssz";
import {BLSPubkey, phase0, ssz} from "@lodestar/types";
import {createBeaconConfig, BeaconConfig, ChainForkConfig} from "@lodestar/config";
import {Genesis} from "@lodestar/types/phase0";
import {Logger} from "@lodestar/utils";
import {Logger, toSafePrintableUrl} from "@lodestar/utils";
import {getClient, Api, routes, ApiError} from "@lodestar/api";
import {computeEpochAtSlot, getCurrentSlot} from "@lodestar/state-transition";
import {Clock, IClock} from "./util/clock.js";
@@ -268,10 +268,10 @@ export class Validator {
let api: Api;
if (typeof opts.api === "string" || Array.isArray(opts.api)) {
const urls = typeof opts.api === "string" ? [opts.api] : opts.api;
logger.info("Beacon node", {urls: urls.toString()});
// This new api instance can make do with default timeout as a faster timeout is
// not necessary since this instance won't be used for validator duties
api = getClient({urls, getAbortSignal: () => opts.abortController.signal}, {config, logger});
logger.info("Beacon node", {urls: urls.map(toSafePrintableUrl).toString()});
} else {
api = opts.api;
}

0 comments on commit d7357fd

Please sign in to comment.