From 86139839f16abb56c734512a4a8c9155e396bd5b Mon Sep 17 00:00:00 2001 From: cheatfate Date: Thu, 27 Aug 2020 16:24:30 +0300 Subject: [PATCH] Add permissions checks and handling to wallets and bls keystores. --- beacon_chain/beacon_node.nim | 16 ++- beacon_chain/keystore_management.nim | 195 +++++++++++++++----------- beacon_chain/nimbus_binary_common.nim | 3 +- 3 files changed, 130 insertions(+), 84 deletions(-) diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim index ae8a4d6a6..c6ac8c32a 100644 --- a/beacon_chain/beacon_node.nim +++ b/beacon_chain/beacon_node.nim @@ -1095,6 +1095,8 @@ programMain: # This is ref so we can mutate it (to erase it) after the initial loading. stateSnapshotContents: ref string + setupStdoutLogging(config.logLevel) + if not(checkAndCreateDataDir(string(config.dataDir))): # We are unable to access/create data folder or data folder's # permissions are insecure. @@ -1321,12 +1323,16 @@ programMain: of WalletsCmd.list: for kind, walletFile in walkDir(config.walletsDir): if kind != pcFile: continue - let walletRes = loadWallet(walletFile) - if walletRes.isOk: - echo walletRes.get.longName + if checkFilePermissions(walletFile): + let walletRes = loadWallet(walletFile) + if walletRes.isOk: + echo walletRes.get.longName + else: + warn "Found corrupt wallet file", + wallet = walletFile, error = walletRes.error else: - warn "Found corrupt wallet file", - wallet = walletFile, error = walletRes.error + warn "Found wallet file with insecure permissions", + wallet = walletFile of WalletsCmd.restore: restoreWalletInteractively(rng[], config) diff --git a/beacon_chain/keystore_management.nim b/beacon_chain/keystore_management.nim index 3446ae090..89793841f 100644 --- a/beacon_chain/keystore_management.nim +++ b/beacon_chain/keystore_management.nim @@ -41,8 +41,8 @@ proc checkAndCreateDataDir*(dataDir: string): bool = ## If folder exists, procedure will check it for access and ## permissions `0750 (rwxr-x---)`, if folder do not exists it will be created ## with permissions `0750 (rwxr-x---)`. + let amask = {AccessFlags.Read, AccessFlags.Write, AccessFlags.Execute} when defined(posix): - let amask = {AccessFlags.Read, AccessFlags.Write, AccessFlags.Execute} if fileAccessible(dataDir, amask): let gmask = {UserRead, UserWrite, UserExec, GroupRead, GroupExec} let pmask = {OtherRead, OtherWrite, OtherExec, GroupWrite} @@ -51,32 +51,71 @@ proc checkAndCreateDataDir*(dataDir: string): bool = fatal "Could not check data folder permissions", data_dir = dataDir, errorCode = $pres.error, errorMsg = ioErrorMsg(pres.error) - return false - let insecurePermissions = pres.get() * pmask - if insecurePermissions != {}: - fatal "Data folder has insecure permissions", - data_dir = dataDir, - insecure_permissions = $insecurePermissions, - current_permissions = pres.get().toString(), - required_permissions = gmask.toString() - return false + false + else: + let insecurePermissions = pres.get() * pmask + if insecurePermissions != {}: + fatal "Data folder has insecure permissions", + data_dir = dataDir, + insecure_permissions = $insecurePermissions, + current_permissions = pres.get().toString(), + required_permissions = gmask.toString() + false + else: + true else: let res = createPath(dataDir, 0o750) if res.isErr(): fatal "Could not create data folder", data_dir = dataDir, errorMsg = ioErrorMsg(res.error), errorCode = $res.error - return false - return true + false + else: + true elif defined(windows): - let res = createPath(dataDir, 0o750) - if res.isErr(): - fatal "Could not create data folder", data_dir = dataDir, - errorMsg = ioErrorMsg(res.error), errorCode = $res.error - return false + if fileAccessible(dataDir, amask): + let res = createPath(dataDir, 0o750) + if res.isErr(): + fatal "Could not create data folder", data_dir = dataDir, + errorMsg = ioErrorMsg(res.error), errorCode = $res.error + false + else: + true + else: + true else: fatal "Unsupported operation system" return false +proc checkFilePermissions*(filePath: string): bool = + ## Check if ``filePath`` has only "(600) rw-------" permissions. + ## Procedure returns ``false`` if permissions are different + when defined(windows): + # Windows do not support per-user/group/other permissions, + # skiping verification part. + true + else: + let allowedMask = {UserRead, UserWrite} + let mask = {UserExec, + GroupRead, GroupWrite, GroupExec, + OtherRead, OtherWrite, OtherExec} + let pres = getPermissionsSet(filePath) + if pres.isErr(): + error "Could not check file permissions", + key_path = filePath, errorCode = $pres.error, + errorMsg = ioErrorMsg(pres.error) + false + else: + let insecurePermissions = pres.get() * mask + if insecurePermissions != {}: + error "File has insecure permissions", + key_path = filePath, + insecure_permissions = $insecurePermissions, + current_permissions = pres.get().toString(), + required_permissions = allowedMask.toString() + false + else: + true + proc loadKeystore(validatorsDir, secretsDir, keyName: string, nonInteractive: bool): Option[ValidatorPrivKey] = let @@ -92,12 +131,17 @@ proc loadKeystore(validatorsDir, secretsDir, keyName: string, let passphrasePath = secretsDir / keyName if fileExists(passphrasePath): - let - passphrase = KeystorePass: - try: readFile(passphrasePath) - except IOError as err: - error "Failed to read passphrase file", err = err.msg, path = passphrasePath - return + if not(checkFilePermissions(passphrasePath)): + error "Password file has insecure permissions", key_path = keyStorePath + return + + let passphrase = KeystorePass: + try: + readFile(passphrasePath) + except IOError as err: + error "Failed to read passphrase file", err = err.msg, + path = passphrasePath + return let res = decryptKeystore(keystore, passphrase) if res.isOk: @@ -174,29 +218,11 @@ type proc loadNetKeystore*(keyStorePath: string, insecurePwd: Option[string]): Option[lcrypto.PrivateKey] = - when defined(windows): - # Windows do not support per-user permissions, skiping verification part. - discard - else: - let allowedMask = {UserRead, UserWrite} - let mask = {UserExec, - GroupRead, GroupWrite, GroupExec, - OtherRead, OtherWrite, OtherExec} - let pres = getPermissionsSet(keyStorePath) - if pres.isErr(): - error "Could not check key file permissions", - key_path = keyStorePath, errorCode = $pres.error, - errorMsg = ioErrorMsg(pres.error) - return - let insecurePermissions = pres.get() * mask - if insecurePermissions != {}: - error "Network key file has insecure permissions", - key_path = keyStorePath, - insecure_permissions = $insecurePermissions, - current_permissions = pres.get().toString(), - required_permissions = allowedMask.toString() - return + if not(checkFilePermissions(keystorePath)): + error "Network keystorage file has insecure permissions", + key_path = keyStorePath + return let keyStore = try: @@ -313,18 +339,28 @@ proc saveKeystore(rng: var BrHmacDrbgContext, password, signingKeyPath) keystoreFile = validatorDir / keystoreFileName - try: createDir validatorDir - except OSError, IOError: return err FailedToCreateValidatorDir + var encodedStorage: string + try: + encodedStorage = Json.encode(keyStore) + except SerializationError: + error "Could not serialize keystorage", key_path = keystoreFile + return err(FailedToCreateKeystoreFile) - try: createDir secretsDir - except OSError, IOError: return err FailedToCreateSecretsDir + let vres = createPath(validatorDir, 0o750) + if vres.isErr(): + return err(FailedToCreateValidatorDir) - try: writeFile(secretsDir / keyName, password.string) - except IOError: return err FailedToCreateSecretFile + let sres = createPath(secretsDir, 0o750) + if sres.isErr(): + return err(FailedToCreateSecretsDir) - try: Json.saveFile(keystoreFile, keyStore) - except IOError, SerializationError: - return err FailedToCreateKeystoreFile + let swres = writeFile(secretsDir / keyName, string(password), 0o600) + if swres.isErr(): + return err(FailedToCreateSecretFile) + + let kwres = writeFile(keystoreFile, encodedStorage, 0o600) + if kwres.isErr(): + return err(FailedToCreateKeystoreFile) ok() @@ -359,19 +395,18 @@ proc generateDeposits*(preset: RuntimePreset, ok deposits proc saveWallet*(wallet: Wallet, outWalletPath: string): Result[void, string] = - try: createDir splitFile(outWalletPath).dir - except OSError, IOError: - let e = getCurrentException() - return err("failure to create wallet directory: " & e.msg) - - try: Json.saveFile(outWalletPath, wallet, pretty = true) - except IOError as e: - return err("failure to write file: " & e.msg) - except SerializationError as e: - # TODO: Saving a wallet should not produce SerializationErrors. - # Investigate the source of this exception. - return err("failure to serialize wallet: " & e.formatMsg("wallet")) - + let walletDir = splitFile(outWalletPath).dir + var encodedWallet: string + try: + encodedWallet = Json.encode(wallet, pretty = true) + except SerializationError: + return err("Could not serialize wallet") + let pres = createPath(walletDir, 0o750) + if pres.isErr(): + return err("Could not create wallet directory [" & walletDir & "]") + let wres = writeFile(outWalletPath, encodedWallet, 0o600) + if wres.isErr(): + return err("Could not write wallet to file [" & outWalletPath & "]") ok() proc saveWallet*(wallet: WalletPathPair): Result[void, string] = @@ -424,14 +459,15 @@ proc importKeystoresFromDir*(rng: var BrHmacDrbgContext, if toLowerAscii(ext) != ".json": continue - let keystore = try: - Json.loadFile(file, Keystore) - except SerializationError as e: - warn "Invalid keystore", err = e.formatMsg(file) - continue - except IOError as e: - warn "Failed to read keystore file", file, err = e.msg - continue + let keystore = + try: + Json.loadFile(file, Keystore) + except SerializationError as e: + warn "Invalid keystore", err = e.formatMsg(file) + continue + except IOError as e: + warn "Failed to read keystore file", file, err = e.msg + continue var firstDecryptionAttempt = true @@ -643,8 +679,10 @@ proc restoreWalletInteractively*(rng: var BrHmacDrbgContext, proc loadWallet*(fileName: string): Result[Wallet, string] = try: ok Json.loadFile(fileName, Wallet) - except CatchableError as e: - err e.msg + except IOError as exc: + err exc.msg + except SerializationError as exc: + err exc.msg proc unlockWalletInteractively*(wallet: Wallet): Result[Mnemonic, string] = echo "Please enter the password for unlocking the wallet" @@ -666,7 +704,8 @@ proc unlockWalletInteractively*(wallet: Wallet): Result[Mnemonic, string] = return err "failure to unlock wallet" -proc findWallet*(config: BeaconNodeConf, name: WalletName): Result[WalletPathPair, string] = +proc findWallet*(config: BeaconNodeConf, + name: WalletName): Result[WalletPathPair, string] = var walletFiles = newSeq[string]() try: diff --git a/beacon_chain/nimbus_binary_common.nim b/beacon_chain/nimbus_binary_common.nim index 113ddd7b9..44fff3218 100644 --- a/beacon_chain/nimbus_binary_common.nim +++ b/beacon_chain/nimbus_binary_common.nim @@ -18,7 +18,7 @@ import # Local modules spec/[datatypes, crypto, helpers], eth2_network, time -proc setupLogging*(logLevel: string, logFile: Option[OutFile]) = +proc setupStdoutLogging*(logLevel: string) = when compiles(defaultChroniclesStream.output.writer): defaultChroniclesStream.outputs[0].writer = proc (logLevel: LogLevel, msg: LogOutputStr) {.gcsafe, raises: [Defect].} = @@ -27,6 +27,7 @@ proc setupLogging*(logLevel: string, logFile: Option[OutFile]) = except IOError as err: logLoggingFailure(cstring(msg), err) +proc setupLogging*(logLevel: string, logFile: Option[OutFile]) = randomize() if logFile.isSome: