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

Add init_api_secure function #206

Merged
merged 21 commits into from
Aug 19, 2019

Conversation

yeastplume
Copy link
Member

@yeastplume yeastplume commented Aug 13, 2019

  • Adds a new V3 Owner API, with the V2 API still running for backwards-compatibility purposes
  • Adds functionality to the controller to encrypt and decrypt responses. All V3 API functions except init_secure_api must be encrypted.
  • Secure requests are JSON-RPC calls that look like the following:
{
   "jsonrpc": "2.0",
   "method: "encrypted_request_v3",
   "id": "1",
   "params": {
      "nonce": "ef32...",
      "body_enc": "e0bcd..."
   }
}
  • Encryption is currently AEAD With the following parameters:
    • AES-256 in GCM mode with 128-bit tags and 96 bit nonces
    • 12 byte nonce which must be included in each request/response to use on the decrypting side
    • Empty vector for additional data
    • Suffix length = AES-256 GCM mode tag length = 16 bytes
    • Open to discussion on the encryption scheme, but AEAD/AES256GCM is currently supported in node.js by default so hopefully this encryption shouldn't throw up too much of a block.
  • Adds an init_secure_api function to the V3 API, performs an ECDH exchange (using the usual libsecp256k1 curve, again supported by node.js), sets the shared key internally and returns the public key to the caller.
  • The shared-key on the server side is static and global (as I don't believe hyper method handlers can mutate their state)
  • Also adds many tests exercising the V3 api encryption and error conditions

@yeastplume yeastplume changed the title [WIP] Add init_api_secure function Add init_api_secure function Aug 15, 2019
@yeastplume
Copy link
Member Author

No longer WIP and ready for review, updated description in top comment

let value: ECDHPubkey = res.unwrap();
let shared_key = derive_ecdh_key(sec_key_str, &value.ecdh_pubkey);

// 12) A normal request, correct key
Copy link
Contributor

Choose a reason for hiding this comment

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

11 😄

@@ -308,15 +415,54 @@ where
OwnerAPIHandlerV3 { wallet }
}

//TODO: Unwraps
Copy link
Contributor

Choose a reason for hiding this comment

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

Has this been addressed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, leftover reminder comment.

where
OUT: DeserializeOwned,
{
let url = Url::parse(dest).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we really have all of these unwraps in send_request_enc and derive_ecdh_key? What RPC error code would be returned if these panic?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just test helper code, I don't usually worry too much about unwraps there. I've been careful to ensure the server side returns proper errors for all conditions (and never unwraps)

@yeastplume yeastplume merged commit a58cae6 into mimblewimble:master Aug 19, 2019
@yeastplume yeastplume deleted the init_api_secure branch August 21, 2019 08:39
@yeastplume
Copy link
Member Author

#212

yyangli pushed a commit to mwcproject/mwc-wallet that referenced this pull request May 13, 2020
* adding initial version of init_secure_api

* rustfmt

* fix ECDH algo

* rustfmt

* trying to figure out best way of doing encryption

* refactor secure requests and responses into json-rpc responses, with base64 payload for encrypted messages

* rustfmt

* return proper errors from encrypted api, include tests covering encrypted API error cases

* rustfmt

* add test for normal error (unencrypted)

* rustfmt

* change ports for test, add foreign listener to V2 sanity tests, add ability to select owner api port via command line

* rustfmt

* turn it to 11

* explicit teardown after rpc tests

* update tests with explicit teardowns

* update tests to perform explicit teardown

* fix warnings, ensure all tests teardown

* log output to diagnose CI windows build failures

* disable owner api doctests on windows

* rustfmt
antiochp pushed a commit to antiochp/grin-wallet that referenced this pull request Aug 7, 2020
* adding initial version of init_secure_api

* rustfmt

* fix ECDH algo

* rustfmt

* trying to figure out best way of doing encryption

* refactor secure requests and responses into json-rpc responses, with base64 payload for encrypted messages

* rustfmt

* return proper errors from encrypted api, include tests covering encrypted API error cases

* rustfmt

* add test for normal error (unencrypted)

* rustfmt

* change ports for test, add foreign listener to V2 sanity tests, add ability to select owner api port via command line

* rustfmt

* turn it to 11

* explicit teardown after rpc tests

* update tests with explicit teardowns

* update tests to perform explicit teardown

* fix warnings, ensure all tests teardown

* log output to diagnose CI windows build failures

* disable owner api doctests on windows

* rustfmt
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

Successfully merging this pull request may close these issues.

2 participants