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

Create signmessagewithprivkey rpc #7953

Merged
merged 2 commits into from May 5, 2016

Conversation

Projects
None yet
5 participants
@achow101
Member

achow101 commented Apr 27, 2016

Since no one else did...

This adds a new RPC which does not rely on the wallet to sign messages. Continuation of #7899

@paveljanik

View changes

Show outdated Hide outdated src/rpc/misc.cpp
@paveljanik

View changes

Show outdated Hide outdated src/rpc/misc.cpp
Create signmessagewithprivkey rpc
New rpc 'signmessagewithprivkey' which takes a private key to sign a message without using the wallet.
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 27, 2016

Member

utACK f90efbf

Member

laanwj commented Apr 27, 2016

utACK f90efbf

throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Sign failed");
return EncodeBase64(&vchSig[0], vchSig.size());
}

This comment has been minimized.

@jonasschnelli

jonasschnelli Apr 27, 2016

Member

IMO strPrivkey, vchSecret and params[0] should be mem-cleansed before the function returns.
I guess key RAII -cleans itself.

@jonasschnelli

jonasschnelli Apr 27, 2016

Member

IMO strPrivkey, vchSecret and params[0] should be mem-cleansed before the function returns.
I guess key RAII -cleans itself.

This comment has been minimized.

@laanwj

laanwj Apr 27, 2016

Member
  • vchSecret is ok - CBitcoinSecret (derived from CBase58Data) uses a zero_after_free allocator
  • strPrivKey could be a SecureString, I guess.

However, params[] is a const parameter vector passed in, provided by a third-party library. I don't think this issue can be solved in general (except by never passing private keys in the API in the first place). The http server, JSON parsing, the RPC mechanism leaves the parameters all over memory. We have a similar problem in importprivkey. No need to solve in this pull.

@laanwj

laanwj Apr 27, 2016

Member
  • vchSecret is ok - CBitcoinSecret (derived from CBase58Data) uses a zero_after_free allocator
  • strPrivKey could be a SecureString, I guess.

However, params[] is a const parameter vector passed in, provided by a third-party library. I don't think this issue can be solved in general (except by never passing private keys in the API in the first place). The http server, JSON parsing, the RPC mechanism leaves the parameters all over memory. We have a similar problem in importprivkey. No need to solve in this pull.

This comment has been minimized.

@jonasschnelli

jonasschnelli Apr 27, 2016

Member

Agree. Its just a nitpick (I forgot to pre-fix it with "nit:"). As long as we are dealing with privatekeys in bitcoin-core there will always be memory containing sensitive stuff.

@jonasschnelli

jonasschnelli Apr 27, 2016

Member

Agree. Its just a nitpick (I forgot to pre-fix it with "nit:"). As long as we are dealing with privatekeys in bitcoin-core there will always be memory containing sensitive stuff.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli
Member

jonasschnelli commented Apr 27, 2016

utACK f90efbf

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 27, 2016

Contributor

ACK f90efbf

@jonasschnelli Right. We should also recommend using -stdin for this when document this new RPC in release notes.

Contributor

paveljanik commented Apr 27, 2016

ACK f90efbf

@jonasschnelli Right. We should also recommend using -stdin for this when document this new RPC in release notes.

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 27, 2016

Contributor

Can you please write some test for this new RPC?

Contributor

paveljanik commented Apr 27, 2016

Can you please write some test for this new RPC?

@achow101

This comment has been minimized.

Show comment
Hide comment
@achow101

achow101 Apr 27, 2016

Member

@paveljanik I've added a test. I think I did it correctly.

Member

achow101 commented Apr 27, 2016

@paveljanik I've added a test. I think I did it correctly.

@MarcoFalke

View changes

Show outdated Hide outdated qa/rpc-tests/signmessages.py
@laanwj

View changes

Show outdated Hide outdated qa/rpc-tests/signmessages.py
Test for signing messages
New rpc test for signing and verifying messages.
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli
Member

jonasschnelli commented Apr 29, 2016

@laanwj laanwj merged commit 7db0ecb into bitcoin:master May 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request May 5, 2016

Merge #7953: Create signmessagewithprivkey rpc
7db0ecb Test for signing messages (Andrew Chow)
f90efbf Create signmessagewithprivkey rpc (Andrew)

@laanwj laanwj referenced this pull request May 5, 2016

Closed

TODO for release notes 0.13.0 #7678

14 of 16 tasks complete

@achow101 achow101 deleted the achow101:signmessagewithprivkey branch Oct 29, 2016

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7953: Create signmessagewithprivkey rpc
7db0ecb Test for signing messages (Andrew Chow)
f90efbf Create signmessagewithprivkey rpc (Andrew)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7953: Create signmessagewithprivkey rpc
7db0ecb Test for signing messages (Andrew Chow)
f90efbf Create signmessagewithprivkey rpc (Andrew)

codablock added a commit to codablock/dash that referenced this pull request Dec 21, 2017

Merge #7953: Create signmessagewithprivkey rpc
7db0ecb Test for signing messages (Andrew Chow)
f90efbf Create signmessagewithprivkey rpc (Andrew)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment