From d76993ef5b0f3e27de07d4fb2070059af0e117af Mon Sep 17 00:00:00 2001 From: Achal Singh Date: Mon, 24 Jun 2024 21:12:18 +0530 Subject: [PATCH] chore: hardening added for some params in /set-config and type checks in updateConfig --- src/API.ts | 15 +++++-- src/Config.ts | 10 +++-- src/Data/Data.ts | 92 ++++++++++++++++++++++++++----------------- src/ShardFunctions.ts | 2 +- 4 files changed, 74 insertions(+), 45 deletions(-) diff --git a/src/API.ts b/src/API.ts index f065cc64..c011f0c4 100644 --- a/src/API.ts +++ b/src/API.ts @@ -919,20 +919,29 @@ export function registerRoutes(server: FastifyInstance { + const RESTRICTED_PARAMS = [ + 'ARCHIVER_IP', + 'ARCHIVER_PORT', + 'ARCHIVER_HASH_KEY', + 'ARCHIVER_SECRET_KEY', + 'ARCHIVER_PUBLIC_KEY', + ] try { const { sign, ...newConfig } = _request.body const validKeys = new Set(Object.keys(config)) const payloadKeys = Object.keys(newConfig) - const invalidKeys = payloadKeys.filter((key) => !validKeys.has(key)) + const invalidKeys = payloadKeys.filter( + (key) => !validKeys.has(key) || RESTRICTED_PARAMS.includes(key) + ) if (invalidKeys.length > 0) - throw new Error(`Invalid config properties provided: ${invalidKeys.join(', ')}`) + throw new Error(`Invalid/Unauthorised config properties provided: ${invalidKeys.join(', ')}`) if (config.VERBOSE) Logger.mainLogger.debug('Archiver config update executed: ', JSON.stringify(newConfig)) const updatedConfig = updateConfig(newConfig) - reply.send({ success: true, updatedConfig }) + reply.send({ success: true, ...updatedConfig, ARCHIVER_SECRET_KEY: '' }) } catch (error) { reply.status(400).send({ success: false, reason: error.message }) } diff --git a/src/Config.ts b/src/Config.ts index 0af894ed..6d09d610 100644 --- a/src/Config.ts +++ b/src/Config.ts @@ -228,10 +228,12 @@ export async function overrideDefaultConfig(file: string): Promise { export function updateConfig(newConfig: Partial): Config { for (const key in newConfig) { - if (newConfig[key] === 'true') newConfig[key] = true - else if (newConfig[key] === 'false') newConfig[key] = false - else if (typeof newConfig[key] !== 'boolean' && !Number.isNaN(Number(newConfig[key]))) - newConfig[key] = Number(newConfig[key]) + if (typeof newConfig[key] !== typeof config[key]) + throw new Error( + `Value with incorrect type passed to update the Archiver Config: ${key}:${ + newConfig[key] + } of type ${typeof newConfig[key]}` + ) } config = merge(config, newConfig) return config diff --git a/src/Data/Data.ts b/src/Data/Data.ts index 513f590d..1824695e 100644 --- a/src/Data/Data.ts +++ b/src/Data/Data.ts @@ -536,49 +536,57 @@ export function addDataSender(sender: DataSender): void { } async function syncFromNetworkConfig(): Promise { - // Define the query function to get the network config from a node - const queryFn = async (node): Promise => { - const REQUEST_NETCONFIG_TIMEOUT_SECOND = 2 // 2s timeout - try { - const response = await P2P.getJson( - `http://${node.ip}:${node.port}/netconfig`, - REQUEST_NETCONFIG_TIMEOUT_SECOND + try { + // Define the query function to get the network config from a node + const queryFn = async (node): Promise => { + const REQUEST_NETCONFIG_TIMEOUT_SECOND = 2 // 2s timeout + try { + const response = await P2P.getJson( + `http://${node.ip}:${node.port}/netconfig`, + REQUEST_NETCONFIG_TIMEOUT_SECOND + ) + return response + } catch (error) { + Logger.mainLogger.error(`Error querying node ${node.ip}:${node.port}: ${error}`) + return null + } + } + // Define the equality function to compare two responses + const equalityFn = (responseA, responseB): boolean => { + return ( + responseA?.config?.sharding?.nodesPerConsensusGroup === + responseB?.config?.sharding?.nodesPerConsensusGroup ) - return response - } catch (error) { - Logger.mainLogger.error(`Error querying node ${node.ip}:${node.port}: ${error}`) - return null } - } - // Define the equality function to compare two responses - const equalityFn = (responseA, responseB): boolean => { - return ( - responseA?.config?.sharding?.nodesPerConsensusGroup === - responseB?.config?.sharding?.nodesPerConsensusGroup + // Get the list of 10 max random active nodes or the first node if no active nodes are available + const nodes = + NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : [NodeList.getFirstNode()] + // Use robustQuery to get the consensusRadius from multiple nodes + const tallyItem = await robustQuery( + nodes, + queryFn, + equalityFn, + 3 // Redundancy (minimum 3 nodes should return the same result to reach consensus) ) - } - // Get the list of 10 max random active nodes or the first node if no active nodes are available - const nodes = - NodeList.getActiveNodeCount() > 0 ? NodeList.getRandomActiveNodes(10) : [NodeList.getFirstNode()] - // Use robustQuery to get the consensusRadius from multiple nodes - const tallyItem = await robustQuery( - nodes, - queryFn, - equalityFn, - 3 // Redundancy (minimum 3 nodes should return the same result to reach consensus) - ) - if (tallyItem?.value?.config) { - // Updating the Archiver Config as per the latest Network Config - const devPublicKeys = tallyItem.value.config.devPublicKeys - const updateConfigProps = { - newPOQReceipt: tallyItem.value.config.useNewPOQ, - DevPublicKey: Object.keys(devPublicKeys).find((key) => devPublicKeys[key] === 3), + if (tallyItem?.value?.config) { + // Updating the Archiver Config as per the latest Network Config + const devPublicKeys = tallyItem.value.config?.devPublicKeys + const updateConfigProps = { + newPOQReceipt: tallyItem.value.config?.useNewPOQ, + DevPublicKey: + devPublicKeys && Object.keys(devPublicKeys).length >= 3 + ? Object.keys(devPublicKeys).find((key) => devPublicKeys[key] === 3) + : '', + } + updateConfig(updateConfigProps) + return tallyItem } - updateConfig(updateConfigProps) - return tallyItem + return null + } catch (error) { + Logger.mainLogger.error('❌ Error in syncFromNetworkConfig: ', error) + return null } - return null } async function getConsensusRadius(): Promise { @@ -590,6 +598,16 @@ async function getConsensusRadius(): Promise { if (tallyItem?.value?.config) { nodesPerEdge = tallyItem.value.config.sharding.nodesPerEdge nodesPerConsensusGroup = tallyItem.value.config.sharding.nodesPerConsensusGroup + + if (!Number.isInteger(nodesPerConsensusGroup) || nodesPerConsensusGroup <= 0) { + Logger.mainLogger.error('nodesPerConsensusGroup is not a valid number:', nodesPerConsensusGroup) + return currentConsensusRadius + } + + if (!Number.isInteger(nodesPerEdge) || nodesPerEdge <= 0) { + Logger.mainLogger.error('nodesPerEdge is not a valid number:', nodesPerEdge) + return currentConsensusRadius + } // Upgrading consensus size to an odd number if (nodesPerConsensusGroup % 2 === 0) nodesPerConsensusGroup++ const consensusRadius = Math.floor((nodesPerConsensusGroup - 1) / 2) diff --git a/src/ShardFunctions.ts b/src/ShardFunctions.ts index 616b15d7..d0d62564 100644 --- a/src/ShardFunctions.ts +++ b/src/ShardFunctions.ts @@ -52,7 +52,7 @@ class ShardFunctions { //make sure nodesPerConsenusGroup is an odd number >= 3 if (nodesPerConsenusGroup % 2 === 0 || nodesPerConsenusGroup < 3) { - throw new Error(`nodesPerConsenusGroup:${nodesPerConsenusGroup} must be odd and >= 3`) + throw new Error(`nodesPerConsenusGroup: ${nodesPerConsenusGroup} must be odd and >= 3`) } shardGlobals.consensusRadius = Math.floor((nodesPerConsenusGroup - 1) / 2)