Feature/remote key manager #202

punto-rojo · 2020-05-25T15:24:48Z

No description provided.

…ation for deriveSharedSecret,decryptMessage methods

…ties

…remoteKeyManager, RemoteKeyHost, RemoteKeyClient

src/application/clients/crux-service-client.ts

src/infrastructure/implementations/crux-messenger.ts

quixote911 · 2020-06-04T07:32:09Z

src/application/clients/crux-service-client.ts

+    private selfIdClaim: any;
+    private cruxProtocolMessenger: any;
+
+    constructor(selfIdClaim: any, secureCruxNetwork: any, protocolSchema?: any) {


Doesn't make sense to take protocolSchema as optional parameter. You always need protocol schema to be Key Management Protocol.
Is there any use case you designed this for? If not, remove.

No, it does make sense I guess because CruxServiceClient can give you the different type of clients and some might use protocolSchema some might not

Lets add this when we add more use cases to CruxServiceClient. for now remove.

Please remove protocolSchema from the constructor itself. Making it non optional doesn't help us. The point is that there is only 1 protocolSchema this class can be used with at the moment. does not make sense for caller to have to send it.

Just like in CruxWalletClient, the caller does NOT have any awareness of protocol schema or messenger internals or anything.

Okay i will remove it

src/application/clients/crux-wallet-client.ts

src/infrastructure/implementations/blockstack-crux-user-repository.ts

src/infrastructure/implementations/crux-messenger.ts

src/core/domain-services/remote-key-service.ts

quixote911 · 2020-06-04T07:45:54Z

src/core/domain-services/remote-key-service.ts

+        let data;
+        if (message.method === "signWebToken") {
+            console.log("HandleMessage entrance:signWebToken");
+            data = await this.keyManager.signWebToken(message.args[0]);


what will happen if function has multiple arguments? does args[0] mean only first argument will go?

all functions require single or zero parameters. So args[0] will work

Better way to do it which will work for any function -

const func = (a, b) => { console.log(a+b); } const args = [1,2] // @ts-ignore func(...args);

or

const func = (a, b) => { console.log(a+b); } const args = [1,2] func.apply(this, args);

understood will modify it accordingly

src/core/domain-services/remote-key-service.ts

quixote911 · 2020-06-04T08:51:19Z

Please fix merge conflicts carefully. There are some after I merged Sanchay's branch.

quixote911 · 2020-06-05T07:36:37Z

src/application/clients/crux-wallet-client.ts

@@ -98,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",


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

shall we put it in config??

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.

quixote911 · 2020-06-05T07:46:10Z

src/core/domain-services/remote-key-service.ts

@@ -71,7 +71,8 @@ export class RemoteKeyHost {
            console.log("Inside RemoteKeyHost::in::Msg, senderId: ", msg, senderId);
            const data = await this.handleMessage(msg);
            console.log("Inside RemoteKeyHost::initialize::Data(handleMessage): ", data);
-            this.sendInvocationResult(data, senderId!);
+            // @ts-ignore
+            await this.sendInvocationResult(data, CruxId.fromString(senderId!)); // getting senderId as string


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.

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)

quixote911 · 2020-06-05T07:53:10Z

src/core/domain-services/remote-key-service.ts

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


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

Sorry will take care of it

quixote911 · 2020-06-05T08:16:41Z

src/core/domain-services/remote-key-service.ts

+    }
+    // @ts-ignore
+    public getPubKey = async () => {
+        return this.makeRemoteMessageCall("getPubKey");


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

quixote911 · 2020-06-05T08:17:45Z

src/core/domain-services/remote-key-service.ts

+    public signWebToken = async (args: any) => {
+        return this.makeRemoteMessageCall("signWebToken", [args]);
+    }
+    // @ts-ignore


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

punto-rojo added 14 commits May 25, 2020 16:39

Added RemoteKeyClient and RemoteKeyHost and testcases for the same

e53d002

added RemoteKeyManager and testcases for the same and added implement…

6ad4672

…ation for deriveSharedSecret,decryptMessage methods

defined message schema for RemoteKeyManager requests and responses

736638c

added test for CruxServiceClient

2d8519c

added remote key client test

706be90

added logging for debugging

5275f85

modified cruxServiceClient and updated testcases

7b829d3

added putAddressMap testcase in cruxServiceClient

6ac6520

Merge branch 'feature/p2pmessaging' into feature/remote-key-manager

f1d0e66

modified remoteKeyManager, remoteKeyHost, remoteKeyClient functionali…

4444029

…ties

added testcase for CruxServiceClient for InMemory feature

0fbe28b

added key managementProtocol in CruxWalletClient, CruxServiceClient, …

7a8a5d0

…remoteKeyManager, RemoteKeyHost, RemoteKeyClient

updated CruxWalletClient setupmessenger functionality

838d4bb

added demo for remote-crux-settings

8155a26

punto-rojo commented Jun 4, 2020

View reviewed changes

src/application/clients/crux-service-client.ts Show resolved Hide resolved

quixote911 requested changes Jun 4, 2020

View reviewed changes

punto-rojo added 3 commits June 4, 2020 17:09

fixed PR comments

65fea20

typecheck fixed

0616570

setup local HiveMQ broker and IP change

f85b7e3

quixote911 reviewed Jun 5, 2020

View reviewed changes

quixote911 requested changes Jun 5, 2020

View reviewed changes

quixote911 reviewed Jun 5, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/remote key manager #202

Feature/remote key manager #202

punto-rojo commented May 25, 2020

quixote911 Jun 4, 2020

punto-rojo Jun 4, 2020

quixote911 Jun 4, 2020

punto-rojo Jun 4, 2020

quixote911 Jun 5, 2020

punto-rojo Jun 5, 2020

quixote911 Jun 4, 2020

punto-rojo Jun 4, 2020

quixote911 Jun 5, 2020 •

edited

Loading

punto-rojo Jun 5, 2020

quixote911 commented Jun 4, 2020

quixote911 Jun 5, 2020

punto-rojo Jun 5, 2020

quixote911 Jun 5, 2020 •

edited

Loading

quixote911 Jun 5, 2020

punto-rojo Jun 5, 2020 •

edited

Loading

quixote911 Jun 5, 2020 •

edited

Loading

punto-rojo Jun 5, 2020

quixote911 Jun 5, 2020

quixote911 Jun 5, 2020

Feature/remote key manager #202

Are you sure you want to change the base?

Feature/remote key manager #202

Conversation

punto-rojo commented May 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quixote911 Jun 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quixote911 commented Jun 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quixote911 Jun 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

punto-rojo Jun 5, 2020 • edited Loading

Choose a reason for hiding this comment

quixote911 Jun 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quixote911 Jun 5, 2020 •

edited

Loading

quixote911 Jun 5, 2020 •

edited

Loading

punto-rojo Jun 5, 2020 •

edited

Loading

quixote911 Jun 5, 2020 •

edited

Loading