From e957b3c5523dea2b4bfc4c24e134275495d2579d Mon Sep 17 00:00:00 2001 From: Anthony Potdevin <31413433+apotdevin@users.noreply.github.com> Date: Sat, 28 Nov 2020 12:40:12 +0100 Subject: [PATCH] =?UTF-8?q?chore:=20=F0=9F=94=A7=20encrypted=20macaroon=20?= =?UTF-8?q?(#173)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 32 ++++++++ server/helpers/__tests__/fileHelpers.test.ts | 31 ++++++++ server/helpers/auth.ts | 5 +- server/helpers/crypto.ts | 25 +++++++ server/helpers/fileHelpers.ts | 58 ++++++++++----- server/schema/auth/resolvers.ts | 77 ++++++++++++++------ server/schema/types.ts | 2 +- server/tests/testMocks.ts | 2 + server/types/apiTypes.ts | 12 +-- server/types/ln-service.types.ts | 1 + src/graphql/types.ts | 2 +- src/views/homepage/Accounts.tsx | 4 + src/views/homepage/Login.tsx | 31 ++------ 13 files changed, 198 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index 4aaa5b72..1d62cee9 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,38 @@ accounts: If you don't specify `defaultNetwork` then `mainnet` is used as the default. +#### Encrypted Macaroons + +You can use AES encrypted macaroons and have ThunderHub decrypt them and store them in memory. This allows you to have encrypted macaroons on your server and avoid having them in cleartext. + +Macaroons should be AES encrypted. This is an example for Javascript: + +```js +const encrypted = CryptoJS.AES.encrypt( + 'Hex or Base64 encoded Macaroon', + 'Secret Passphrase' +).toString(); +``` + +You can use the `macaroonPath` field and let ThunderHub look for the file or directly use the `macaroon` field and paste your encrypted macaroon. + +You must let ThunderHub know that the macaroon is encrypted by adding an `encrypted` field to your account like such: + +```yaml +masterPassword: 'password' +accounts: + - name: 'Account 1' + serverUrl: 'url:port' + macaroonPath: '/path/to/encrypted.admin.macaroon' + encrypted: true # This field is necessary + - name: 'Account 2' + serverUrl: 'url:port' + macaroon: 'EnCrYpTeD-MaCaRoOn' + encrypted: true # This field is necessary +``` + +To login you must use the same secret passphrase that you used to encrypt the macaroon. + #### Security On the first start of the server, the `masterPassword` and all account `password` fields will be **hashed** and the file will be overwritten with these new values to avoid having cleartext passwords on the server. diff --git a/server/helpers/__tests__/fileHelpers.test.ts b/server/helpers/__tests__/fileHelpers.test.ts index f2004314..5636bc93 100644 --- a/server/helpers/__tests__/fileHelpers.test.ts +++ b/server/helpers/__tests__/fileHelpers.test.ts @@ -4,6 +4,7 @@ import { getParsedAccount, hashPasswords, getAccountsFromYaml, + AccountType, } from '../fileHelpers'; const mockedExistsSync: jest.Mock = existsSync as any; @@ -212,6 +213,36 @@ describe('getParsedAccount', () => { }); }); +describe('encrypted accounts', () => { + it('returns correct props when encrypted', () => { + const raw: AccountType = { + name: 'NAME', + certificate: 'RAW CERT', + serverUrl: 'server.url', + macaroon: 'RAW MACAROON', + encrypted: true, + }; + + const account = getParsedAccount(raw, 0, 'master password', 'mainnet'); + expect(account?.encrypted).toBeTruthy(); + expect(account?.macaroon).toBe(raw.macaroon); + expect((account as any)?.encryptedMacaroon).toBe(raw.macaroon); + }); + it('returns correct props when not encrypted', () => { + const raw = { + lndDir: 'LND DIR', + name: 'NAME', + certificate: 'RAW CERT', + serverUrl: 'server.url', + macaroon: 'RAW MACAROON', + }; + + const account = getParsedAccount(raw, 0, 'master password', 'mainnet'); + expect(account?.encrypted).toBeFalsy(); + expect(account).not.toHaveProperty('encryptedMacaroon'); + }); +}); + describe('hashPasswords', () => { it('does not throw on missing master password', () => { const config = { diff --git a/server/helpers/auth.ts b/server/helpers/auth.ts index 482db851..d408bce0 100644 --- a/server/helpers/auth.ts +++ b/server/helpers/auth.ts @@ -1,7 +1,8 @@ import { authenticatedLndGrpc } from 'ln-service'; -import { SSOType, AccountType } from 'server/types/apiTypes'; +import { SSOType } from 'server/types/apiTypes'; import { LndObject } from 'server/types/ln-service.types'; import { v5 as uuidv5 } from 'uuid'; +import { ParsedAccount } from './fileHelpers'; import { logger } from './logger'; type LndAuthType = { @@ -18,7 +19,7 @@ export const getUUID = (text: string): string => export const getAuthLnd = ( id: string, sso: SSOType | null, - accounts: AccountType[] + accounts: ParsedAccount[] ): LndObject | null => { if (!id) { logger.silly('Account not authenticated'); diff --git a/server/helpers/crypto.ts b/server/helpers/crypto.ts index b890aa77..5bc7036b 100644 --- a/server/helpers/crypto.ts +++ b/server/helpers/crypto.ts @@ -1,6 +1,11 @@ import { createHash, randomBytes } from 'crypto'; import * as bip39 from 'bip39'; import * as bip32 from 'bip32'; +import AES from 'crypto-js/aes'; +import Utf8 from 'crypto-js/enc-utf8'; +import bcrypt from 'bcryptjs'; +import { PRE_PASS_STRING } from './fileHelpers'; +import { logger } from './logger'; export const getPreimageAndHash = () => { const preimage = randomBytes(32); @@ -26,3 +31,23 @@ export const getPrivateAndPublicKey = () => { return { privateKey, publicKey }; }; + +export const decodeMacaroon = (macaroon: string, password: string) => { + try { + return AES.decrypt(macaroon, password).toString(Utf8); + } catch (err) { + logger.error(`Error decoding macaroon with password: ${password}`); + throw new Error('WrongPasswordForLogin'); + } +}; + +export const hashPassword = (password: string): string => + `${PRE_PASS_STRING}${bcrypt.hashSync(password, 12)}`; + +export const isCorrectPassword = ( + password: string, + correctPassword: string +): boolean => { + const cleanPassword = correctPassword.replace(PRE_PASS_STRING, ''); + return bcrypt.compareSync(password, cleanPassword); +}; diff --git a/server/helpers/fileHelpers.ts b/server/helpers/fileHelpers.ts index 065d1846..1cdde02e 100644 --- a/server/helpers/fileHelpers.ts +++ b/server/helpers/fileHelpers.ts @@ -4,14 +4,13 @@ import path from 'path'; import os from 'os'; import { logger } from 'server/helpers/logger'; import yaml from 'js-yaml'; -import bcrypt from 'bcryptjs'; -import { AccountType as ContextAccountType } from 'server/types/apiTypes'; import { getUUID } from './auth'; +import { hashPassword } from './crypto'; type EncodingType = 'hex' | 'utf-8'; type BitcoinNetwork = 'mainnet' | 'regtest' | 'testnet'; -type AccountType = { +export type AccountType = { name?: string; serverUrl?: string; lndDir?: string; @@ -21,15 +20,27 @@ type AccountType = { password?: string | null; macaroon?: string; certificate?: string; + encrypted?: boolean; }; -type ParsedAccount = { + +export type ParsedAccount = { name: string; id: string; socket: string; macaroon: string; cert: string; password: string; -}; +} & EncryptedAccount; + +type EncryptedAccount = + | { + encrypted: true; + encryptedMacaroon: string; + } + | { + encrypted: false; + }; + type AccountConfigType = { hashed: boolean | null; masterPassword: string | null; @@ -124,10 +135,7 @@ export const hashPasswords = ( hashedMasterPassword.indexOf(PRE_PASS_STRING) < 0 ) { hasChanged = true; - hashedMasterPassword = `${PRE_PASS_STRING}${bcrypt.hashSync( - hashedMasterPassword, - 12 - )}`; + hashedMasterPassword = hashPassword(hashedMasterPassword); } cloned.masterPassword = hashedMasterPassword; @@ -141,10 +149,7 @@ export const hashPasswords = ( if (hashedPassword.indexOf(PRE_PASS_STRING) < 0) { hasChanged = true; - hashedPassword = `${PRE_PASS_STRING}${bcrypt.hashSync( - account.password, - 12 - )}`; + hashedPassword = hashPassword(account.password); } hashedAccounts.push({ ...account, password: hashedPassword }); @@ -181,7 +186,7 @@ const getCertificate = ({ }; const getMacaroon = ( - { macaroon, macaroonPath, network, lndDir }: AccountType, + { macaroon, macaroonPath, network, lndDir, encrypted }: AccountType, defaultNetwork: BitcoinNetwork ): string | null => { if (macaroon) { @@ -189,7 +194,7 @@ const getMacaroon = ( } if (macaroonPath) { - return readFile(macaroonPath); + return readFile(macaroonPath, encrypted ? 'utf-8' : 'hex'); } if (!lndDir) { @@ -208,7 +213,7 @@ const getMacaroon = ( ); }; -export const getAccounts = (filePath: string): ContextAccountType[] => { +export const getAccounts = (filePath: string): ParsedAccount[] => { if (filePath === '') { logger.verbose('No account config file path provided'); return []; @@ -219,7 +224,7 @@ export const getAccounts = (filePath: string): ContextAccountType[] => { logger.info(`No account config file found at path ${filePath}`); return []; } - return getAccountsFromYaml(accountConfig, filePath) as ContextAccountType[]; + return getAccountsFromYaml(accountConfig, filePath); }; export const getParsedAccount = ( @@ -236,6 +241,7 @@ export const getParsedAccount = ( macaroonPath, macaroon: macaroonValue, password, + encrypted, } = account; const missingFields: string[] = []; @@ -280,6 +286,17 @@ export const getParsedAccount = ( const id = getUUID(`${name}${serverUrl}${macaroon}${cert}`); + let encryptedProps: EncryptedAccount = { + encrypted: false, + }; + + if (encrypted) { + encryptedProps = { + encrypted: true, + encryptedMacaroon: macaroon, + }; + } + return { name: name || '', id, @@ -287,18 +304,19 @@ export const getParsedAccount = ( macaroon, cert: cert || '', password: password || masterPassword || '', + ...encryptedProps, }; }; export const getAccountsFromYaml = ( config: AccountConfigType, filePath: string -) => { +): ParsedAccount[] => { const { hashed, accounts: preAccounts } = config; if (!preAccounts || preAccounts.length <= 0) { logger.warn(`Account config found at path ${filePath} but had no accounts`); - return null; + return []; } const { defaultNetwork, masterPassword, accounts } = hashPasswords( @@ -323,7 +341,7 @@ export const getAccountsFromYaml = ( .join(', ')}` ); - return parsedAccounts; + return parsedAccounts as ParsedAccount[]; }; export const readMacaroons = (macaroonPath: string): string | null => { diff --git a/server/schema/auth/resolvers.ts b/server/schema/auth/resolvers.ts index e4ad29c9..23e6e989 100644 --- a/server/schema/auth/resolvers.ts +++ b/server/schema/auth/resolvers.ts @@ -1,16 +1,15 @@ import getConfig from 'next/config'; import jwt from 'jsonwebtoken'; -import { - readCookie, - refreshCookie, - PRE_PASS_STRING, -} from 'server/helpers/fileHelpers'; +import { readCookie, refreshCookie } from 'server/helpers/fileHelpers'; import { ContextType } from 'server/types/apiTypes'; import { logger } from 'server/helpers/logger'; import cookie from 'cookie'; import { requestLimiter } from 'server/helpers/rateLimiter'; -import bcrypt from 'bcryptjs'; import { appConstants } from 'server/utils/appConstants'; +import { GetWalletInfoType } from 'server/types/ln-service.types'; +import { authenticatedLndGrpc, getWalletInfo } from 'ln-service'; +import { toWithError } from 'server/helpers/async'; +import { decodeMacaroon, isCorrectPassword } from 'server/helpers/crypto'; const { serverRuntimeConfig } = getConfig() || {}; const { cookiePath, nodeEnv } = serverRuntimeConfig || {}; @@ -69,32 +68,58 @@ export const authResolvers = { }, getSessionToken: async ( _: undefined, - params: any, - context: ContextType - ) => { - const { ip, secret, res } = context; + { id, password }: { id: string; password: string }, + { ip, secret, res, accounts }: ContextType + ): Promise => { await requestLimiter(ip, 'getSessionToken'); - const account = context.accounts.find(a => a.id === params.id) || null; + const account = accounts.find(a => a.id === id) || null; if (!account) { - logger.debug(`Account ${params.id} not found`); - return null; + logger.debug(`Account ${id} not found`); + return ''; } - const cleanPassword = account.password.replace(PRE_PASS_STRING, ''); + if (account.encrypted) { + if (nodeEnv === 'development') { + logger.error( + 'Encrypted accounts only work in a production environment' + ); + throw new Error('UnableToLogin'); + } - const isPassword = bcrypt.compareSync(params.password, cleanPassword); + const macaroon = decodeMacaroon(account.encryptedMacaroon, password); - if (!isPassword) { - logger.error( - `Authentication failed from ip: ${ip} - Invalid Password!` - ); - throw new Error('WrongPasswordForLogin'); + // Store decrypted macaroon in memory. + // In development NextJS rebuilds the files so this only works in production env. + account.macaroon = macaroon; + + logger.debug(`Decrypted the macaroon for account ${id}`); + } else { + if (!isCorrectPassword(password, account.password)) { + logger.error( + `Authentication failed from ip: ${ip} - Invalid Password!` + ); + throw new Error('WrongPasswordForLogin'); + } + + logger.debug(`Correct password for account ${id}`); } - logger.debug(`Correct password for account ${params.id}`); - const token = jwt.sign({ id: params.id }, secret); + // Try to connect to node. The authenticatedLndGrpc method will also check if the macaroon is base64 or hex. + const { lnd } = authenticatedLndGrpc(account); + const [info, error] = await toWithError( + getWalletInfo({ + lnd, + }) + ); + + if (error) { + logger.error('Unable to connect to this node'); + throw new Error('UnableToConnectToThisNode'); + } + + const token = jwt.sign({ id }, secret); res.setHeader( 'Set-Cookie', cookie.serialize(appConstants.cookieName, token, { @@ -103,11 +128,15 @@ export const authResolvers = { path: '/', }) ); - return true; + return info?.version || ''; }, }, Mutation: { - logout: async (_: undefined, __: any, context: ContextType) => { + logout: async ( + _: undefined, + __: any, + context: ContextType + ): Promise => { const { ip, res } = context; await requestLimiter(ip, 'logout'); diff --git a/server/schema/types.ts b/server/schema/types.ts index fc9ec19e..e112f65b 100644 --- a/server/schema/types.ts +++ b/server/schema/types.ts @@ -86,7 +86,7 @@ export const queryTypes = gql` lastMessage: String ): getMessagesType getAuthToken(cookie: String): Boolean! - getSessionToken(id: String, password: String): Boolean + getSessionToken(id: String, password: String): String! getServerAccounts: [serverAccountType] getAccount: serverAccountType getLatestVersion: String diff --git a/server/tests/testMocks.ts b/server/tests/testMocks.ts index 827c6e54..b5c909c5 100644 --- a/server/tests/testMocks.ts +++ b/server/tests/testMocks.ts @@ -19,6 +19,7 @@ export const ContextMock: ContextType = { macaroon: 'macaroon', cert: 'cert', password: 'password', + encrypted: false, }, ], res: {} as ServerResponse, @@ -54,6 +55,7 @@ export const ContextMockNoSSO: ContextType = { macaroon: 'macaroon', cert: 'cert', password: 'password', + encrypted: false, }, ], res: {} as ServerResponse, diff --git a/server/types/apiTypes.ts b/server/types/apiTypes.ts index 8c714b42..fc83ee85 100644 --- a/server/types/apiTypes.ts +++ b/server/types/apiTypes.ts @@ -1,4 +1,5 @@ import { ServerResponse } from 'http'; +import { ParsedAccount } from 'server/helpers/fileHelpers'; import { LndObject } from './ln-service.types'; export type SSOType = { @@ -7,22 +8,13 @@ export type SSOType = { socket: string; }; -export type AccountType = { - name: string; - id: string; - socket: string; - macaroon: string; - cert: string | null; - password: string; -}; - export type ContextType = { ip: string; lnd: LndObject | null; secret: string; id: string | null; sso: SSOType | null; - accounts: AccountType[]; + accounts: ParsedAccount[]; res: ServerResponse; lnMarketsAuth: string | null; }; diff --git a/server/types/ln-service.types.ts b/server/types/ln-service.types.ts index b8737c5c..635bbcd2 100644 --- a/server/types/ln-service.types.ts +++ b/server/types/ln-service.types.ts @@ -101,6 +101,7 @@ export type ForwardType = { export type GetWalletInfoType = { alias: string; public_key: string; + version: string; }; export type DiffieHellmanComputeSecretType = { diff --git a/src/graphql/types.ts b/src/graphql/types.ts index f1b3ac6b..c85ba330 100644 --- a/src/graphql/types.ts +++ b/src/graphql/types.ts @@ -84,7 +84,7 @@ export type Query = { getUtxos?: Maybe>>; getMessages?: Maybe; getAuthToken: Scalars['Boolean']; - getSessionToken?: Maybe; + getSessionToken: Scalars['String']; getServerAccounts?: Maybe>>; getAccount?: Maybe; getLatestVersion?: Maybe; diff --git a/src/views/homepage/Accounts.tsx b/src/views/homepage/Accounts.tsx index 6dd95966..f102b108 100644 --- a/src/views/homepage/Accounts.tsx +++ b/src/views/homepage/Accounts.tsx @@ -10,6 +10,7 @@ import { Link } from 'src/components/link/Link'; import { useGetServerAccountsQuery } from 'src/graphql/queries/__generated__/getServerAccounts.generated'; import { ServerAccountType } from 'src/graphql/types'; import { LoadingCard } from 'src/components/loading/LoadingCard'; +import { useLogoutMutation } from 'src/graphql/mutations/__generated__/logout.generated'; import { Section } from '../../components/section/Section'; import { Card, @@ -78,6 +79,8 @@ export const Accounts = () => { null ); + const [logout] = useLogoutMutation({ refetchQueries: ['GetServerAccounts'] }); + React.useEffect(() => { prefetch(appendBasePath('/home')); }, [prefetch]); @@ -91,6 +94,7 @@ export const Accounts = () => { fetchPolicy: 'network-only', onError: () => { toast.error('Unable to connect to this node'); + logout(); }, }); diff --git a/src/views/homepage/Login.tsx b/src/views/homepage/Login.tsx index 31243625..c283dd1c 100644 --- a/src/views/homepage/Login.tsx +++ b/src/views/homepage/Login.tsx @@ -3,13 +3,11 @@ import { toast } from 'react-toastify'; import styled from 'styled-components'; import { useRouter } from 'next/router'; import { appendBasePath } from 'src/utils/basePath'; -import { useGetCanConnectLazyQuery } from 'src/graphql/queries/__generated__/getNodeInfo.generated'; import { useGetSessionTokenLazyQuery } from 'src/graphql/queries/__generated__/getSessionToken.generated'; import { getErrorContent } from 'src/utils/error'; import { Lock } from 'react-feather'; import { ServerAccountType } from 'src/graphql/types'; import { getVersion } from 'src/utils/version'; -import { useLogoutMutation } from 'src/graphql/mutations/__generated__/logout.generated'; import { SingleLine, Sub4Title, Card } from '../../components/generic/Styled'; import { ColorButton } from '../../components/buttons/colorButton/ColorButton'; import { Input } from '../../components/input'; @@ -43,19 +41,7 @@ export const Login = ({ account }: LoginProps) => { const [pass, setPass] = useState(''); - const [getCanConnect, { data, loading }] = useGetCanConnectLazyQuery({ - fetchPolicy: 'network-only', - onError: err => { - toast.error(getErrorContent(err)); - }, - }); - - const [logout] = useLogoutMutation(); - - const [ - getSessionToken, - { data: sData, loading: sLoading }, - ] = useGetSessionTokenLazyQuery({ + const [getSessionToken, { data, loading }] = useGetSessionTokenLazyQuery({ fetchPolicy: 'network-only', onError: err => { toast.error(getErrorContent(err)); @@ -63,23 +49,16 @@ export const Login = ({ account }: LoginProps) => { }); useEffect(() => { - if (!sLoading && sData && sData.getSessionToken) { - account && getCanConnect(); - } - }, [sLoading, sData, push, getCanConnect, account]); - - useEffect(() => { - if (loading || !data?.getNodeInfo?.version) return; - const { mayor, minor } = getVersion(data.getNodeInfo.version); + if (loading || !data?.getSessionToken) return; + const { mayor, minor } = getVersion(data.getSessionToken); if (mayor <= 0 && minor < 11) { toast.error( 'ThunderHub supports LND version 0.11.0 and higher. Please update your node, you are in risk of losing funds.' ); - logout(); } else { push(appendBasePath('/home')); } - }, [data, loading, pass, account, push, logout]); + }, [data, loading, push]); if (!account) return null; @@ -113,7 +92,7 @@ export const Login = ({ account }: LoginProps) => { onClick={() => handleEnter()} withMargin={'16px 0 0'} fullWidth={true} - loading={loading || sLoading} + loading={loading} > Connect