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

Disallow operations on the network with server side stored private key #638

Closed
sichen1234 opened this issue Oct 24, 2022 · 14 comments
Closed

Comments

@sichen1234
Copy link
Contributor

Just watched the video of signing with users wallet

I think this feature should not be allowed so that we're not passing the private key from the server to the client.

The consumer or operator could request a transaction, but then it should be stored in the database and routed to an auditor.

We can also set it up so that auditor roles can only be granted to users without a server stored private key.

@brioux
Copy link
Member

brioux commented Oct 25, 2022

Restricting all token creations operation to auditors makes sense. Even if it is to just initialize a request. See comment in issue637 for handling new carbon tracker requests from an oil & gas operator.

@brioux
Copy link
Member

brioux commented Oct 25, 2022

Not sure we need to limit transactions to accounts that have opted to store their keys on the server side when signing up.

It is better practice for accounts to hold keys directly, in line with the principles of web3.0 transaction. But an auditor may prefer to use a key management service.

Server side key management was introduced to the user sign-up function to help with onboarding of consumers that don't have the experience operating a server side wallet.

Should NET provide its own internal cloud key management for signing transactions? It already does in the referenced video, but with limited security.

@sichen1234
Copy link
Contributor Author

What do you think of signing the transaction on the server, instead of transferring the private key to the client and signing it there?

@brioux
Copy link
Member

brioux commented Oct 25, 2022

@sichen1234 I was just writing this up...

Here are some ideas for improving security.

Private keys never leave the server side to sign transactions, only when exporting keys and permanently deleting it from the server. This is structurally different from what was shown in the referenced video. Transaction messages could be sent encrypted to the server (using the public key) where the message is decrypted and signed. However, this requires creating a secure communication line between the client and server for a given signing session ....

@brioux
Copy link
Member

brioux commented Oct 25, 2022

Use the window.Crypto object (a secure way to use javascript to access the DOM) to store privateKeys in the client browser when receiving them from the server during sign in.

@brioux
Copy link
Member

brioux commented Oct 25, 2022

The Wallet interface with private key is loaded around here from the server on the sign in page

const rslt = await signInUser(form.email, form.password);

if (wallet.address && wallet.private_key) {

The private key should be sent by the server to the client's Wallet interface encrypted by a public key address generated by the clients window.Crypto object.

@brioux
Copy link
Member

brioux commented Oct 25, 2022

As discussed at the end of this older post a variety of modern browser features can be used to securely store the keys with the client -- this is basically what browser extensions like Metamask achieve

@sichen1234
Copy link
Contributor Author

sichen1234 commented Oct 25, 2022 via email

@brioux
Copy link
Member

brioux commented Oct 25, 2022

both require similar security development.

Keeping keys on the server limits attack vectors to server breaches. The benefit of moving the signing to the client is probably the legal context in how the server uses the private keys.

If the server is setup to sign transactions on behalf of the client, but used in a malicious transaction, the company operating the server could be held accountable.

The user agrees to the terms associated with the key management service provided by the client interface. Assuming the server provider makes all precautions to deliver the key securely, the clients are accountable for signing transactions.

@brioux
Copy link
Member

brioux commented Oct 25, 2022

A secure and legally sound client application should have baked in client key gen and storage (e.g., electron or mobile app).
Both of the above are patchy solutions at best.

@sichen1234
Copy link
Contributor Author

sichen1234 commented Oct 25, 2022 via email

@brioux
Copy link
Member

brioux commented Oct 25, 2022

Agree.

As long as the signed in user is only received retired tokens, no need to access the privateKey.

Minimize key management to the bare minimum, and rely on third party wallet party providers and custom built client applications.

Suggest putting private keys into a separate write only schema, with read access limited to key export and deletion.

Once transferable tokens are issued to the user (e.g., offset credits or carbon trackers), the keys must be exported to access the tokens.

Another more secure option is to instruct the server to use the keys once to transfer the tokens to a new public address generated by the users external wallet. This eliminates security risk from potential breach of the original private keys - but would come with additional transaction costs.

@brioux
Copy link
Member

brioux commented Oct 25, 2022

Question, if a client has connected their external wallet to NET, is there a form they can use to register information about their public address with the Postgres server - e.g. organization, name, email... ?

I have started a separate discussion on handling user information using Distributed identity management tools. A topic of discussion for future development.

@sichen1234
Copy link
Contributor Author

sichen1234 commented Oct 25, 2022 via email

sichen1234 added a commit that referenced this issue Oct 26, 2022
remove some code for private key #638
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants