XRPLF / rippled

Decentralized cryptocurrency blockchain daemon implementing the XRP Ledger protocol in C++
https://xrpl.org
ISC License
4.53k stars 1.47k forks source link

Augmented submit fields should be included in sign-and-submit mode (Version: 1.5.0-b6) #3284

Open mDuo13 opened 4 years ago

mDuo13 commented 4 years ago

Issue Description

The new fields added to the submit method response by #3125 are only provided when you submit a binary blob, not when you provide a JSON and secret.

The fields should be provided in all responses to the submit command.

Steps to Reproduce

For a quick offline test:

  1. Start rippled in standalone mode with a fresh ledger.

    rippled -a --start
  2. Sign-and-submit a payment from the genesis account to another address of your choice.

    rippled submit masterpassphrase '{"TransactionType":"Payment", "Account":"rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh","Destination":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "Amount":"50000000000000000"}'

Expected Result

The new fields, such as applied, broadcast, account_sequence_available, etc. should be included in the response. Here's an example of a similar transaction submitted as binary, with the new fields provided:

Loading: "/home/mduo13/.config/ripple/rippled.cfg"
2020-Mar-05 00:28:16.291423410 UTC HTTPClient:NFO Connecting to 127.0.0.1:5005

{
   "result" : {
      "accepted" : true,
      "account_sequence_available" : 3,
      "account_sequence_next" : 3,
      "applied" : false,
      "broadcast" : true,
      "engine_result" : "tecUNFUNDED_PAYMENT",
      "engine_result_code" : 104,
      "engine_result_message" : "Insufficient XRP balance to send.",
      "kept" : true,
      "open_ledger_cost" : "10",
      "queued" : false,
      "status" : "success",
      "tx_blob" : "120000228000000024000000026140B1A2BC2EC5000068400000000000000A73210330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD02074473045022100856282EB45272689B3E51E9888089497DBCE3C01F28E11BD8403D5195878656502202A32A11DA97AAA2D27F1BEE36FD03017CD7F1645A6E4D863E83B8A570077F67C8114B5F762798A53D543A014CAF8B297CFF8F2F937E883144B4E9C06F24296074F7BC48F92A97916C6DC5EA9",
      "tx_json" : {
         "Account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
         "Amount" : "50000000000000000",
         "Destination" : "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
         "Fee" : "10",
         "Flags" : 2147483648,
         "Sequence" : 2,
         "SigningPubKey" : "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020",
         "TransactionType" : "Payment",
         "TxnSignature" : "3045022100856282EB45272689B3E51E9888089497DBCE3C01F28E11BD8403D5195878656502202A32A11DA97AAA2D27F1BEE36FD03017CD7F1645A6E4D863E83B8A570077F67C",
         "hash" : "9712FE0C29D1FF0351BFC1E7874CB077F95C032B07C7DEE8D84B13211008C6CF"
      },
      "validated_ledger_index" : 2
   }
}

Actual Result

Actual result matches the old (v1.4.0 and lower) result fields for the submit method:

Loading: "/home/mduo13/.config/ripple/rippled.cfg"
2020-Mar-05 00:28:07.292019643 UTC HTTPClient:NFO Connecting to 127.0.0.1:5005

{
   "result" : {
      "deprecated" : "This command has been deprecated and will be removed in a future version of the server. Please migrate to a standalone signing tool.",
      "status" : "success",
      "tx_blob" : "120000228000000024000000026140B1A2BC2EC5000068400000000000000A73210330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD02074473045022100856282EB45272689B3E51E9888089497DBCE3C01F28E11BD8403D5195878656502202A32A11DA97AAA2D27F1BEE36FD03017CD7F1645A6E4D863E83B8A570077F67C8114B5F762798A53D543A014CAF8B297CFF8F2F937E883144B4E9C06F24296074F7BC48F92A97916C6DC5EA9",
      "tx_json" : {
         "Account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
         "Amount" : "50000000000000000",
         "Destination" : "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
         "Fee" : "10",
         "Flags" : 2147483648,
         "Sequence" : 2,
         "SigningPubKey" : "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020",
         "TransactionType" : "Payment",
         "TxnSignature" : "3045022100856282EB45272689B3E51E9888089497DBCE3C01F28E11BD8403D5195878656502202A32A11DA97AAA2D27F1BEE36FD03017CD7F1645A6E4D863E83B8A570077F67C",
         "hash" : "9712FE0C29D1FF0351BFC1E7874CB077F95C032B07C7DEE8D84B13211008C6CF"
      }
   }
}

Environment

Version: rippled version 1.5.0-b6 OS: Arch Linux (latest) x86_64 Self-compiled, Boost 1.72.0. All unit tests pass.

intelliot commented 4 years ago

I think we should consider deprecating sign-and-submit mode, and create an issue for building a separate signing tool. The separate signing tool can be implemented in C++ or in TypeScript.

(More diverse implementations is a good thing, in my opinion. We should start with C++ or TypeScript because the building blocks are already complete. I'd also be in favor of signing tools/libraries in other popular languages, like Python, Java, and Ruby, but these would require a lot more work.)

MarkusTeufelberger commented 4 years ago

Neither C++ nor TypeScript are easy to call from other languages (e.g. via CFFI) - until there's a solution that is as easy to use from any(!) language as JSON-RPC I'd like to keep this mode within rippled behind a switch.

mDuo13 commented 3 years ago

I agree with @MarkusTeufelberger and I believe I can sum up the case in favor as follows:

To submit transactions to the XRP Ledger, you need the following bare minimum:

  1. The ability to create a signed transaction in the XRP Ledger's canonical binary format. You need approximately total trust¹ in this software.
  2. A connection to the XRP Ledger to submit the transaction to the network. You need a low level of trust² in this software.

Everything else is optional, but you absolutely need both of these things. Currently rippled is capable of doing both, and nothing other than rippled is capable of doing (2); anything that exists today that submits transactions is going to eventually use rippled to peer up with the network and relay your transaction. So as long as it's not too difficult (and it's currently not), it makes perfect sense for one program to provide both pieces of the bare minimum.

¹ If your signing software screws up, any number of things can go wrong: not being able to send a transaction at a crucial time (invalid signature or blob format) is the most likely outcome, accidentally opening yourself up to losing all your money (leaking your secret key), suffering reputational damage, accidentally discharging funds (correct signature of incorrect transaction instructions) etc.

² You can, to a large extent, rely on "public" rippled servers run by third parties to submit binary transaction blobs to the network for you. However, that relationship is not totally devoid of trust. A server operator could have their machine selectively filter out/censor or delay transactions in a way that's convenient to the operator: for example, preventing competing attempts at claiming liquidity in the decentralized exchange, blocking payments to particular destinations at key times, and so on. You can (theoretically) detect and avoid this sort of behavior if you submit your signed blobs to multiple servers; so, the amount you must trust a server is inversely proportional to the number of servers you rely on. At present, one of the most convenient ways to establish a connection to multiple servers in the network is to peer with them directly using your own server—that is, to run rippled yourself.

mvadari commented 1 year ago

FWIW I find sign-and-submit mode very useful in standalone mode when writing new transaction types. If I want to sign with a library, I need to update the binary codec, which is annoying when I'm still testing things out and everything is in flux.

ckeshava commented 10 months ago

I'd like to reiterate the security concerns highlighted by Nik in commit 38c3a46a:

Deprecate commands that perform remote tx signing (RIPD-1649):

In order to facilitate transaction signing, `rippled` offers the `sign` and
`sign_for` and `submit` commands, which, given a seed, can be used to sign or
sign-and-submit transactions. These commands are accessible from the command
line, as well as over the WebSocket and RPC interfaces that `rippled` can be
configured to provide.

These commands, unfortunately, have significant security implications:

  1. They require divulging an account's seed (commonly known as a "secret
     key") to the server.
  2. When executing these commands against remote servers, the seeds can be
     transported over clear-text links.
  3. When executing these commands over the command line, the account
     seed may be visible using common tools that show running processes
     and may potentially be inadvertently stored by system monitoring
     tools or facilities designed to maintain a history of previously
     typed commands.

While this commit cannot prevent users from issuing these commands to a
server, whether locally or remotely, it restricts the `sign` and `sign_for`
commands, as well as the `submit` command when used to sign-and-submit,
so that they require administrative privileges on the server.

Server operators that want to allow unrestricted signing can do so by
adding the following stanza to their configuration file:

    [signing_support]
    true

Ripple discourages server operators from doing so and advises against using
these commands, which will be removed in a future release. If you rely on
these commands for signing, please migrate to a standalone signing solution
as soon as possible. One option is to use `ripple-lib`; documentation is
available at https://developers.ripple.com/rippleapi-reference.html#sign.

If the commands are administratively enabled, the server includes a warning
on startup and adds a new field in the resulting JSON, informing the caller
that the commands are deprecated and may become unavailable at any time.

Acknowledgements:
Jesper Wallin for reporting this issue to Ripple.

Bug Bounties and Responsible Disclosures:
We welcome reviews of the rippled code and urge researchers to responsibly
disclose any issues that they may find. For more on Ripple's Bug Bounty
program, please visit: https://ripple.com/bug-bounty

Instead of reading the secret in clear-text from command line, we could read it from a file. This reduces visibility to other users on a server, but it's still not foolproof.

ximinez commented 10 months ago

Instead of reading the secret in clear-text from command line, we could read it from a file. This reduces visibility to other users on a server, but it's still not foolproof.

While we could do this, the command-line risks are a much smaller problem than transmitting a secret over the internet, potentially in plain-text, and potentially to an unknown / hostile server.

ckeshava commented 10 months ago

Instead of reading the secret in clear-text from command line, we could read it from a file. This reduces visibility to other users on a server, but it's still not foolproof.

While we could do this, the command-line risks are a much smaller problem than transmitting a secret over the internet, potentially in plain-text, and potentially to an unknown / hostile server.

Yes, that is true. This problem must have been solved in other contexts. For instance, we could mimic the ways of web browsers in handling and transferring secrets. That would still leave hostile servers as a possible attack vector. :/

intelliot commented 10 months ago

Users should use public key cryptography the way it was intended to be used: the private key always remains private and should never be transmitted. Transaction signing can, and should, be performed locally. That said, sign-and-submit is still useful on trusted servers (i.e. you can use it on a server you solely control) and for testing on networks where the assets/tokens (and keys) have no value.

mvadari commented 6 months ago

Could sign-and-submit be switched to an admin-only command? I think the level of trust is roughly equivalent.

ckeshava commented 6 months ago

In the sign-and-submit command, the user is trusting the server with his/her secret key material.

An admin method places restrictions on who can invoke the sign-and-submit method, but it doesn't change the inherent trust assumptions.

Being a non-admin user, I've found this method very useful. It would be an inconvenience if we upgrade it to an admin method.

mvadari commented 6 months ago

In the sign-and-submit command, the user is trusting the server with his/her secret key material.

An admin method places restrictions on who can invoke the sign-and-submit method, but it doesn't change the inherent trust assumptions.

Being a non-admin user, I've found this method very useful. It would be an inconvenience if we upgrade it to an admin method.

How are you using it as a non-admin user? No public server supports it.

ckeshava commented 6 months ago

I use it with my local running version of rippled. But I don't use sudo or any admin privileges. Maybe I'm still considered an admin user? I don't know.

mvadari commented 6 months ago

I use it with my local running version of rippled. But I don't use sudo or any admin privileges. Maybe I'm still considered an admin user? I don't know.

You're considered an admin user, then. It is likely automatically giving you the admin versions of the RPCs; if not, you can easily change the configuration to do that.

https://xrpl.org/docs/tutorials/http-websocket-apis/build-apps/get-started/#admin-access

ckeshava commented 6 months ago

thanks, I'll look into it