search

bitcoin-core / secp256k1

Optimized C library for EC operations on curve secp256k1
MIT License
2.08k stars 1k forks source link

Integrating with OSS-Fuzz #739

Closed Google-Autofuzz closed 4 years ago

Google-Autofuzz commented 4 years ago

Greetings secp256k1 developers and contributors,

We’re reaching out because your project is an important part of the open source ecosystem, and we’d like to invite you to integrate with our fuzzing service, OSS-Fuzz. OSS-Fuzz is a free fuzzing infrastructure you can use to identify security vulnerabilities and stability bugs in your project. OSS-Fuzz will:

Many widely used open source projects like OpenSSL, FFmpeg, LibreOffice, and ImageMagick are fuzzing via OSS-Fuzz, which helps them find and remediate critical issues.

Even though typical integrations can be done in < 100 LoC, we have a reward program in place which aims to recognize folks who are not just contributing to open source, but are also working hard to make it more secure.

We want to stress that anyone who meets the eligibility criteria and integrates a project with OSS-Fuzz is eligible for a reward.

If you're not interested in integrating with OSS-Fuzz, it would be helpful for us to understand why—lack of interest, lack of time, or something else—so we can better support projects like yours in the future.

If we’ve missed your question in our FAQ, feel free to reply or reach out to us at oss-fuzz-outreach@googlegroups.com.

Thanks!

Tommy OSS-Fuzz Team

gmaxwell commented 4 years ago

Is it just be or does the FAQ here not say anything about the disclosure process if an issue is found?

IIRC Bitcoin previously didn't participate with this google program because there was some extremely short timeframe mandatory disclosure which basically made anything except blindly installed automatic updates a viable way to deploy fixes.

elichai commented 4 years ago

Is it just be or does the FAQ here not say anything about the disclosure process if an issue is found?

IIRC Bitcoin previously didn't participate with this google program because there was some extremely short timeframe mandatory disclosure which basically made anything except blindly installed automatic updates a viable way to deploy fixes.

I think this is it? https://google.github.io/oss-fuzz/getting-started/bug-disclosure-guidelines/

practicalswift commented 4 years ago

@Google-Autofuzz

Thanks for reaching out! I think the type of continous fuzzing you are suggesting is a great opportunity to avoid shipping newly introduced issues (I think we are covered with regards to existing code). Thanks!

Very strong concept ACK

@gmaxwell

IIRC Bitcoin previously didn't participate with this google program because there was some extremely short timeframe mandatory disclosure which basically made anything except blindly installed automatic updates a viable way to deploy fixes.

Some counter-arguments:

Personally I'm convinced that our users are much better off with continuous fuzzing using OSS-Fuzz than without it :)

elichai commented 4 years ago

FYI, I'm working on writing local fuzzers and integrating into autotools, this is orthogonal to oss-fuzz, but can be integrated into them if we choose we want.

(I want to learn more about fuzzers and I think this is a good opportunity)

practicalswift commented 4 years ago

Regardless if we choose to integrate directly into OSS-Fuzz or not I hope that @guidovranken will integrate libsecp256k1 in his impressive cryptofuzz project (differential cryptography fuzzing) which is already thoroughly fuzzed via OSS-Fuzz. I really love that project! :)

Update: Opened the issue https://github.com/guidovranken/cryptofuzz/issues/13 with a suggestion to test libsecp256k1 as part of the cryptofuzz effort :)

gmaxwell commented 4 years ago

practicalswift, I think your response is both misguided and highly inappropriate. This project is extraordinarily deeply fuzz tested as is. The integrated unit tests all work by fuzzing, and though they don't themselves use a whitebox harness they achieve nearly 100% condition/decision branch coverage.

Yet your response insults the current and past contributors by making it like it isn't fuzzed at all, yet that couldn't be further from the truth.

Instead this question is about inviting some additional fuzzing which might only add a small percentage to the testing work that the project does but with an unconstrained additional liability of an extremely dangerous unethical public disclosure process which indirectly ram rods users into blindly accepting binary updates.

Instead, your responses, specifically and particular along these lines have been a major factor in my decision to no longer work on this library or bitcoin.

So best of luck, I'm done subscribing to this repo.

practicalswift commented 4 years ago

@gmaxwell I'm very sorry to hear that but how am I or anyone else supposed to know about non-public fuzzing harnesses or non-public fuzzing efforts? :)

I don't know what I could have done differently to be honest: I cannot take into account non-public information that is unknown to me. Sorry if I offended or insulted you in any way: that was certainly not my intention.

FWIW I love your contributions as I've stated in https://github.com/bitcoin-core/secp256k1/pull/708#issuecomment-574412266 and multiple other similar comments. I hope you'll re-consider your decision to unsubscribe from this repo: our users are much better off with you in the project than without you in the project. We don't have to agree on everything :)

Google-Autofuzz commented 4 years ago

Regardless if we choose to integrate directly into OSS-Fuzz or not I hope that @guidovranken will integrate libsecp256k1 in his impressive cryptofuzz project (differential cryptography fuzzing) which is already thoroughly fuzzed via OSS-Fuzz. I really love that project! :)

Update: Opened the issue guidovranken/cryptofuzz#13 with a suggestion to test libsecp256k1 as part of the cryptofuzz effort :)

Thanks for this update :) Looking forward to the integration.

sipa commented 4 years ago

Thanks for reaching out, @Google-Autofuzz.

Let me offer some historical perspective here.

By its nature, Bitcoin's rules are effectively defined by the software that users choose to run. As no authority exists that can compel anyone to upgrade, some classes of code issues effectively are not bugs, but for better or worse the rules of the network. If two implementations differ, sometimes in a tiny detail, this may be exploitable to split the network in two.

This extraordinary requirement for exact consistency between implementations means that these classes of issues cannot be fixed in the time scale of software releases, but requires network-wide coordination. The most relevant example is how a platform dependent deviation from the DER standard in OpenSSL threatened a network split a few years ago; you can read about the issue and its timeline here. In that case, it took 9 months between discovery and eventual resolution, but I believe that similar issues these days would take significantly longer still.

I hope you see why the OSS-Fuzz timeline of disclosure in 90 days (+ 14 days if fixed) would not be appropriate for such issues, and thus would be hard for us to agree to.

That said, I am excited about fuzzing based testing, in this library, and in open source in general, and happy for the work that you guys are doing to promote that. As far as libsecp256k1 is concerned:

maflcko commented 4 years ago

No comment from me, just cross-links to related issues:

guidovranken commented 4 years ago
TheBlueMatt commented 4 years ago

If you're not interested in integrating with OSS-Fuzz, it would be helpful for us to understand why—lack of interest, lack of time, or something else—so we can better support projects like yours in the future.

I find this comment somewhat ironic, given OSS-Fuzz and Bitcoin Core have had lots of interactions in the past where the issues preventing its use have been made very clear, and the response from OSS-Fuzz has been somewhat hostile.

practicalswift commented 4 years ago

If you're not interested in integrating with OSS-Fuzz, it would be helpful for us to understand why—lack of interest, lack of time, or something else—so we can better support projects like yours in the future.

I find this comment somewhat ironic, given OSS-Fuzz and Bitcoin Core have had lots of interactions in the past where the issues preventing its use have been made very clear, and the response from OSS-Fuzz has been somewhat hostile.

Can you point to such a somewhat hostile response from the OSS-Fuzz people? That seems entirely out of character TBH. I have had lots of interactions with the OSS-Fuzz people and they have always been super friendly and super positive :)

My perception is that they genuinely care about open source security and regardless of whether or not their services are suitable for our project I don't think anyone in the infosec community questions the huge positive impact OSS-Fuzz/ClusterFuzz (and also the team in general) has brought to the open source security ecosystem during the last couple of years. I have nothing but love for OSS-Fuzz and the fine folks behind it! ❤️

TheBlueMatt commented 4 years ago

