Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/remote key manager #202

Open
wants to merge 17 commits into
base: feature/p2pmessaging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"jsdom-global/register",
"--allow-uncaught",
"--colors",
"--timeout 50000",
"--timeout 100000",
// "--reporter",
// "mocha-reporter",
"${workspaceFolder}/src/test/*.ts"
Expand All @@ -35,7 +35,7 @@
"request": "attach",
"port": 9229,
"protocol": "inspector",
"timeout": 30000,
"timeout": 100000,
"stopOnEntry": false
}
]
Expand Down
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
},
"mochaExplorer.files": ["src/test/*.ts", "src/test/crux-messenger/*.ts", "src/test/integration-tests/crux-messenger/*.ts"],
"mochaExplorer.require": ["ts-node/register", "mock-local-storage", "jsdom-global/register"],
"mochaExplorer.timeout": 50000,
"mochaExplorer.timeout": 100000,
"testExplorer.codeLens": true,
"testExplorer.gutterDecoration": true,
"testExplorer.onStart": "reset",
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
"test": "TS_NODE_PROJECT='./src/test/tsconfig.commonjs.json' TS_NODE_TRANSPILE_ONLY=true ./node_modules/.bin/mocha --exit --require ts-node/register --require mock-local-storage --require jsdom-global/register --allow-uncaught --colors --reporter mocha-reporter --timeout 5000 src/test/*.ts src/test/crux-messenger/*.ts",
"start-bridge-server": "node dist/crux-gateway-bridge-without-auth.js &",
"stop-bridge-server": "pkill -- signal SIGINT crux-gateway-bridge-server-without-auth",
"integration-test": "TS_NODE_PROJECT='./src/test/tsconfig.commonjs.json' TS_NODE_TRANSPILE_ONLY=true ./node_modules/.bin/mocha --exit --require ts-node/register --require mock-local-storage --require jsdom-global/register --allow-uncaught --colors --reporter mocha-reporter --timeout 50000 src/test/integration-tests/crux-messenger/*.ts",
"integration-test": "TS_NODE_PROJECT='./src/test/tsconfig.commonjs.json' TS_NODE_TRANSPILE_ONLY=true ./node_modules/.bin/mocha --exit --require ts-node/register --require mock-local-storage --require jsdom-global/register --allow-uncaught --colors --reporter mocha-reporter --timeout 80000 src/test/integration-tests/crux-messenger/*.ts",
"integration": "npm run build-crux-bridge-server-without-auth && npm run start-bridge-server && npm run integration-test && npm run stop-bridge-server",
"copy-latest-docs": "cp -a docs/$npm_package_version/. docs/",
"version-docs": "./node_modules/.bin/typedoc --out docs/$npm_package_version src/index.ts",
"coverage": "./node_modules/.bin/nyc npm run test",
"wallet_demo_legacy": "./node_modules/.bin/parcel src/samples/wallet_demo_legacy.html --https --no-cache",
"wallet_demo": "./node_modules/.bin/parcel src/samples/wallet_demo.html --no-cache",
"wallet_demo": "./node_modules/.bin/parcel src/samples/wallet_demo.html --no-cache &",
"remote-crux-settings": "npm run wallet_demo && ./node_modules/.bin/parcel src/samples/remote-crux-settings.html --no-cache",
"onboarding_demo": "./node_modules/.bin/parcel src/samples/onboarding_demo.html --https --no-cache",
"typecheck": "./node_modules/.bin/tsc --noEmit",
"validate_lockfile": "./node_modules/.bin/lockfile-lint --type npm --path package-lock.json --allowed-hosts npm github.com --allowed-schemes \"https:\" \"git+https:\"",
Expand Down
2 changes: 1 addition & 1 deletion src/application/clients/crux-gateway-bridge-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"BROKER_HOST": "127.0.0.1"
},
"PORTS": {
"BROKER_PORT": 1883,
"BROKER_PORT": 8000,
"TCP_PORT": 4005
}
}
28 changes: 28 additions & 0 deletions src/application/clients/crux-service-client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { CruxProtocolMessenger, RemoteKeyManager} from "../../core/domain-services";
import { keyManagementProtocol } from "../../infrastructure";
import { CruxId, InMemStorage } from "../../packages/";
import { CruxWalletClient } from "./crux-wallet-client";

