Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Oct 1, 2016

Base64 contains '/', and the '/' character in credentials is problematic for AuthServiceProxy which represents the RPC endpoint as an URI with user and password embedded.

Closes #8399. @gmaxwell

Base64 contains '/', and the '/' character in credentials is problematic
for AuthServiceProxy which represents the RPC endpoint as an URI with
user and password embedded.

Closes bitcoin#8399.
@maflcko
Copy link
Member

maflcko commented Oct 1, 2016

utACK 1c80386

@btcdrak
Copy link
Contributor

btcdrak commented Oct 1, 2016

utACK 1c80386

@dcousens
Copy link
Contributor

dcousens commented Oct 2, 2016

utACK 1c80386, but base58 could have worked, not sure if that matters though.

@laanwj
Copy link
Member Author

laanwj commented Oct 2, 2016

utACK 1c80386, but base58 could have worked, not sure if that matters though.

Thought about that, but I eventually decided on hex. It doesn't matter here, but if there's no need to introduce a dependency on something so bitcoin-specific in general RPC protocol code that may be better. Hex encoding on the other hand is trivial and readily available.

@paveljanik
Copy link
Contributor

ACK 1c80386

@gmaxwell
Copy link
Contributor

gmaxwell commented Oct 3, 2016

ACK.

Thanks for fixing this problem I reported then forgot about.

@sipa
Copy link
Member

sipa commented Oct 3, 2016

utACK.

@laanwj laanwj merged commit 1c80386 into bitcoin:master Oct 3, 2016
laanwj added a commit that referenced this pull request Oct 3, 2016
1c80386 rpc: Generate auth cookie in hex instead of base64 (Wladimir J. van der Laan)
laanwj added a commit that referenced this pull request Oct 18, 2016
Base64 contains '/', and the '/' character in credentials is problematic
for AuthServiceProxy which represents the RPC endpoint as an URI with
user and password embedded.

Closes #8399.

Github-Pull: #8858
Rebased-From: 1c80386
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Apr 26, 2021
8817d76 Add config changes to release notes (random-zebra)
988d428 [tests] Unit tests for -testnet/-regtest in [test]/[regtest] sections (Anthony Towns)
7b72630 ArgsManager: special handling for -regtest and -testnet (Anthony Towns)
51c4aa1 [tests] Unit tests for network-specific config entries (Anthony Towns)
30f3bab ArgsManager: Warn when ignoring network-specific config setting (Anthony Towns)
1bddffe ArgsManager: limit some options to only apply on mainnet when in default section (Anthony Towns)
ef505ea [Tests] Fix and re-activate rpc_users.py functional test (random-zebra)
0eacce7 [RPC] Add rpcauth startup flag for multiple RPC users (random-zebra)
e79ff3c [Tests] Fix and re-activate feature_config_args functional test (random-zebra)
3fcaaa4 Replace cookie auth in tests (random-zebra)
b907f33 rpc: Generate auth cookie in hex instead of base64 (random-zebra)
bf443a9 [tests] Use regtest section in functional tests configs (Anthony Towns)
7857bcd [tests] Unit tests for config file sections (Anthony Towns)
4cf2ee6 ArgsManager: support config file sections (Anthony Towns)
2658771 ArgsManager: drop m_negated_args (Anthony Towns)
4ee69b5 ArgsManager: keep command line and config file arguments separate (random-zebra)
4f416b8 [tests] Add additional unit tests for -nofoo edge cases (Anthony Towns)
5d1200b [tests] Check GetChainName works with config entries (Anthony Towns)
157da7c [tests] Add unit tests for ReadConfigStream (Anthony Towns)
23a4633 ReadConfigStream: assume the stream is good (Anthony Towns)
56ea59e Separate out ReadConfigStream from ReadConfigFile (Anthony Towns)
a3438a2 Test datadir in conf file exists (MeshCollider)
459c4ad [tests] Add unit tests for GetChainName (Anthony Towns)
0ab3e99 Move ChainNameFromCommandLine into ArgsManager, rename to GetChainName (Anthony Towns)
1ea7ce6 Globals: Decouple GetConfigFile and ReadConfigFile from global mapArgs (random-zebra)

Pull request description:

  It is now possible for a single configuration file to set different options for different networks. This is done by using sections or by prefixing the option with the network, such as:
  ```
      main.uacomment=pivx
      test.uacomment=pivx-testnet
      regtest.uacomment=regtest
      [main]
      mempoolsize=300
      [test]
      mempoolsize=100
      [regtest]
      mempoolsize=20
  ```
  The `addnode=`, `connect=`, `port=`, `bind=`, `rpcport=`, `rpcbind=`, and `wallet=` options will only apply to mainnet when specified in the configuration file, unless a network is specified.

  Also
  - fix cookie-based authentication for the functional tests, and re-enable `feature_config_args.py`
  - add `-rpcauth` startup flag, for multiple RPC users, and re-enable `rpc_users.py`.

  Backports relevant commits from:
  - bitcoin#8856 Globals: Decouple GetConfigFile and ReadConfigFile from global mapArgs
  - bitcoin#11829 Test datadir specified in conf file exists
  - bitcoin#12878 Config handling refactoring in preparation for network-specific sections
  - bitcoin#11862 Network specific conf sections
  - bitcoin#10533 [tests] Use cookie auth instead of rpcuser and rpcpassword
  - bitcoin#8858 Generate auth cookie in hex instead of base64

ACKs for top commit:
  Fuzzbawls:
    Tested ACK 8817d76
  furszy:
    utACK 8817d76

Tree-SHA512: 0d23dbf3d254b186a5378576601cf43f8322abe036b0b135a39b22e13ffa2e299cb1323160d87c7d8284e6aaa229c47f54c8a3a3ebf07cc298e644fb8bd69dc0
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cookie auth picks passwords which are incompatible with some URL parsers

7 participants