I dunno that this is really an appropriate venue to hash out good-actors-vs-bad (nor do I disagree that they've done good work), but feel free to peruse twitter. In any case, I think its pretty well-established that OSS-Fuzz isn't appropriate for Bitcoin Core in its current form (not sure why there was any comment to the contrary, given its been hashed out again and again), and they've made very clear they don't have any desire to adopt the changes that would be required for it to make sense. This should be closed, not sure why it was open to begin with.

jonasnick commented 4 years ago

@Google-Autofuzz thank you for this offer. While the core of this library is very well tested, other parts are evolving and have not been extensively fuzz-tested so far.

In the case of a consensus issue (see sipa's post above) there is only a small difference between a malicious actor with 0 days "disclosure" and a 90 days disclosure deadline. Would it be possible to get an exception for the Bitcoin Core project? A general disclosure deadline on the order of 15 months would be more responsible. I can not support efforts committed to disclosure policies that are irresponsible to Bitcoin Core users. This does not necessarily mean that there's an increased risk of 0-days because stakeholders in the Bitcoin ecosystem may contribute their computational resources once we have good fuzz testing harnesses.

Can you point to such a somewhat hostile response from the OSS-Fuzz people?

I did not find any discussion on the topic besides the two links Marco posted, nor previous interactions with OSS-Fuzz.

practicalswift commented 4 years ago

@sipa @jonasnick

Thanks a lot for putting forward your best arguments in a respectful and friendly way. That is consensus building the academic way at its finest (as opposed to "effective" corporate style top-down decision making where it might matter who says something instead of only what is said, and where underlings voicing counter-arguments is sometimes weirdly seen as attempts to "insult" those in power).

@Google-Autofuzz

Yesterday the question was raised on IRC about what computing resources a project like libsecp256k1 or Bitcoin Core would get if integrated with OSS-Fuzz:

<BlueMatt> i mean how many cpu hours does oss-fuzz provide? I cant imagine it is anything significantly more than the 100 cores/project full-time that we already have?
…
<sipa> if it's indeed not (significantly more than) 100 cores, there isn't much to be gained from using it

For us to better understand the trade-offs we're facing here, could you please clarify? :)

Full context from #bitcoin-core-dev:

<elichai2> ariard  told me there is past interaction with oss-fuzz and it was decided that the disclosure policy doesn't fit Core, is that right? I'm interested to hear about it :)
<sipa> ping BlueMatt
<sipa> though if i remember correctly, at the time there also was no (meaningful) fuzzers for bitcoin core, so it was mostly a philosophical point
<BlueMatt> elichai2: yea, oss-fuzz has some rather-strict rules about how they will disclose issues publicly after N days of reporting them.
<BlueMatt> elichai2: at the time, and imo quite reasonably, we decided that was completely unacceptable for a p2p consensus system which may, depending on the type of failure, require network-wide update for something to be safe, and some rapid disclosure policy is not compatible with that
<BlueMatt> elichai2: also note that oss-fuzz primarily just provides cpu hours, it doesn't do any actual implementation work for you, and getting access to a few hundred cores for bitcoin core fuzzing is rather easy :p
<sipa> BlueMatt: on the other hand, we currently have fuzzers for the codebase (yay), and we can't prevent anyone from running them secretly on a massive cluster
<BlueMatt> (as a reminder: lots of cpu cores are available in a few locations for those doing bitcoin open source work)
<BlueMatt> sipa: sadly they're mostly all kinda boring targets :(
<BlueMatt> but we *have* a few rather reasonably-sized clusters in various places...
<sipa> BlueMatt: have you paid attention to the more recently added ones?
<BlueMatt> dont think any are currently fuzzing core, but...
<BlueMatt> ah, marco's fuzz stuff got merged! I missed that.
<sipa> so perhaps... either we keep the more delicate ones private (how?), or we're ok with having the ones we have publicly participate in oss-fuzz?
<BlueMatt> well, someone should give them more cpu hours. afair marco had previously done stuff, but...
<sipa> of course - we can also just increase how much cpu we spend ourselves
<BlueMatt> or we could, you know, use the cluster(s) we have to get fuzz hours :)
<sipa> well, or both
<BlueMatt> there's probably at least 120 cpu cores lying around for such usage, just need someone to step up and write scripts
<BlueMatt> i mean how many cpu hours does oss-fuzz provide? I cant imagine it is anything significantly more than the 100 cores/project full-time that we already have?
<BlueMatt> and the tradeoffs for using it...kinda suck
<sipa> that's a great question
<BlueMatt> we could also ask osuosl if heir CI cluster has free cores: https://osuosl.org/services/hosting/details/
<sipa> if it's indeed not (significantly more than) 100 cores, there isn't much to be gained from using it
<BlueMatt> a few bitcoin groups gave them some funds afaiu
<BlueMatt> but, you know, we should probably use our own cpus first.
<BlueMatt> anyway, really just need someone to step up and manage VMs that run fuzzing. I can help a bit, I have it all set up for rust-lightning and regularly pull ~100 cores for that, but I dont really have the bandwidth to manage that all the time, let alone also for core.
<BlueMatt> someone just let me or dongcarl know and we can get you set up with a few VMs with a bunch of cores, I presume there's soeone at blockstream who can do the same
<elichai2> BlueMatt: are these bare metal? because running something like this to detect improvements/regressions can be very helpful: https://perf.rust-lang.org/
<BlueMatt> kvm, but there are separate bare-metal servers intended for benchmarking cc jamesob
<BlueMatt> (which are slower, but bare-metal and more 'average-grade' hardware)
<elichai2> yeah but sadly you need bare metal for profiling, VMs are too dynamic for that AFAIK
<BlueMatt> right, kvm just for non-benchmark things
<BlueMatt> we have ~9 servers which are provisioned bare-metal for benchmarking
<BlueMatt> 9 being 8-outbound-1-target for ibd benchmarking :)
<BlueMatt> though you'd have to ask james as to the status of those
<elichai2> I have my own PC dedicated for profiling benchmarking, trying different system wide changes to Bitcoin Core
<BlueMatt> anyway, hardware/cpu resources isnt the issue, just gotta have someone write bash scripts :)
<BlueMatt> well, and babysit to watch for errors
<elichai2> BlueMatt: from what I looked in oss-fuzz it looks really simple, You write a simple dockerfile to compile and run and that's about it https://github.com/google/oss-fuzz/blob/master/projects/libsodium/Dockerfile
<BlueMatt> well by the time you've done that much work you might as well run it on our own hardware (without the aggressive public-disclosure timelines that may put bitcoin users at risk...)
<elichai2> and if I understand correctly Google will actually pay you if you integrate into oss-fuzz https://github.com/bitcoin-core/secp256k1/issues/739 "We want to stress that anyone who meets the eligibility criteria and integrates a project with OSS-Fuzz is eligible for a reward."
practicalswift commented 4 years ago

