zebra icon indicating copy to clipboard operation
zebra copied to clipboard

docs(audit): List of dependencies

Open oxarbitrage opened this issue 1 year ago • 3 comments

Motivation

Currently this PR has all zebra dependencies in different categories. This PR is not to be merged (at least not as it is) but for people to make changes.

https://github.com/ZcashFoundation/zebra/issues/5214

Specifications

We need to figure out if this is the categories and format we want or if we want to do something else before making too much more progress on it.

Please submit PRs to this branch instead of making a lot of suggestions specially if the changes are too big. Thanks.

oxarbitrage avatar Oct 12 '22 18:10 oxarbitrage

Also, just wondering if the versions are the same as the ones from the audit tag? https://github.com/ZcashFoundation/zebra/releases/tag/v1.0.0-rc.0

I don't know if we changed any important dependency versions between when you last ran the tool. (We can't use the main branch now, we've changed some versions since we tagged.)

Did you want to leave checking the versions and links until later? We could just do it for the ones that are being audited.

teor2345 avatar Oct 13 '22 01:10 teor2345

@oxarbitrage I think this should be a high priority so we can kick off the audit. What do you think?

teor2345 avatar Oct 13 '22 01:10 teor2345

We should be very careful about what we mean by auditing Zebra dependencies. It's ok to audit our use of external dependencies but asking the auditors to look at non-Zebra code that we have not written ourselves is too much. It's not our responsibility to get external dependency code audited.

mpguerra avatar Oct 14 '22 13:10 mpguerra

Action Items from the meeting:

  • include ed25519-zebra
  • split the list into full audit, partial audit, and out of scope
  • ask ECC if incrementalmerkletree has been audited
  • remove zebra-chain cryptographic implementations that are unused, because they are only for full-node clients
  • tag another audit release candidate which includes the code changes

Questions:

  • Is there anything that needs a specialist cryptographic audit?
    • ed25519-zebra
    • cryptographic parts of zebra-chain

teor2345 avatar Oct 18 '22 21:10 teor2345

  • remove zebra-chain cryptographic implementations that are unused, because they are only for full-node clients

@mpguerra we might need a separate ticket or PR for these code changes?

teor2345 avatar Oct 18 '22 22:10 teor2345

It seems the final list is just:

Name Notes
tower-batch
tower-fallback
zebra-chain There are a few cryptographic implementations that are unused
zebra-consensus
zebra-network
zebra-node-services
zebra-rpc
zebra-script
zebra-state
zebrad
zebra-utils Only zebra-checkpoints utility needs to be audited.
ed25519-zebra

I dont think we should tag a new release or do much more here, we already did too much just to end with a list of our crates.

oxarbitrage avatar Oct 19 '22 11:10 oxarbitrage

I dont think we should tag a new release or do much more here, we already did too much just to end with a list of our crates.

The original tasks in the ticket were:

We will need 3 lists:

  • [ ] Direct ECC dependencies that are in scope for the audit (most will be out of scope as they have already been audited)
  • [ ] Direct Zebra dependencies that are in scope as they are consensus or security critical
  • [ ] All other direct dependencies which are out of scope, including the ECC ones.

We wanted a list of all the dependencies, so we could say which dependencies are definitely out of scope, rather than leaving the auditors to guess.

I don't think the latest list is complete yet, because:

I'd like to include the download code from zcash_proofs, because we wrote it, and the Zcash parameters are consensus-critical.

We don't want the auditors to waste time reviewing unused cryptographic code with known bugs, so we want to make this change:

  • remove zebra-chain cryptographic implementations that are unused, because they are only for full-node clients

And then we want to give them a tag with that code removed, so we need to:

  • tag another audit release candidate which includes the code changes

teor2345 avatar Oct 19 '22 19:10 teor2345

Looks pretty close to done.

Do we still want this to be split up by full audit, partial audit, and out of scope categories, or do the bold and italicized items suffice?

Yes, we do want it split up by categories :)

mpguerra avatar Oct 25 '22 10:10 mpguerra

@mergifyio update

teor2345 avatar Oct 26 '22 23:10 teor2345

update

✅ Branch has been successfully updated

mergify[bot] avatar Oct 26 '22 23:10 mergify[bot]

@arya2 do you think this is done ? if so, can you approve, i think we need this for the next release. Thanks.

oxarbitrage avatar Oct 28 '22 14:10 oxarbitrage

@arya2 discard this, it seems this is the other way around. This should be merged after we tag, so no rush. Thanks.

oxarbitrage avatar Oct 28 '22 14:10 oxarbitrage

I think we might want to just link to the things we actually want audited, not main branches:

  • #5523

teor2345 avatar Nov 01 '22 03:11 teor2345