search

zingolabs / zingolib

An API and test-app that exposes zcash functionality for app consumption
MIT License
15 stars 23 forks source link

Consistently use `rustls` across the entire workspace. #1207

Open nejucomo opened 3 months ago

nejucomo commented 3 months ago

Summary

Across the whole workspace, there are two TLS crates (rustls and openssl-sys) and 3 distinct versions used (shown below). One improvement would be to use a single crate. Another improvement would be to use a single version of a single crate (to minimize the "bug / security issue window").

Selecting a Single Crate

My understanding of these two options are:

My personal recommendation would be to rely only on rustls primarily because of rust's safety features and a newer leaner code base.

Pinning a Single Version

I haven't investigated how to get the workspace to (transitively) depend on a single rustls version yet.

Analysis

$ cargo tree -i openssl-sys
openssl-sys v0.9.102
├── native-tls v0.2.12
│   ├── hyper-tls v0.6.0
│   │   └── reqwest v0.12.4
│   │       └── zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib)
│   │           ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│   │           ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│   │           ├── zingo-cli v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingocli)
│   │           └── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils)
│   │               ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│   │               ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│   │               └── zingo-cli v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingocli)
│   ├── reqwest v0.12.4 (*)
│   └── tokio-native-tls v0.3.1
│       ├── hyper-tls v0.6.0 (*)
│       └── reqwest v0.12.4 (*)
└── openssl v0.10.64
    └── native-tls v0.2.12 (*)

$ cargo tree -i rustls
error: There are multiple `rustls` packages in your project, and the specification `rustls` is ambiguous.
Please re-run this command with one of the following specifications:
  rustls@0.20.9
  rustls@0.21.1

$ cargo tree -i rustls@0.20.9
rustls v0.20.9
├── hyper-rustls v0.23.2
│   └── zingo-netutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-netutils)
│       ├── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils)
│       │   ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│       │   ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│       │   └── zingo-cli v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingocli)
│       └── zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib)
│           ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│           ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│           ├── zingo-cli v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingocli)
│           └── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils) (*)
└── tokio-rustls v0.23.4
    ├── hyper-rustls v0.23.2 (*)
    └── zingo-netutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-netutils) (*)

$ cargo tree -i rustls@0.21.12
rustls v0.21.12
├── tokio-rustls v0.24.1
│   └── tonic v0.10.2
│       ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│       ├── zcash_client_backend v0.12.1 (https://github.com/zingolabs/librustzcash.git?tag=fix_change_required#471a062d)
│       │   ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│       │   ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│       │   ├── zingo-memo v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-memo)
│       │   │   └── zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib)
│       │   │       ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│       │   │       ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│       │   │       ├── zingo-cli v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingocli)
│       │   │       └── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils)
│       │   │           ├── darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)
│       │   │           ├── libtonode-tests v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/libtonode-tests)
│       │   │           └── zingo-cli v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingocli)
│       │   ├── zingo-netutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-netutils)
│       │   │   ├── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils) (*)
│       │   │   └── zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib) (*)
│       │   ├── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils) (*)
│       │   └── zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib) (*)
│       ├── zingo-netutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-netutils) (*)
│       ├── zingo-testutils v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/zingo-testutils) (*)
│       └── zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib) (*)
└── tonic v0.10.2 (*)

How to Remove openssl-sys

It looks like the only thing depending on openssl-sys from this workspace is transitively via reqwest which has a feature flag to use rustls instead of "native" implementations: rustls-tls

So I think the fix is a one-liner to add that feature.

Discovery

I discovered this issue by cloning the repo then doing cargo build on a nix system which was configured only to build pure rust projects without any "system" dependencies. The build failure comes from a build.rs script failing to invoke pkg-config which is a standard linux utility to query for C library paths / metadata:

$ cargo build
   Compiling serde_derive v1.0.203
   Compiling tracing-attributes v0.1.27
   Compiling zeroize_derive v1.4.2
   Compiling tokio-macros v2.3.0
   Compiling futures-macro v0.3.30
   Compiling thiserror-impl v1.0.61
   Compiling prost-derive v0.12.6
   Compiling secp256k1 v0.26.0
   Compiling pin-project-internal v1.1.5
   Compiling regex v1.10.4
   Compiling openssl-sys v0.9.102
   Compiling async-trait v0.1.80
   Compiling serde_json v1.0.117
   Compiling foreign-types v0.3.2
   Compiling axum v0.6.20
   Compiling rustls-native-certs v0.6.3
error: failed to run custom build command for `openssl-sys v0.9.102`

Caused by:
  process didn't exit successfully: `/home/user/src/github.com/zingolabs/zingolib/target/debug/build/openssl-sys-bc98a2ebf2dbb3d0/build-script-main` (exit status: 101)
  --- stdout
  cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR
  X86_64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_LIB_DIR
  OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR
  X86_64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_INCLUDE_DIR
  OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR
  X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_DIR
  OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_NO_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG
  cargo:rerun-if-env-changed=OPENSSL_STATIC
  cargo:rerun-if-env-changed=OPENSSL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
  run pkg_config fail: Could not run `PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 pkg-config --libs --cflags openssl`
  The pkg-config command could not be found.

  Most likely, you need to install a pkg-config package for your OS.
  Try `apt install pkg-config`, or `yum install pkg-config`,
  or `pkg install pkg-config`, or `apk add pkgconfig` depending on your distribution.

  If you've already installed it, ensure the pkg-config command is one of the
  directories in the PATH environment variable.

  If you did not expect this build to link to a pre-installed system library,
  then check documentation of the openssl-sys crate for an option to
  build the library from source, or disable features or dependencies
  that require pkg-config.

  --- stderr
  thread 'main' panicked at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/openssl-sys-0.9.102/build/find_normal.rs:190:5:

  Could not find directory of OpenSSL installation, and this `-sys` crate cannot
  proceed without this knowledge. If OpenSSL is installed and this crate had
  trouble finding it,  you can set the `OPENSSL_DIR` environment variable for the
  compilation process.

  Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

  If you're in a situation where you think the directory *should* be found
  automatically, please open a bug at https://github.com/sfackler/rust-openssl
  and include information about your system as well as this message.

  $HOST = x86_64-unknown-linux-gnu
  $TARGET = x86_64-unknown-linux-gnu
  openssl-sys = 0.9.102

  It looks like you're compiling on Linux and also targeting Linux. Currently this
  requires the `pkg-config` utility to find OpenSSL but unfortunately `pkg-config`
  could not be found. If you have OpenSSL installed you can likely fix this by
  installing `pkg-config`.
nejucomo commented 3 months ago

I began a PR to try this by simply adding rustls-tls to reqwest feature flags, and I hit a snag:

$ cargo check
error: failed to select a version for `subtle`.
    ... required by package `rustls v0.22.2`
    ... which satisfies dependency `rustls = "^0.22.2"` of package `reqwest v0.12.4`
    ... which satisfies dependency `reqwest = "^0.12"` (locked to 0.12.4) of package `zingolib v0.2.0 (/home/user/src/github.com/zingolabs/zingolib/zingolib)`
    ... which satisfies path dependency `zingolib` (locked to 0.2.0) of package `darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)`
versions that meet the requirements `^2.5.0` are: 2.5.0

all possible versions conflict with previously selected packages.

  previously selected package `subtle v2.4.1`
    ... which satisfies dependency `subtle = "^2.3"` (locked to 2.4.1) of package `orchard v0.8.0`
    ... which satisfies dependency `orchard = "^0.8"` (locked to 0.8.0) of package `darkside-tests v0.1.0 (/home/user/src/github.com/zingolabs/zingolib/darkside-tests)`

failed to select a version for `subtle` which could resolve this conflict

I have tried a variety of dependency version tweaks to get around this. Downgrading reqwest to ^0.11 resolves this transitive version conflict, but the resulting build still relies on openssl-sys, so I'm unsure how to proceed.