@TheBlueMatt

I find this comment somewhat ironic, given OSS-Fuzz and Bitcoin Core have had lots of interactions in the past where the issues preventing its use have been made very clear, and the response from OSS-Fuzz has been somewhat hostile.

Can you point to such a somewhat hostile response from the OSS-Fuzz people? That seems entirely out of character TBH. I have had lots of interactions with the OSS-Fuzz people and they have always been super friendly and super positive :)

I dunno that this is really an appropriate venue to hash out good-actors-vs-bad (nor do I disagree that they've done good work), but feel free to peruse twitter.

I fully agree, but I'm afraid the choice of venue was made when the surprising claim about previous hostility was made. Extraordinary claims require extraordinary evidence :)

As a huge fan of a.) Bitcoin Core, b.) OSS-Fuzz and c.) friendly and positive inter-human communication I was a bit saddened about the claim about previous hostility in the interactions between Bitcoin Core and OSS-Fuzz.

The only comment I could find that had a tiny bit of hostility in it was a comment where a member of one of the projects referred to the other project as "shit" (https://github.com/bitcoin/bitcoin/issues/10364#issuecomment-299996319).

maflcko commented 4 years ago

I think it is not helpful to dig out past conversations to collect evidence who insulted whom first. Let's focus on what can be done to improve this project and Bitcoin Core in the future.

When it comes to the question of integrating with oss-fuzz, it has been laid out by multiple contributors that the strict public disclosure policy is unsuitable for this project (as well as Bitcoin Core).

Until the policy changes, there is not much that can be done with regard to this issue. I suggest closing this issue for now and then potentially revisit when (1) secp256k1 has a libfuzzer harness AND (2) the disclosure policy is adjusted accordingly.

michaelfolkson commented 4 years ago

Great discussion, very informative and educational for people like me who haven't been privy to prior conversations on the topic. Agree with @MarcoFalke above barring movement on an exception for the Bitcoin Core project (@jonasnick). I certainly hope we can have similar discussions in future. (Ideally with less emotion but much better to have the discussion than not at all.)