Skip to content
This repository has been archived by the owner on Sep 4, 2022. It is now read-only.

Verifying libsodium with minisign instead of sha2 #329

Closed
wants to merge 1 commit into from

Conversation

pcrockett
Copy link

@pcrockett pcrockett commented Apr 20, 2019

This would resolve 326, and makes it so we don't have to keep updating hard-coded SHA hashes all the time. The build seems to run fine on Windows and Ubuntu.

Sorry if anything is out of order - I'm a bit new to Rust, not to mention contributing to open source projects in general.

@coveralls
Copy link

coveralls commented Apr 20, 2019

Pull Request Test Coverage Report for Build 931

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.537%

Totals Coverage Status
Change from base Build 929: 0.0%
Covered Lines: 2907
Relevant Lines: 3075

💛 - Coveralls

@kpp
Copy link
Member

kpp commented Apr 21, 2019

Looks good. How does it affect total number of deps in dependency tree? See ‘cargo tree’

@pcrockett
Copy link
Author

pcrockett commented Apr 21, 2019

The relevant part of the dependency tree looks like this:

├── minisign-verify v0.1.1
│   ├── base64 v0.10.1 (*)
│   └── ed25519-dalek v1.0.0-pre.1
│       ├── clear_on_drop v0.2.3
│       │   [build-dependencies]
│       │   └── cc v1.0.35 (*)
│       ├── curve25519-dalek v1.1.3
│       │   ├── byteorder v1.3.1 (*)
│       │   ├── clear_on_drop v0.2.3 (*)
│       │   ├── digest v0.8.0
│       │   │   └── generic-array v0.12.0
│       │   │       └── typenum v1.10.0
│       │   ├── rand_core v0.3.1
│       │   │   └── rand_core v0.4.0
│       │   └── subtle v2.0.0
│       │   [build-dependencies]
│       │   ├── byteorder v1.3.1 (*)
│       │   ├── clear_on_drop v0.2.3 (*)
│       │   ├── digest v0.8.0 (*)
│       │   ├── rand_core v0.3.1 (*)
│       │   └── subtle v2.0.0 (*)
│       ├── failure v0.1.5
│       ├── rand v0.6.5
│       │   ├── libc v0.2.51 (*)
│       │   ├── rand_chacha v0.1.1
│       │   │   └── rand_core v0.3.1 (*)
│       │   │   [build-dependencies]
│       │   │   └── autocfg v0.1.2
│       │   ├── rand_core v0.4.0 (*)
│       │   ├── rand_hc v0.1.0
│       │   │   └── rand_core v0.3.1 (*)
│       │   ├── rand_isaac v0.1.1
│       │   │   └── rand_core v0.3.1 (*)
│       │   ├── rand_jitter v0.1.3
│       │   │   └── rand_core v0.4.0 (*)
│       │   ├── rand_os v0.1.3
│       │   │   ├── libc v0.2.51 (*)
│       │   │   └── rand_core v0.4.0 (*)
│       │   ├── rand_pcg v0.1.2
│       │   │   └── rand_core v0.4.0 (*)
│       │   │   [build-dependencies]
│       │   │   └── autocfg v0.1.2 (*)
│       │   └── rand_xorshift v0.1.1
│       │       └── rand_core v0.3.1 (*)
│       │   [build-dependencies]
│       │   └── autocfg v0.1.2 (*)
│       └── sha2 v0.8.0
│           ├── block-buffer v0.7.3
│           │   ├── block-padding v0.1.3
│           │   │   └── byte-tools v0.3.1
│           │   ├── byte-tools v0.3.1 (*)
│           │   ├── byteorder v1.3.1 (*)
│           │   └── generic-array v0.12.0 (*)
│           ├── digest v0.8.0 (*)
│           ├── fake-simd v0.1.2
│           └── opaque-debug v0.2.2
│           [dev-dependencies]
│           └── digest v0.8.0 (*)

Notice that the sha2 package is still in there even though we're not depending on it directly.

On the master branch, I count 47 unique dependencies total. On the minisign branch, 64.

@kpp
Copy link
Member

kpp commented Apr 21, 2019

That’s a shame we need so many deps to verify hashes :(

@dnaq
Copy link
Collaborator

dnaq commented Apr 21, 2019

The ironic part is that minisign would be easy to implement without further dependencies if we had access to libsodium :)

@jedisct1
Copy link

These are just build dependencies, and are required by ed25519-dalek. I can get rid of ed25519-dalek, but is it worth it, especially for a build dependency?

@kpp
Copy link
Member

kpp commented Apr 21, 2019

but is it worth it, especially for a build dependency?

https://github.com/tox-rs/tox-node already got 195 deps and roughly half of them are ssl, http, tar and sha2 to download and verify libsodium. I thought it did not matter until I tried to compile tox-node under AWS micro and it took 12 minutes.

I believe we don't have an option, so it's a yes from me. A sad yes.

@kpcyrd
Copy link
Member

kpcyrd commented Apr 21, 2019

Keep in mind that this resolves the build failure, but the root cause that we're using mutable data in our build makes the build nondeterministic from a reproducible builds point of view. To guarantee independent verification of the binary we would need to track which rebuild of the library release was used during our build or make sure all inputs of the build are immutable (eg. by building libsodium from immutable source releases).

More on that topic can be found here: https://reproducible-builds.org/

@kpcyrd
Copy link
Member

kpcyrd commented Apr 21, 2019

Also note that this allows a subtle way for the owner of the minisign private key to carry out very targeted attacks by serving a malicious pre-built library with a valid signature. Since there's no binary transparency you can't be sure you are getting the same library everybody else is getting unless we're pinning the file content by hash.

@jedisct1
Copy link

Version 0.1.2 of the crate doesn't have any dependency any more besides base64.

@kpp
Copy link
Member

kpp commented Apr 21, 2019

serving a malicious pre-built library with a valid signature

That is so true.

@pcrockett
Copy link
Author

That is a really good point about reproducible builds. Especially when considering what kind of downstream projects could be using this library, reproducible builds would be a good feature to provide. I wonder if it would be worthwhile maintaining our own libsodium repo by forking it, and using the GitHub Releases feature to host libsodium releases in an immutable way (i.e. 1.0.17.1, 1.0.17.2, etc.).

And to be clear, I'm not suggesting forking the project. Just forking the repository and keeping it in sync with the original.

@kpp
Copy link
Member

kpp commented Apr 27, 2019

Closed in favor of #332

@kpp kpp closed this Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants