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

Switch from using the global agent to using global keypairs for tests #404

Closed
5 of 6 tasks
emmacasolin opened this issue Jul 11, 2022 · 13 comments · Fixed by #391
Closed
5 of 6 tasks

Switch from using the global agent to using global keypairs for tests #404

emmacasolin opened this issue Jul 11, 2022 · 13 comments · Fixed by #391
Assignees
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity

Comments

@emmacasolin
Copy link
Contributor

emmacasolin commented Jul 11, 2022

Specification

#264 introduced our global agent which is used in our tests as a way to reduce setup time (since we no longer need to create an agent for every test). However, this solution complicates our tests by requiring all side effects to be reversed (some side effects cannot be undone, e.g. sigchain changes and some identities changes) and preventing tests that use the global agent from running in parallel.

An alternative solution is to have a pre-generated root private key that we can pass into the PolykeyAgent creation. This should sidestep the key generation process and speed up agent creation significantly.

Additional context

Tasks

  • 1. Add an override to KeyManager generation to provide a root private key to skip root key-pair generation.
  • 2. Propagate this override up to the PolykeyAgent parameters
  • 3. create tests confirming this behaviour.
  • 4. Propagate this change up to pk agent start and pk bootstrap. Including new parameter + ENV variable.
  • 5. Replace all usages of global agent with root key override.
  • 6. Move global keypair fixtures to a common directory as tests/fixtures
@emmacasolin emmacasolin added the development Standard development label Jul 11, 2022
@tegefaulkes
Copy link
Contributor

I'm going to change this up slightly and include it in MatrixAI/Polykey-CLI#10, #391. Instead of using mocking for this I'll add a parameter/ENV for setting the root private key to skip the generation step. We can extend this change to key mocking across all tests. That should be a separate issue however.

The changes here should be

  1. Allow override to KeyManager generation to provide a root private key to skip keypair generation.
  2. Propigate this override up to the PolykeyAgent parameters
  3. create tests confirming this behaviour.
  4. Propagate this change up to pk agent start and pk bootstrap. Including new parameter + env variable.
  5. Replace all usages of global agent with root key override.

I also need to create a new issue to replace key mocking for all tests.

@tegefaulkes tegefaulkes self-assigned this Jul 15, 2022
tegefaulkes added a commit that referenced this issue Jul 15, 2022
…erride` parameter for `createKeyManager`

This will skip key generation and use the provided `PrivateKey` instead. This should speed up testing by skipping the key generation.

Related #404
@tegefaulkes
Copy link
Contributor

The neat thing is, with the new private-Key override we gain the feature of having a node signed by another node. This can be done by generating the node with a private key of another node and then renewing the keys to generate a new root key-pair signed by the previous one. The affect of which is having the node essentially signed by the other node.

@CMCDragonkai
Copy link
Member

Loading a key that is created by another node is not exactly the same as cross-signing (PKI). See #154 for what I mean.

tegefaulkes added a commit that referenced this issue Jul 15, 2022
…ootstrap.ts`

This should allow us to override the keypair generation with the provided private key. this will speed up agent starting.

Still need to test this for `agent start` and `bootstrap`.

Related #404
tegefaulkes added a commit that referenced this issue Jul 18, 2022
…mandBootstrap.ts`

This should allow us to override the keypair generation with the provided private key. this will speed up agent starting.

Note that the key is provided as a Pem. The `PrivateKey` type contains functions that get destroyed somewhere between `commandStart.ts` and `keyManager.createKeyManager`. So I'm using the Pem string to keep the type primitive

Related #404
@tegefaulkes
Copy link
Contributor

pk agent start and pk bootstrap take the --private-key-file parameter now. This will be a path to a file containing the private key in Pem format. SInce the Pem format includes newlines, we can't pass it directly as an ENV or parameter.

The Pem is passed through createPolykeyAgent to createKeyManager. It is converted to a keyPair inside the key manager during the key generation step. I originally tried using the PrivateKey format but that contained functions inside it that were lost at certain stages of passing the key around.

I'm starting on removing the global agent in the tests now.

@CMCDragonkai
Copy link
Member

I'm thinking you should call this parameter --root-key. Do we use -file suffix in other parameters?

@CMCDragonkai
Copy link
Member

And yes you can pass newlines as environment variable. It's not a problem. Variables in shell Env can have newlines.

@CMCDragonkai
Copy link
Member

Also if this parameter is specified, you have to ignore any usage of recovery code. It needs to be ignored.

If the file is password protected you'll need both a password to decrypt it, and a password to encrypt it.

So --password would decrypt it if necessary and be the same password to encrypt.

@tegefaulkes
Copy link
Contributor

I'll double check the Env.

As for the -file suffix. we have --recovery-code-file, --password-new-file, --password-file et al. I can change it to --root-key-file.

tegefaulkes added a commit that referenced this issue Jul 18, 2022
tegefaulkes added a commit that referenced this issue Jul 18, 2022
This is a quick and easy way to create an agent with a pre-generated key.

Related #404
tegefaulkes added a commit that referenced this issue Jul 18, 2022
@tegefaulkes
Copy link
Contributor

While this issue deals specifically with removing the global agent. The root key override can be used to optimise any test using a polykey agent, not just the bin tests. Such as the nodes, discovery, vaults? etc.

I think these changes would be slightly out of the scope of this issue. So I should create a new issue for it.

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Jul 19, 2022

Looks like theres a bug with allows/disallows/gets gestalt permissions by node in tests/bin/identities/allowDisallowPermissions.test.ts:101. It throws an error when shutting down the agents right after the test. This started happening because I changed the afterAll to afterEach.

The error is

ErrorConnectionNotRunning: 
    at constructor_.getConnectionInfoByProxy (/home/faulkes/matrixcode/polykey/js-polykey/src/network/Proxy.ts:273:40)
    at constructor_.getConnectionInfoByProxy (/home/faulkes/matrixcode/polykey/js-polykey/node_modules/@matrixai/async-init/src/StartStop.ts:176:18)
    at constructor_.getRootCertChain (/home/faulkes/matrixcode/polykey/js-polykey/src/nodes/NodeConnection.ts:202:33)
    at constructor_.getRootCertChain (/home/faulkes/matrixcode/polykey/js-polykey/node_modules/@matrixai/async-init/src/CreateDestroy.ts:155:18)
    at constructor_.getExpectedPublicKey (/home/faulkes/matrixcode/polykey/js-polykey/src/nodes/NodeConnection.ts:218:31)
    at constructor_.getExpectedPublicKey (/home/faulkes/matrixcode/polykey/js-polykey/node_modules/@matrixai/async-init/src/CreateDestroy.ts:155:18)
    at /home/faulkes/matrixcode/polykey/js-polykey/src/nodes/NodeManager.ts:176:40
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at /home/faulkes/matrixcode/polykey/js-polykey/src/nodes/NodeConnectionManager.ts:204:16
    at withF (/home/faulkes/matrixcode/polykey/js-polykey/node_modules/@matrixai/resources/src/utils.ts:24:12)
    at constructor_.withConnF (/home/faulkes/matrixcode/polykey/js-polykey/src/nodes/NodeConnectionManager.ts:196:12)
    at constructor_.requestChainData (/home/faulkes/matrixcode/polykey/js-polykey/src/nodes/NodeManager.ts:152:7)
    at constructor_.setupDiscoveryQueue (/home/faulkes/matrixcode/polykey/js-polykey/src/discovery/Discovery.ts:370:21)

I think @emmacasolin wrote the tests for this. Do you have any idea why this is happening? It's coming from the discovery queue. Is it not ending properly during agent shutdown?

tegefaulkes added a commit that referenced this issue Jul 19, 2022
I've also used the key override where needed for `createPolykeyAgent`.

Related #404
@tegefaulkes
Copy link
Contributor

It's an edge case, the discovery queue is handling ErrorNodeConnectionDestroyed and ErrorNodeConnectionTimeout when doing requestChainData. But it's not handling ErrorConnectionNotRunning. It's a quick fix.

@tegefaulkes
Copy link
Contributor

This is done now. I'll create a new issue for the key generation override for all the other tests.

@tegefaulkes
Copy link
Contributor

This likely fixed #347, I'll include it in the PR.

tegefaulkes added a commit that referenced this issue Jul 26, 2022
…erride` parameter for `createKeyManager`

This will skip key generation and use the provided `PrivateKey` instead. This should speed up testing by skipping the key generation.

Related #404
tegefaulkes added a commit that referenced this issue Jul 26, 2022
…mandBootstrap.ts`

This should allow us to override the keypair generation with the provided private key. this will speed up agent starting.

Note that the key is provided as a Pem. The `PrivateKey` type contains functions that get destroyed somewhere between `commandStart.ts` and `keyManager.createKeyManager`. So I'm using the Pem string to keep the type primitive

Related #404
tegefaulkes added a commit that referenced this issue Jul 26, 2022
tegefaulkes added a commit that referenced this issue Jul 26, 2022
This is a quick and easy way to create an agent with a pre-generated key.

Related #404
tegefaulkes added a commit that referenced this issue Jul 26, 2022
tegefaulkes added a commit that referenced this issue Jul 26, 2022
I've also used the key override where needed for `createPolykeyAgent`.

Related #404
tegefaulkes added a commit that referenced this issue Jul 27, 2022
…erride` parameter for `createKeyManager`

This will skip key generation and use the provided `PrivateKey` instead. This should speed up testing by skipping the key generation.

Related #404
tegefaulkes added a commit that referenced this issue Jul 27, 2022
…mandBootstrap.ts`

This should allow us to override the keypair generation with the provided private key. this will speed up agent starting.

Note that the key is provided as a Pem. The `PrivateKey` type contains functions that get destroyed somewhere between `commandStart.ts` and `keyManager.createKeyManager`. So I'm using the Pem string to keep the type primitive

Added `PK_ROOT_KEY` ENV and changed `--private-key-file` to `--root-key-file`

Related #404
tegefaulkes added a commit that referenced this issue Jul 27, 2022
This is a quick and easy way to create an agent with a pre-generated key.

Related #404
tegefaulkes added a commit that referenced this issue Jul 27, 2022
I've also used the key override where needed for `createPolykeyAgent`.

Related #404
@CMCDragonkai CMCDragonkai added the r&d:polykey:supporting activity Supporting core activity label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:supporting activity Supporting core activity
Development

Successfully merging a pull request may close this issue.

3 participants