export class CruxServiceClient {
private selfIdClaim: any;
private cruxProtocolMessenger: any;

constructor(selfIdClaim: any, secureCruxNetwork: any, protocolSchema: any) {
this.selfIdClaim = selfIdClaim;
this.cruxProtocolMessenger = new CruxProtocolMessenger(secureCruxNetwork, protocolSchema);
}

public async getWalletClientForUser(remoteUserId: CruxId) {
await this.cruxProtocolMessenger.initialize();
const remoteKeyManager = new RemoteKeyManager(this.cruxProtocolMessenger, remoteUserId);

await remoteKeyManager.initialize();
return new CruxWalletClient({
cacheStorage: new InMemStorage(),
disableCruxMessenger: true,
// @ts-ignore
privateKey: remoteKeyManager,
walletClientName: remoteUserId.components.domain,
quixote911 marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
27 changes: 21 additions & 6 deletions src/application/clients/crux-wallet-client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Importing packages
import Logger from "js-logger";
import {CruxProtocolMessenger, SecureCruxNetwork} from "../../core/domain-services";
import {CruxProtocolMessenger, RemoteKeyHost, SecureCruxNetwork} from "../../core/domain-services";
import {
CruxDomain,
CruxSpec,
Expand All @@ -26,6 +26,7 @@ import {
CruxNetPubSubClientFactory, cruxPaymentProtocol,
IBlockstackCruxDomainRepositoryOptions,
IBlockstackCruxUserRepositoryOptions,
keyManagementProtocol,
} from "../../infrastructure/implementations";
import {CruxDomainId, CruxId, getLogger, InMemStorage, StorageService} from "../../packages";
import {Encryption} from "../../packages/encryption";
Expand Down Expand Up @@ -79,6 +80,7 @@ export interface ICruxWalletClientOptions {
cacheStorage?: StorageService;
walletClientName: string;
debugLogging?: boolean;
disableCruxMessenger?: boolean;
}

export interface ICruxIDState {
Expand All @@ -96,7 +98,7 @@ export const getCruxUserRepository = (options: IBlockstackCruxUserRepositoryOpti

export const getPubsubClientFactory = (): IPubSubClientFactory => {
return new CruxNetPubSubClientFactory({defaultLinkServer: {
host: "broker.hivemq.com",
host: "127.0.0.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change this in code. In code we should have a globally accessible server.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we put it in config??

Copy link
Member

@quixote911 quixote911 Jun 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ideally do it in the same way we've done it for Blockstack user repository. It also requires such a default for Blockstack stuff.

Look at line 127 of crux-wallet-client.ts

this.cruxBlockstackInfrastructure = options.blockstackInfrastructure || CruxSpec.blockstack.infrastructure;

Defaults are defined in CruxSpec. just read directly from there. No need to do the options.blockstackInfrastructure || part.

path: "/mqtt",
port: 8000,
}});
Expand All @@ -107,6 +109,7 @@ export class CruxWalletClient {
public walletClientName: string;
// TODO: make private
public paymentProtocolMessenger?: CruxProtocolMessenger;
public keyManagerProtocolMessenger?: CruxProtocolMessenger;
public secureCruxNetwork?: SecureCruxNetwork;
private cruxBlockstackInfrastructure: ICruxBlockstackInfrastructure;
private initPromise: Promise<void>;
Expand All @@ -119,6 +122,7 @@ export class CruxWalletClient {
private resolvedClientAssetMapping?: IResolvedClientAssetMap;
private cacheStorage?: StorageService;
private selfCruxUser?: CruxUser;
private remoteKeyHost?: RemoteKeyHost;

constructor(options: ICruxWalletClientOptions) {
getLogger(cruxWalletClientDebugLoggerName).setLevel(options.debugLogging ? Logger.DEBUG : Logger.OFF);
Expand All @@ -136,7 +140,7 @@ export class CruxWalletClient {
}
this.cruxDomainRepo = getCruxDomainRepository({cacheStorage: this.cacheStorage, blockstackInfrastructure: this.cruxBlockstackInfrastructure});
this.cruxDomainId = new CruxDomainId(this.walletClientName);
this.initPromise = this.asyncInit();
this.initPromise = this.asyncInit(options);
}

@throwCruxClientError
Expand Down Expand Up @@ -423,7 +427,7 @@ export class CruxWalletClient {
return this.cruxDomain;
}

private asyncInit = async (): Promise<void> => {
private asyncInit = async (options: ICruxWalletClientOptions): Promise<void> => {
this.cruxDomain = await this.cruxDomainRepo.get(this.cruxDomainId);
if (!this.cruxDomain) {
throw ErrorHelper.getPackageError(null, PackageErrorCode.InvalidWalletClientName);
Expand All @@ -433,9 +437,16 @@ export class CruxWalletClient {
throw ErrorHelper.getPackageError(null, PackageErrorCode.CouldNotFindBlockstackConfigurationServiceClientConfig);
}
this.cruxAssetTranslator = new CruxAssetTranslator(this.cruxDomain.config.assetMapping, this.cruxDomain.config.assetList);
if (options.disableCruxMessenger) {
return;
}
const selfIdClaim = await this.getSelfClaim();
if (selfIdClaim) {
await this.setupCruxMessenger(selfIdClaim);
try {
await this.setupCruxMessenger(selfIdClaim);
} catch (err) {
console.log(err);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this? Remove if it was for your personal debugging only.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it was for catching errors from setupCruxMessenger but now we can remove it(options are being used till here only)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'catching error' is not a valid purpose. 'catching error to _________' is a valid purpose

  1. Catching error to silence/suppress it
  2. Catching error to transform it and throw it again as another error
  3. Catching error to retry
  4. etc

Here you've suppressed it and logged it. Any particular cases/errors you're trying to catch here? If this function is too failure prone then it may block other functionalities of SDK. So then it may be a good idea to suppress error here like youve done
But in that case we should figure out why its failing and fix that.

}
}
}

Expand All @@ -461,8 +472,12 @@ export class CruxWalletClient {
}
const pubsubClientFactory = getPubsubClientFactory();
this.secureCruxNetwork = new SecureCruxNetwork(this.cruxUserRepository, pubsubClientFactory, selfIdClaim);
await this.secureCruxNetwork.initialize();
this.paymentProtocolMessenger = new CruxProtocolMessenger(this.secureCruxNetwork, cruxPaymentProtocol);
this.keyManagerProtocolMessenger = new CruxProtocolMessenger(this.secureCruxNetwork, keyManagementProtocol);
await this.keyManagerProtocolMessenger.initialize();
const remoteKeyHost = new RemoteKeyHost(this.keyManagerProtocolMessenger, this.keyManager!);
this.remoteKeyHost = remoteKeyHost;
await this.remoteKeyHost.initialize();
}

}
1 change: 1 addition & 0 deletions src/application/clients/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./crux-explorer-client";
export * from "./crux-wallet-client";
export * from "./crux-wallet-onboarding";
export * from "./crux-service-client";
4 changes: 3 additions & 1 deletion src/core/domain-services/crux-messenger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
export class CertificateManager {
public static make = async (idClaim: ICruxIdClaim): Promise<ICruxIdCertificate> => {
const payload = idClaim.cruxId.toString();
console.log("CertificateManager::make:claim: ", payload);
const signedProof = await idClaim.keyManager.signWebToken(payload);
return {
claim: idClaim.cruxId.toString(),
Expand All @@ -28,6 +29,7 @@ export class CertificateManager {
public static verify = (certificate: ICruxIdCertificate, senderPubKey: any) => {
const proof: any = decodeToken(certificate.proof).payload;
const verified = new TokenVerifier("ES256K", senderPubKey).verify(certificate.proof);
console.log("CertificateManager::verify:: verified, claim, proof", verified, certificate.claim, proof);
if (proof && proof === certificate.claim && verified) {
return true;
}
Expand All @@ -45,6 +47,7 @@ export class EncryptionManager {
}
public static decrypt = async (encryptedContent: string, keyManager: IKeyManager): Promise<string> => {
try {
console.log("EncryptionManager::decrypt::keymanager, encryptionContent: ", keyManager, encryptedContent);
const decryptedContent = await keyManager.decryptMessage!(encryptedContent);
return decryptedContent;
} catch (e) {
Expand Down Expand Up @@ -249,7 +252,6 @@ export class SecureContext {
data,
};
const serializedSecurePacket = JSON.stringify(securePacket);

const recipientCruxUser: CruxUser | undefined = await this.cruxUserRepo.getByCruxId(recipientId);
if (!recipientCruxUser) {
throw Error("No Such CRUX User Found");
Expand Down
1 change: 1 addition & 0 deletions src/core/domain-services/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from "./crux-messenger";
export * from "./remote-key-service";
153 changes: 153 additions & 0 deletions src/core/domain-services/remote-key-service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import {makeUUID4} from "blockstack/lib";
import { createNanoEvents, DefaultEvents, Emitter } from "nanoevents";
import { CruxId } from "../../packages";
import { IKeyManager } from "../interfaces";
import { CruxProtocolMessenger, SecureCruxNetwork } from "./crux-messenger";

const VALID_METHODS = ["signWebToken", "getPubKey", "deriveSharedSecret", "decryptMessage"];

export class RemoteKeyClient {
private cruxProtocolMessenger: CruxProtocolMessenger;
private remoteUserId: CruxId;
private emitter: Emitter<DefaultEvents>;

constructor(cruxProtocolMessenger: CruxProtocolMessenger, remoteUserId: CruxId) {
this.cruxProtocolMessenger = cruxProtocolMessenger;
this.remoteUserId = remoteUserId;
this.emitter = createNanoEvents();
}

public async initialize() {
this.cruxProtocolMessenger.on("KEY_MANAGER_RESPONSE", async (msg: any, senderId: CruxId | undefined) => {
console.log("Inside RemoteKeyClient::initialize::Msg, senderId: ", msg, senderId);
this.emitter.emit(msg.invocationId, msg, senderId);
});
}

public async invoke(method: string, args: any[]) {
console.log("RemoteKeyClient::Inside Invoke");
if (!this.cruxProtocolMessenger) {
throw Error("RemoteKeyClient cannot send with no secureCruxNetwork");
}
const methodData = this.generateMethodData(method, args);
console.log("RemoteKeyClient::Inside Invoke, RemoteUserId, MethodData", this.remoteUserId, methodData);
// @ts-ignore
await this.cruxProtocolMessenger.send({
content: methodData,
type: "KEY_MANAGER_REQUEST",
}, this.remoteUserId);
return methodData.invocationId;
}

public listenToInvocation = (invocationId: string, resultCallback: (msg: any, senderId: CruxId | undefined) => any, errorCallback: (err: any) => any): void => {
if (!this.cruxProtocolMessenger) {
throw Error("RemoteKeyClient cannot listen with no secureCruxNetwork");
}
console.log("RemoteKeyClient::ListenToInvocation::invocationId", invocationId);
this.emitter.on(invocationId, resultCallback);
this.emitter.on("error", errorCallback);
}

private generateMethodData(method: string, args: any[]) {
return {
args,
invocationId: makeUUID4(),
method,
};
}
}

export class RemoteKeyHost {
private cruxProtocolMessenger: CruxProtocolMessenger;
private keyManager: IKeyManager;

constructor(cruxProtocolMessenger: CruxProtocolMessenger, keyManager: IKeyManager) {
this.keyManager = keyManager;
this.cruxProtocolMessenger = cruxProtocolMessenger;
}

public async initialize() {
this.cruxProtocolMessenger.on("KEY_MANAGER_REQUEST", async (msg: any, senderId: CruxId | undefined) => {
console.log("Inside RemoteKeyHost::in::Msg, senderId: ", msg, senderId);
const data = await this.handleMessage(msg);
console.log("Inside RemoteKeyHost::initialize::Data(handleMessage): ", data);
// @ts-ignore
await this.sendInvocationResult(data, CruxId.fromString(senderId!)); // getting senderId as string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ID should be coming as object here. If it isn't then it's a bug! Can you fix it?
SecureReceiveSocket will be the right place to do it. Please think about why it's the right place to do it, let me know if you disagree.

Copy link
Member Author

@punto-rojo punto-rojo Jun 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah seems so. will check it again and fix it. And yes SecureRecieveSocket will be best as it will make sure that it's consistent throughout (CruxID)

});
}
private async sendInvocationResult(result: any, receiverId: CruxId) {
if (!this.cruxProtocolMessenger) {
throw Error("RemoteKeyClient cannot send with no selfMessenger");
}
const resultData = this.generateInvocationResponse(result);
console.log("RemoteKeyHost::Inside sendInvocationResult::resultData: ", resultData);
await this.cruxProtocolMessenger.send({
content: resultData,
type: "KEY_MANAGER_RESPONSE",
}, receiverId);
}

private async handleMessage(message: any) {
if (!this.keyManager) {
throw new Error("Key Manager not available");
}
let data;
// @ts-ignore
data = await this.keyManager[message.method](message.args[0]);
return {
data,
invocationId: message.invocationId,
};
}

private generateInvocationResponse(result: any) {
return {
invocationId: result.invocationId,
result,
};
}
}

export class RemoteKeyManager implements IKeyManager {
private remoteKeyClient: RemoteKeyClient;
private remoteUserId: CruxId;

constructor(cruxProtocolMessenger: CruxProtocolMessenger, remoteUserId: CruxId) {
this.remoteKeyClient = new RemoteKeyClient(cruxProtocolMessenger, remoteUserId);
this.remoteUserId = remoteUserId;
}

public async initialize() {
await this.remoteKeyClient.initialize();
}
// @ts-ignore
public signWebToken = async (args: any) => {
return this.makeRemoteMessageCall("signWebToken", [args]);
}
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove ts ignore and fix errors. Let me know if you're getting confused with 'args' and how to make it generic for arguments.

public getPubKey = async () => {
return this.makeRemoteMessageCall("getPubKey");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do an optimization here -
We know the Crux ID. And we're doing secure communication on the sole basis of the fact that we know the ID's Public Key. So we don't need to do makeRemoteMessageCall to getPubKey. You can get a CruxUser from CruxId and get Public Key from there. How to get CruxUser? We need UserRepository. SecureCruxNetwork already has a userRepository, Just that it's a private variable at the moment. You can make it public, and access it via CruxProtocolMessenger->SecureCruxNetwork->userRepository

}
// @ts-ignore
public deriveSharedSecret = async (args: string) => {
return this.makeRemoteMessageCall("deriveSharedSecret", [args]);
}
// @ts-ignore
public decryptMessage = async (args: string) => {
Copy link
Member

@quixote911 quixote911 Jun 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong naming of parameter in all methods - look at other KeyManager implementation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry will take care of it

return this.makeRemoteMessageCall("decryptMessage", [args]);
}

private makeRemoteMessageCall = async (method: string, args: any = []) => {
console.log("makeRemoteMessageCall::", method, args);
const invocationId = await this.remoteKeyClient.invoke(method, args);
console.log("RemoteKeyManager::makeRemoteMessageCall::invokationId: ", invocationId);
return new Promise(async (resolve, reject) => {
this.remoteKeyClient.listenToInvocation(invocationId, (msg, senderId) => {
console.log("RemoteKeyManager::deriveSharedSecret::msg: ", msg);
resolve(msg.result.data);
}, (err) => {
reject(err);
});
});
}
}
1 change: 1 addition & 0 deletions src/core/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from "./entities";
export * from "./interfaces";
export * from "./domain-services";
2 changes: 2 additions & 0 deletions src/infrastructure/implementations/basic-key-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ export class BasicKeyManager implements IKeyManager {
let privateKey = await this.getDecryptedPrivateKey();
const curve = new ec("secp256k1");
const selfKey = curve.keyFromPrivate(privateKey, "hex");
console.log("Inside DeriveSharedSecret: pubKey, pvtKey", publicKey, privateKey);
const userKey = curve.keyFromPublic(publicKey, "hex");
privateKey = "0".repeat(privateKey.length);
return selfKey.derive(userKey.getPublic()).toString(16);
}

public decryptMessage = async (encryptedMessage: string): Promise<string> => {
console.log("Inside DecryptMessage: type, value", typeof encryptedMessage, encryptedMessage);
const toDecrypt = BufferJSONSerializer.JSONStringToBufferObject(encryptedMessage);
const privateKey = await this.getDecryptedPrivateKey();
const decrypted = await ECIESEncryption.decrypt(toDecrypt, privateKey);
Expand Down
Loading