Skip to content
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

bx seed warning can be ignored #728

Closed
evoskuil opened this issue Aug 11, 2023 · 20 comments
Closed

bx seed warning can be ignored #728

evoskuil opened this issue Aug 11, 2023 · 20 comments

Comments

@evoskuil
Copy link
Member

I suggest just removing the command, as users may not bother to read or understand even explicit warnings. This precludes its future unsafe use even based on external documentation outside of our control.

Removal consists of marking the command as obsolete in the commands xml and regenerating. Related /src and /test sources may then be removable (verify based on other obsoleted commands).

This requires that all documentation examples (wiki) be updated to use hardwired entropy data. This alone may result in people using that data to create a live wallet. So documentation for commands that accept entropy should provide a warning to provide your own true entropy for live wallet usage (such commands should already have similar help). This amounts to the same type of documentation that existed for seed but will instead be spread across each entropy-accepting command.

Published bx binaries should be removed and a notice sent out via the libbitcoin slack and mailing list with the breaking change (obsoletion) announcement. Minor (middle) version number should be bumped in the build xml, with artifact regeneration after command are updated.

@thecodefactory
Copy link
Member

Examples with hardcoded entropy are slightly less secure than using the existing bx seed when copied verbatim, which is sadly likely. The documentation updates should be sufficient (IMO). Maybe instead bx seed should output a fixed seed random number to illustrate even to developers that don't read about it that it can't be secure?

@evoskuil
Copy link
Member Author

evoskuil commented Aug 11, 2023

It does suck to lose the convenience of it for demos, and the risk will now shift to copy/paste of constants. But that’s probably lesser and also eliminates risk of existing poor/dated third party documentation.

A fixed seed output may not actually be noticed.

@awnumar
Copy link

awnumar commented Aug 11, 2023

Why couldn't the seed command have called the operating system CSPRNG for entropy? This would've been sufficient for demos.

@evoskuil
Copy link
Member Author

We removed all references to OS entropy, as they cannot be secured. Retaining it for this one command would serve no purpose but to imply it was somehow safe (and people would presumably also have used it for live seeding). We instead reused the pseudorandom class from libbitcoin-system for demo seeding. This class exists for support of other library systems that do not require true randomness, which is the same requirement for demo seeding.

@awnumar
Copy link

awnumar commented Aug 11, 2023

We removed all references to OS entropy, as they cannot be secured.

What specifically is meant by "secure" in this context?

@evoskuil
Copy link
Member Author

evoskuil commented Aug 11, 2023

Guarded against injection. Both the hardware and OS can be manipulated. When accepting a seed from the OS there is no way for a user to ensure this is not the case.

By rolling dice (for example) the user can at least prove whether a given platform has altered a resulting key by performing the same key generation on one or more other machines using the same dice entropy. This is not possible when trusting the OS.

We made the decision years ago to remove all use of rand() for this reason. It was not acceptable to embed trust in the platform. This is the reason people roll dice. Trusting the OS is unsecurable. There is no way to measure how insecure it is at any given time, or to predict when widespread exploits in hardware generators or OS builds may occur. But the more Bitcoin value that is out there, the greater is the bounty for such attacks. There could also be state-level targeted attacks, as we are well aware that these do in fact occur.

kulpreet added a commit to kulpreet/libbitcoin-explorer that referenced this issue Aug 11, 2023
@awnumar
Copy link

awnumar commented Aug 12, 2023

Both the hardware and OS can be manipulated.

I'm not sure I understand the threat model. If the hardware or kernel is compromised then any user-space software is also compromised. That invalidates all cryptographic software ever written, including all software wallets, exchanges, miners, and any software deployed for mission critical security applications, e.g. financial institutions and certificate authorities. It's a nuclear assumption.

Someone could have an air gapped machine with a verified kernel and hardware. Why's it assumed all systems are untrustworthy, can't users decide for themselves whether they trust their own systems?

Retaining it for this one command would serve no purpose but to imply it was somehow safe

Don't you think providing a seed command in a software called "The Bitcoin Command Line Tool" implies some level of safety?

OS random is widely trusted to be a reasonably good cryptographic randomness source. Using it would've protected users from misuse while having no impact on the tool's use as a demo. Someone who doesn't trust their system would've "rolled dice" anyways and so wouldn't be affected by any particular implementation. Who benefited from weak randomness being used here?

Removing the command now is sensible, though originally calling it insecure-seed might have avoided a great deal of pain

@evoskuil
Copy link
Member Author

evoskuil commented Aug 12, 2023

The command is not “insecure”, it is not for live wallet use. There is a difference. Using it as documented presents no security issue.

It is not true that since a platform can be compromised one might as well accept any seed the platform may provide. A platform’s trustworthiness for entropy cannot be determined, but your own entropy can be trusted, and as I previously pointed out, its uncorrupted use can be verified. This is why we do not accept platform seeds. The fact that others may accept such insecurity is their issue.

This is not a proper forum to second guess what might have been done differently. Please take this to the slack channel or mailing list if you find that interesting.

kulpreet added a commit that referenced this issue Aug 13, 2023
@kulpreet
Copy link
Member

@kulpreet
Copy link
Member

Shared a patch on slack to remove bx seed references from the libbitcoin-explorer wiki.

Once that is pushed, we need to

  • make the binaries, sign them and release them
  • make announcements

@advanture-space
Copy link

@evoskuil

Quote from today in libbitcoin-explorer wiki

$ bx seed | bx hd-new

Insecure.

Also in the bitcoin wiki up to 9 August 2023‎ the version created by you on 12 July 2015‎ you in Bitcoin wiki:

$ bx seed | bx ec-new | bx ec-to-public | bx ec-to-address

Insecure.

The commit which made all hacked hackable was done on 21 Oct 2016.

Also the commit to Mastering Bitcoin book using bx seed.

The timeline...

  1. You invented and promoted bx seed. Injected it into at least two projects (bitcoin wiki and Mastering Bitcoin book).
  2. There was bx seed that was kinda secure at the time and even now. At least there was no known hackable way. There was no proof of concept available on how to hack it. Granted, some theoretic attacks are possible.
  3. You changed bx seed to a known hackable state.
  4. It got exploited because of your entropy reduction.
  5. Yet after this mess, you blame it on reckless wallet development and claim bx seed works as intended.

How you can blame Mastering Bitcoin book and Bitcoin wiki where you yourself made the edits?

What are other places where you or others promoted the use of bx seed? These need to be notified and fixed too.

Then also bitcoin.stackexchange.com has a many answers with examples using bx seed. Nobody got it right.

Nice commentary by @gmaxwell in If you used "bx seed" you probably already lost your bitcoins, but if...:

Basically it seems they changed from code that had security problems in theory but usually not (and maybe never) in practice to code that was certainly broken all the time for everyone in both theory and practice.

@evoskuil
Copy link
Member Author

evoskuil commented Aug 14, 2023

Greg has always had an ax to grind here - and is a widely-reputed sock puppeteer, so this sock puppet account quoting his theory may just as well be him.

He probably never advocated that key security should be “maybe secure” or “usually secure”, or dismissed “theoretical security problems” before this, but here he is promoting that condition when it serves to support his theory in the service of maligning Libbitcoin.

IMG_1408

@advanture-space
Copy link

How about you address the content rather than personal stuff?

@evoskuil
Copy link
Member Author

I see no problem with addressing both at once, as I did.

@evoskuil
Copy link
Member Author

evoskuil commented Aug 20, 2023

Since release 3.7.0 had been previously defined in source (but not tagged for release), it could have led to misinterpretation of the version with seed removed. Therefore @pmienk updated the current version to 3.8.0 and tagged it for release. So technically there will never have been a 3.7.0 release.

@evoskuil
Copy link
Member Author

@kulpreet has updated the bx documentation (wiki). All examples of the seed command have been replaced with echo [user entropy] (except for the seed command page itself). The seed page is no longer indexed but is retained due to the presence of its warning, in the case where someone has an old version.

@evoskuil
Copy link
Member Author

The following notice has been posted to Slack and the libbitcoin email list:

The following CVE has been published in relation to BX.

https://nvd.nist.gov/vuln/detail/CVE-2023-39910

Libbitcoin developers have:

* Removed the `seed` command from version3 (and master).
* Tagged a new 3.8.0 release (without the command).
* Replaced `bx seed` with `echo [user entropy]` in certain documentation examples.
* Removed precompiled binaries for previous bx versions.
* Submitted the two items of feedback below to MITRE (CVE maintainers).
* Posted this notice to the libbitcoin Slack and mailing list.

----------------------------------------------------------------------------------

The libbitcoin open source team rejects the implication that the issue is of "bad cryptography" and accepts that people may have used the command in a manner not intended (and warned against). Consequently we have simply removed the command in updated version 3.8.0. All examples in the provided documentation have been updated to replace `bx seed` with `echo [user entropy]`. Precompiled binaries of older versions previously available for download have been removed. As such there is no longer any potential for the command to be misused in the current release.

https://github.com/libbitcoin/libbitcoin-explorer/releases/tag/v3.8.0
https://github.com/libbitcoin/libbitcoin-explorer/wiki/bx-hd-new#example-4
https://github.com/libbitcoin/libbitcoin-explorer/wiki/Download-BX

