Fixes #23412
Few solutions were suggested in the issue:
I am not sure if people will agree to make these changes so workaround is to just document the security recommendation. I understand *notify options are helpful for users, projects ,devs etc. however it can be misused in some cases and help attackers as well.
ACK https://github.com/bitcoin/bitcoin/pull/23850/commits/67ded3d13406de54d26ce13fb44b1c6d678fd715
Agree with the security recommendation and changes in docs. As suggested in #23395 (review) security recommendations about command line options can be added in this doc.
ACK 67ded3d
61 | @@ -62,3 +62,9 @@ Linux | `$HOME/.bitcoin/` | `/home/username/.bitcoin/bitcoin.conf` 62 | macOS | `$HOME/Library/Application Support/Bitcoin/` | `/Users/username/Library/Application Support/Bitcoin/bitcoin.conf` 63 | 64 | You can find an example bitcoin.conf file in [share/examples/bitcoin.conf](../share/examples/bitcoin.conf). 65 | + 66 | +### Command-line options and Security 67 | + 68 | +`-startupnotify`, `-shutdownnotify`, `-blocknotify`, `-walletnotify` and `-alertnotify` configuration parameters accept shell commands to be executed based on different events. This is useful for notifying the user of certain events, however it can be misused by malicious software or attackers using social engineering. Restrict scripts and commands in the shell/system used for bitcoind to avoid running potentially unsafe scripts.
AFAIK #23395 is not yet merged, and hence -shutdownnotify has not yet been added to master. So you should remove that from the docs.
`-startupnotify`, `-blocknotify`, `-walletnotify`, and `-alertnotify` configuration parameters accept shell commands to be executed based on different events. These are useful for notifying the user of certain events; however, they can be misused by malicious software or social engineering attacks. Restrict scripts and commands in the shell/system used for bitcoind to avoid running potentially unsafe scripts.
AFAIK #23395 is not yet merged, and hence -shutdownnotify has not yet been added to master. So you should remove that from the docs.
This PR can be considered a part of #23395 so they should be merged together. It adds docs and I have suggested to add the link for this doc in release notes. There is no code.
Fixed other errors in grammar in https://github.com/bitcoin/bitcoin/commit/ad6a7f7f2a8baf4d456436a2ea89b00ccac27a0e
I have not yet made up my mind on this PR.
However, I have commented on some changes that need to consider.
.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
61 | @@ -62,3 +62,9 @@ Linux | `$HOME/.bitcoin/` | `/home/username/.bitcoin/bitcoin.conf` 62 | macOS | `$HOME/Library/Application Support/Bitcoin/` | `/Users/username/Library/Application Support/Bitcoin/bitcoin.conf` 63 | 64 | You can find an example bitcoin.conf file in [share/examples/bitcoin.conf](../share/examples/bitcoin.conf). 65 | + 66 | +### Command-line options and Security 67 | + 68 | +`-startupnotify`, `-shutdownnotify`, `-blocknotify`, `-walletnotify` and `-alertnotify` configuration parameters accept shell commands to be executed based on different events. These are useful for notifying the user of certain events; however, they can be misused by malicious software or social engineering attacks. Restrict scripts and commands in the shell/system used for bitcoind to avoid running potentially unsafe scripts.
`-startupnotify`, `-shutdownnotify`, `-blocknotify`, `-walletnotify`, and `-alertnotify` configuration parameters accept shell commands to be executed based on different events.
These are useful for notifying the user of certain events; however, they can be misused by malicious software or social engineering attacks.
Restrict scripts and commands in the shell/system used for bitcoind to avoid running potentially unsafe scripts.
`-startupnotify`, `-shutdownnotify`, `-blocknotify`, `-walletnotify`, and `-alertnotify` configuration parameters accept shell commands to be executed based on different events. These are useful for notifying the user of certain events; however, they can be misused by malicious software or social engineering attacks. Please do not run any script and command before sufficiently knowing what they do to avoid the potential risk of harm.
The difference between suggesting to restrict scripts and asking the user to examine each script before running:
Alice might be able to identify if something is malicious however this attack or security recommendation was never meant for Alice. Bob cannot identify if something is malicious and runs few commands someone suggested to use with *notify config param to fix one of the issues. Bob thinks he is doing the right thing but he is running a command which could end up in losing all his money.
How to avoid this? Don't examine each script. Create some restrictions that blocks any suspicious commands, scripts, connections etc. Unfortunately we don't have any such feature in Bitcoin Core so users have to do it at system level.
Examples:
PowerShell has execution polices which can be used to restrict scripts: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_execution_policies?view=powershell-7.2
Macros can be used to run malicious scripts in Word documents. However we have settings to disable it (restriction). Attackers still try weird things using social engineering and many are successful.
https://www.ncsc.gov.uk/guidance/macro-security-for-microsoft-office
It might be better to use-*notify instead. This removes the need to constantly update this doc when a new -*notify parameter is added. (Just a suggestion, feel free to ignore)
`-*notify` configuration parameters accept shell commands to be executed based on different events. These are useful for notifying the user of certain events; however, they can be misused by malicious software or social engineering attacks. Restrict scripts and commands in the shell/system used for bitcoind to avoid running potentially unsafe scripts.
Concept ACK
It makes sense to include docs about the safety measures one should consider before using a feature that can be potentially used in harmful ways.
The overall framing of the added documentation looks good. However, I have some suggestions that you might consider:
ACK ad6a7f7f2a8baf4d456436a2ea89b00ccac27a0e. Thanks for picking this up!
Approach NACK
Users should never make config changes which they do not understand. Furthermore, users should always be wary of accepting any config changes provided to them by another source (even if they believe they understand them). If this is the real concern, perhaps including a general warning in the documentation is warranted.
However, I disagree with providing a specific warning for a subset of options (i.e the -notify commands) as it implies that the -notify commands are introducing an exploit or are somehow less safe than other command-line options. This is not true.
I also disagree with this approach of warning against specific, hypothetical attacks for each option when the underlying message in each warning is "don't change anything in bitcoin.conf if you don't fully understand the change and don't accept changes from other people."
In summary, I think the suggested changes are misleading and do nothing to help protect the end-user from the hypothetical attacks described.
However, I disagree with providing a specific warning for a subset of options (i.e the -notify commands) as it implies that the -notify commands are introducing an exploit or are somehow less safe than other command-line options. This is not true.
This is true because *notify config options are the only options that allow executing commands or scripts based on certain event. Let me know if you know other config option in Bitcoin Core that do the same thing.
I also disagree with this approach of warning against specific, hypothetical attacks for each option when the underlying message in each warning is "don't change anything in bitcoin.conf if you don't fully understand the change and don't accept changes from other people."
This is not about bitcoin.conf. These options can be used from command line or with shortcuts in Windows.
What other approach do you think should help in documenting this behavior?
In summary, I think the suggested changes are misleading and do nothing to help protect the end-user from the hypothetical attacks described.
No they are not misleading. They just describe what *notify config options do and what can they can be used for. If we can't document these things we should probably remove all these fancy *notify config options or change their implementation to make them secure.
@klementtan replaced all notify options with *notify @shaavan I tried adding line break but that gives error in CI. Other thing is answered above and can't be changed because it has a different meaning.
*notifyconfig options are the only options that allow executing commands or scripts based on certain event
As I mentioned, the real threat is setting config options that the user does not understand and that were potentially provided from another source with malicious intent. There is no additional risk or extra exploit introduced by the -*notify commands. To imply that they are unsafe or riskier than other config options is untrue.
This is not about bitcoin.conf. These options can be used from command line or with shortcuts in Windows.
You are proposing changes to bitcoin-conf.md. This document is for documenting how to use the bitcoin.conf file and is not the right place for general security advice or describing hypothetical attacks.
What other approach do you think should help in documenting this behavior?
As I mentioned above, perhaps a general warning against making changes not fully understood by the user and against accepting configs from another source is warranted. I'd suggest opening a new issue specific to bitcoin-conf.md to deliberate on the wording.
This document is for documenting how to use the bitcoin.conf file and is not the right place for general security advice or describing hypothetical attacks.
This document has lot of things mentioned about command line options. I could not find a better place in this repository to document it.
I'd suggest opening a new issue specific to bitcoin-conf.md to deliberate on the wording.
You can suggest wording in this pull request. I hope you have read everything mentioned in #23412 so that we are on the same page
reACK https://github.com/bitcoin/bitcoin/pull/23850/commits/fe0847aa7a9b757df2e1e9dec301d600edc32cac
Only one change that would avoid updating this doc if more notifiy parameters are added.
Restrict scripts and commands in the shell/system used for bitcoind to avoid running potentially unsafe scripts.
NAK.
The recommended mediation is simply absurd and could not be acted on by anybody. It assumes a user is competent enough to set up a system with absolutely no access to any executables or files whatsoever, but is inept enough that they will happily modify their bitcoin.conf at the behest of an attacker in order to have a backdoor loaded using blocknotify which would then steal their funds.
blocknotify="bitcoin-cli sendtoaddress 1BitcoinEaterAddressDontSendf59kuE 999999999999999999"
Bitcoin Core has the backdoor built in, in any case.
bitcoind -shrinklogfile -debuglogfile=wallet.dat
For extra safety, suggest disallowing Bitcoin Core from writing files.
but is inept enough that they will happily modify their bitcoin.conf at the behest of an attacker in order to have a backdoor loaded
This was never about bitcoin.conf maybe read everything before reviewing a pull request.
This documentation would have allowed *notify options to be added but users made aware of known issues. Since its being blocked I think we should remove all *notify options and a better way to educate users or document would be to submit CVE and post on bitcoin-dev mailing list.
would be to submit CVE
What would you be submitting a CVE for?
Known issues acknowledged by one of the maintainers which neither anyone wants to fix nor document.