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

rpc: Implement random-cookie based authentication #1999

Merged
merged 1 commit into from Jan 18, 2017

Conversation

Projects
None yet
5 participants
@str4d
Contributor

str4d commented Jan 9, 2017

Cherry-picked from bitcoin/bitcoin#6388.

Closes #1950.

rpc: Implement random-cookie based authentication
When no `-rpcpassword` is specified, use a special 'cookie' file for
authentication. This file is generated with random content when the
daemon starts, and deleted when it exits. Read access to this file
controls who can access through RPC. By default this file is stored in
the data directory but it be overriden with `-rpccookiefile`.

This is similar to Tor CookieAuthentication: see
https://www.torproject.org/docs/tor-manual.html.en

Alternative to #6258. Like that pull, this allows running bitcoind
without any manual configuration. However, daemons should ideally never write to
their configuration files, so I prefer this solution.
@bitcartel

ACK once the following issue is addressed: option '-server' is by default 1, so with this backport, when simply launching zcashd out of the box, the rpc interface is available.

I think we should change bitcoind.cpp line 145 SoftSetBoolArg("-server", true); to false, and also update the configuration instructions to add 'server=1' to zcash.conf: https://github.com/zcash/zcash/wiki/1.0-User-Guide

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Jan 17, 2017

Contributor

I disagree, for two reasons:

  • From a security perspective, this is strictly equivalent to the user blindly copy-pasting the generated rpcuser and rpcpassword into their zcash.conf - namely, anyone with read access to the Zcash data dir can access the RPC.
  • From a usability perspective, your concern is already addressed. Now that #2013 has been merged, zcashd cannot be launched out of the box, and some user interaction is required. Specifically, if the user blindly follows the messages we show them, then the end result of this PR is no different to what would happen with current master.
Contributor

str4d commented Jan 17, 2017

I disagree, for two reasons:

  • From a security perspective, this is strictly equivalent to the user blindly copy-pasting the generated rpcuser and rpcpassword into their zcash.conf - namely, anyone with read access to the Zcash data dir can access the RPC.
  • From a usability perspective, your concern is already addressed. Now that #2013 has been merged, zcashd cannot be launched out of the box, and some user interaction is required. Specifically, if the user blindly follows the messages we show them, then the end result of this PR is no different to what would happen with current master.
@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Jan 18, 2017

Contributor

@str4d I was reviewing the PR in isolation and not taking into account #2013. I agree with your comment.
ACK

Contributor

bitcartel commented Jan 18, 2017

@str4d I was reviewing the PR in isolation and not taking into account #2013. I agree with your comment.
ACK

@ebfull

This comment has been minimized.

Show comment
Hide comment
@ebfull

ebfull Jan 18, 2017

Contributor

@zkbot r+

Contributor

ebfull commented Jan 18, 2017

@zkbot r+

@zkbot

This comment has been minimized.

Show comment
Hide comment
@zkbot

zkbot Jan 18, 2017

Contributor

📌 Commit e957192 has been approved by ebfull

Contributor

zkbot commented Jan 18, 2017

📌 Commit e957192 has been approved by ebfull

@zkbot

This comment has been minimized.

Show comment
Hide comment
@zkbot

zkbot Jan 18, 2017

Contributor

⌛️ Testing commit e957192 with merge 94f427a...

Contributor

zkbot commented Jan 18, 2017

⌛️ Testing commit e957192 with merge 94f427a...

zkbot added a commit that referenced this pull request Jan 18, 2017

Auto merge of #1999 - str4d:1950-random-cookie-rpc-auth, r=ebfull
rpc: Implement random-cookie based authentication

Cherry-picked from bitcoin/bitcoin#6388.

Closes #1950.
@zkbot

This comment has been minimized.

Show comment
Hide comment
@zkbot

zkbot Jan 18, 2017

Contributor

☀️ Test successful - zcash

Contributor

zkbot commented Jan 18, 2017

☀️ Test successful - zcash

@zkbot zkbot merged commit e957192 into zcash:master Jan 18, 2017

1 check passed

homu Test successful
Details

ondrejsika added a commit to ondrejsika/zcash that referenced this pull request Jan 25, 2017

Merge tag 'v1.0.5' into ondrejsika/regtest-200-9
Zcash 1.0.5

This release contains a number of bug fixes and minor usability improvements, including:

