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

Barrier of entry #488

Open
LeCyberDucky opened this issue Aug 10, 2022 · 4 comments
Open

Barrier of entry #488

LeCyberDucky opened this issue Aug 10, 2022 · 4 comments

Comments

@LeCyberDucky
Copy link

Hey there!

I've been wanting to step up my Rust security game for ages now. Therefore, I just decided to install cargo-crev, but I was immediately reminded as to why I had given up on it during earlier attempts: It doesn't just work. Upon the first step of the tl;dr in the getting started guide, I'm greeted by:

error: failed to run custom build command for `openssl-sys v0.9.75`

Caused by:
  process didn't exit successfully: `C:\Users\LeCyberDucky\AppData\Local\Temp\cargo\target\release\build\openssl-sys-6b7fc20198a5049a\build-script-main` (exit code: 101)
  --- stdout
  cargo:rustc-cfg=const_fn
  cargo:rerun-if-env-changed=X86_64_PC_WINDOWS_MSVC_OPENSSL_NO_VENDOR
  X86_64_PC_WINDOWS_MSVC_OPENSSL_NO_VENDOR unset
  cargo:rerun-if-env-changed=OPENSSL_NO_VENDOR
  OPENSSL_NO_VENDOR unset
  running "perl" "./Configure" "--prefix=C:\\Users\\LeCyberDucky\\AppData\\Local\\Temp\\cargo\\target\\release\\build\\openssl-sys-fc4a11b1e1756f30\\out\\openssl-build\\install" "--openssldir=SYS$MANAGER:[OPENSSL]" "no-dso" "no-shared" "no-ssl3" "no-unit-test" "no-comp" "no-zlib" "no-zlib-dynamic" "no-md2" "no-rc5" "no-weak-ssl-ciphers" "no-camellia" "no-idea" "no-seed" "no-engine" "no-asm" "VC-WIN64A"

  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: NotFound, message: "program not found" }', C:\Rust\cargo\registry\src\github.aaakk.us.kg-1ecc6299db9ec823\openssl-src-111.22.0+1.1.1q\src\lib.rs:488:39
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
error: failed to compile `cargo-crev v0.23.2`, intermediate artifacts can be found at `C:\Users\LeCyberDucky\AppData\Local\Temp\cargo\target`

The documentation at https://docs.rs/cargo-crev/latest/cargo_crev/doc/user/getting_started/index.html refers to the build instructions in case of compilation errors: https://docs.rs/cargo-crev/latest/cargo_crev/doc/user/getting_started/compiling.md
That page doesn't exist. Some Googling has lead me to believe that I need to install OpenSSL to fix things. See this post, for example:
https://www.reddit.com/r/rust/comments/uw5lhg/openssl_wont_compile_on_windows/

There, somebody also mentions that the hassles of OpenSSL could be avoided by using rust-tls instead. Before randomly installing OpenSSL, though, I decided to take a closer look at this repository, so I searched the closed issues for anything relevant, and I ended up finding #372 which I believe is the issue that I'm facing. I haven't read through that issue carefully yet, because I wanted to write this issue before putting it off again.

I gave this project a try again just now, because I was just about to add a dependency to my project that I didn't feel entirely comfortable with. But, in all honesty, when faced with such issues, I'm more likely to just go ahead and add the random dependency than spending my evening trying to fix things. That's how things have gone in the past, and the only difference this time is that I decided to create this issue instead of just shrugging it off again. Now, I'm well aware that this may sound very entitled, but that's not my point at all. My point is just that I'm lazy. And I believe a lot of security issues arise because other people are lazy as well. Therefore, I think it's very unfortunate for such an important project to have such annoyances when trying to get started. I think that making this as user friendly as possible would be very valuable step towards the goal of the project. In that regard, I have three suggestions that I think would be helpful:

  • Prevent this compilation error from happening.
  • Document the problem better. If this is a known issue, I think the getting started guide should point it out.
  • Provide the Windows binaries as zip files (this is minor, but on Windows I can natively unpack zip files. I don't immediately know how to handle the provided tar.gz files).

I apologize in case this issue is completely uncalled for or has already been discussed. I don't currently have the time to do more research, but I have had this experience a couple of times already, and I didn't want to just ignore it again.

@dpc
Copy link
Collaborator

dpc commented Aug 11, 2022

I apologize in case this issue is completely uncalled for or has already been discussed. I don't currently have the time to do more research, but I have had this experience a couple of times already, and I didn't want to just ignore it again.

No worries. I know it's a pain.

OpenSSL is a PITA. I wish we could not have to use it, but AFAIK libgit2 can't do rust-tls https://github.com/rust-lang/git2-rs/blob/3e0a6134634f7b5129b6dfeeb0db97ccc9510866/libgit2-sys/Cargo.toml#L33

Provide the Windows binaries as zip files

Noted. I think this shouldn't be too hard to fix.

7z a -ttar "${NAME}.tar" "${NAME}"

Maybe you want to give it a try?

@LeCyberDucky
Copy link
Author

LeCyberDucky commented Aug 11, 2022

I'm quite busy at the moment, but in about one week from now I should get some more free time. I'll be happy to give it a shot then!

Edit: Oh, and thank you for getting back to me so quickly!

@LeCyberDucky
Copy link
Author

LeCyberDucky commented Aug 24, 2022

Well, I downloaded 7zip in order to experiment with this. Following this description of the command line interface, I suppose the command should just be

7z a -tzip "${NAME}.zip" "${NAME}"

I think there's something wrong with my 7zip setup, though, because I get an error about an "Unsupported archive type" no matter which archive type I specify here. If I just do 7z a test.zip example.txt, I do get a zip file out, but only 7zip can unzip it again. Windows complains that the archive is broken ¯\_(ツ)_/¯. Therefore, I'm not sure how to test this further.

Edit:
Alright, I managed to fix my 7zip installation. With that, I was able to unpack the .tar.gz file.

I then used 7z a -tzip "${NAME}.zip" "${NAME}" with the unpacked file to test this command, and then I unzipped the resulting zip archive with the native Windows unzipping tool for a sanity check. Everything works as expected :)

Should I make a pull request to add this? If so, do you still want to keep the tar.gz version for Windows as well, or should it be replaced with just the ´.zip` version?

@Kixunil
Copy link

Kixunil commented Nov 17, 2022

How about adding this to Cargo.toml?

[features]
default = ["vendored"]
vendored = ["git2/vendored-libgit2", "git2/vendored-openssl"]

Then it should work out of the box and advanced users wishing to dynamically link can still turn them on.

Also if crev is doing only basic git operations maybe it wouldn't be hard to rewrite them in Rust. I've seen a project that can already clone git repos (very fast) so maybe build it on top of that. Or maybe even just run git executable - slower but may work well enough?

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

No branches or pull requests

3 participants