----------------------------------------------------------------------------------

From the CVE Description:

"...NOTE: the vendor's position is that there was sufficient documentation advising against "bx seed" but others disagree."

The question is not of "sufficiency", but intent. The bx 3.x `seed` command was not intended for live wallet use. The warning against it is even referenced in the "Milk Sad" article. The fact that some people appear to have used the command for live wallet seeding implies that documentation may have been "insufficient", however it is ultimately impossible to force people to read, understand and respect documentation. In fact we have been advised by the authors of this CVE that the two parties identified in their research did not even read the documentation for the `seed` command - they assumed its behavior.

It is a design goal that users provide entropy on the command line (or by parameter to any API that requires it), which is why the command is not necessary. It was provided as a convenient way to obtain properly formatted and sized (length, not strength) pseudo entropy for the vast majority of use cases. bx demonstrates C++ calling conventions and usage of many of the APIs exposed by the 9 other libraries, tying those calls to real-world scenarios in order to provide context and therefore better understanding of what are often difficult topics. bx is not a wallet - manual creation of live wallets using the "development toolkit" requires significant expertise and caution and is not advised.

Suggested description:

"The Bitcoin developer toolkit utility application Libbitcoin Explorer (bx) provides a pseudo entropy command not intended for live cryptocurrency wallet seeding, yet analysis implies that some users may have not heeded warnings against such use. This allows remote attackers to recover any wallet private keys generated from "bx seed" entropy output and steal funds. (Affected users need to move funds to a secure new cryptocurrency wallet.) NOTE: this was exploited in the wild in June and July 2023."

https://github.com/libbitcoin/libbitcoin-explorer/wiki/Random-Numbers
https://github.com/libbitcoin/libbitcoin-explorer/wiki/bx-cert-new
https://github.com/libbitcoin/libbitcoin-explorer/wiki/bx-seed

----------------------------------------------------------------------------------

This completes the actions described above.

@evoskuil
Copy link
Member Author

evoskuil commented Sep 6, 2023

A full report has been posted, with inputs from @evoskuil, @pmienk, @kulpreet, and @thecodefactory. Links have been issued through Slack, Listserv, and Twitter.

@ariard
Copy link

ariard commented Apr 22, 2024

A full report has been posted, with inputs from @evoskuil, @pmienk, @kulpreet, and @thecodefactory. Links have been issued through Slack, Listserv, and Twitter.

Thanks for the full report, I must say I can only salute the professionalism in the response from the Libbitcoin maintenance team.

I had an in-depth look at the milk.sad disclosure by their core team (Distrust + Independent) when it was published during the summer of 2023. Reading it, at the time I echoed my concern in private to few other experts on the technical motivation behind libbitcoin-system’s PR 559. If I remember correctly I voiced concern on naively using system pseudo random function. I was unaware that PR 559 which is hardening pseudo-random number generation came from ground reports on the Libbitcoin mailing list.

On the lack of redundant documentation in matters of cryptographic / secure API, I can only say this is a common weakness of many large-scale open-source projects (Bitcoin Core included), so I don’t think this is something which can be put on the discredit of Libbitcoin.

Distinction between CSPRNG and PRNG is somehow known since the ~2000s in security literature, so if you’re designing a key-management system for production entreprise, you certainly will read twice or three times the randomness generation flow. As it’s underscored in your full report, this is more likely that the friends or professional connections of the milk.sad
vulnerability report core team forgot to do so, in the state of information which have been publicly published on milk.sad and your report.

Note the earlier comment, "We discovered this after being affected through more than one of these wallets, with funds lost > on our end as well." We were later informed that their loss constituted a substantial part of the whole and that they had not > read the bx seed documentation. In other words, the authors failed to disclose that they were personally and materially > affected, and faulted the implementation of Libbitcoin despite documentation to the contrary which they did not actually > read.

This conflict of interest may explain why they were quick to dismiss the possibility of user error and/or poor documentation > despite the evidence that the behavior was intended. Additionally several members of the write-up team are advertising > their commercial consulting services on the back of the disclosure.

Finally, I can only share the expressed opinion on the apparent conflict of interests by the milk.sad group of authors. When you’re publishing a vulnerability as a full disclosure, I think one ethical rule from the decades-old infosec tradition is to decently segregate your open-source and commercial interests. Certainly not attaching a “please hire me” for commercial work at the end of a full-write up public disclosure report, and I’m speaking with a lot of experience...

@evoskuil
Copy link
Member Author

Thanks for the taking the time to provide this feedback @ariard

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

No branches or pull requests

7 participants
@kulpreet @evoskuil @thecodefactory @awnumar @ariard @advanture-space and others