1. The chain is now fully rescanned when keys are imported that are older than the wallet. (zcash#1978)
2. The number of commitments in the note commitment tree is now displayed by `getblockchaininfo`. (zcash#1946)
3. `zcash.conf` now must exist in order to start zcashd. (zcash#2013)
4. Fixed a bug where `z_sendmany` logged incorrect txid fragments when sending from transparent addresses. (zcash#1980)
5. We integrated upstream's cookie-based RPC authentication. (zcash#1999)
6. We added a restriction to wallet export paths to protect user security. (zcash#2006)
7. `z_getoperationstatus` is now sorted chronologically. (zcash#2015)
8. Messages containing newlines are now rendered properly by the metrics UI. (zcash#1972)
9. We added more tools for benchmarking JoinSplit creation. (zcash#1953)
10. We now show serialized transaction size in `listtransactions`, more operation details in `z_getoperationstatus`, and the age of the note being spent in `z_sendmany` logging. (zcash#2001, zcash#1976, zcash#1977)
11. We now instruct users to run `fetch-params` if the parameters could not be found locally. (zcash#1979)
12. We handle exceptions better in some situations for more user-friendly error messages. (zcash#1976)

For a more complete list of changes, see our [1.0.5 milestone](https://github.com/zcash/zcash/milestone/49?closed=1).

radix42 added a commit to radix42/zcash that referenced this pull request Jan 26, 2017

Merge tag 'v1.0.5' into v1.0.5-win-cross
Zcash 1.0.5

This release contains a number of bug fixes and minor usability improvements, including:

1. The chain is now fully rescanned when keys are imported that are older than the wallet. (zcash#1978)
2. The number of commitments in the note commitment tree is now displayed by `getblockchaininfo`. (zcash#1946)
3. `zcash.conf` now must exist in order to start zcashd. (zcash#2013)
4. Fixed a bug where `z_sendmany` logged incorrect txid fragments when sending from transparent addresses. (zcash#1980)
5. We integrated upstream's cookie-based RPC authentication. (zcash#1999)
6. We added a restriction to wallet export paths to protect user security. (zcash#2006)
7. `z_getoperationstatus` is now sorted chronologically. (zcash#2015)
8. Messages containing newlines are now rendered properly by the metrics UI. (zcash#1972)
9. We added more tools for benchmarking JoinSplit creation. (zcash#1953)
10. We now show serialized transaction size in `listtransactions`, more operation details in `z_getoperationstatus`, and the age of the note being spent in `z_sendmany` logging. (zcash#2001, zcash#1976, zcash#1977)
11. We now instruct users to run `fetch-params` if the parameters could not be found locally. (zcash#1979)
12. We handle exceptions better in some situations for more user-friendly error messages. (zcash#1976)

For a more complete list of changes, see our [1.0.5 milestone](https://github.com/zcash/zcash/milestone/49?closed=1).

joshuayabut added a commit to z-classic/zclassic that referenced this pull request Jan 31, 2017

Merge tag 'v1.0.5' into v1.0.5-multios
Zcash 1.0.5

This release contains a number of bug fixes and minor usability improvements, including:

1. The chain is now fully rescanned when keys are imported that are older than the wallet. (zcash#1978)
2. The number of commitments in the note commitment tree is now displayed by `getblockchaininfo`. (zcash#1946)
3. `zcash.conf` now must exist in order to start zcashd. (zcash#2013)
4. Fixed a bug where `z_sendmany` logged incorrect txid fragments when sending from transparent addresses. (zcash#1980)
5. We integrated upstream's cookie-based RPC authentication. (zcash#1999)
6. We added a restriction to wallet export paths to protect user security. (zcash#2006)
7. `z_getoperationstatus` is now sorted chronologically. (zcash#2015)
8. Messages containing newlines are now rendered properly by the metrics UI. (zcash#1972)
9. We added more tools for benchmarking JoinSplit creation. (zcash#1953)
10. We now show serialized transaction size in `listtransactions`, more operation details in `z_getoperationstatus`, and the age of the note being spent in `z_sendmany` logging. (zcash#2001, zcash#1976, zcash#1977)
11. We now instruct users to run `fetch-params` if the parameters could not be found locally. (zcash#1979)
12. We handle exceptions better in some situations for more user-friendly error messages. (zcash#1976)

For a more complete list of changes, see our [1.0.5 milestone](https://github.com/zcash/zcash/milestone/49?closed=1).

Conflicts:
	README.md
	configure.ac
	contrib/debian/changelog
	contrib/debian/control
	contrib/debian/manpages/zcash-cli.1
	contrib/debian/manpages/zcashd.1
	contrib/gitian-descriptors/gitian-linux.yml
	doc/authors.md
	src/Makefile.gtest.include
	src/chainparams.cpp
	src/clientversion.h
	src/init.cpp
	src/main.h
	src/metrics.cpp
	src/metrics.h
	src/util.cpp
	src/wallet/asyncrpcoperation_sendmany.cpp
	src/wallet/asyncrpcoperation_sendmany.h
	src/wallet/rpcwallet.cpp
	src/zcbenchmarks.cpp
	zcutil/build-debian-package.sh

@str4d str4d deleted the str4d:1950-random-cookie-rpc-auth branch Feb 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment