instructure / paseto

A paseto implementation in rust.
MIT License
151 stars 14 forks source link

SIGSEGV: invalid memory reference #4

Closed kennethjeremyau closed 5 years ago

kennethjeremyau commented 5 years ago

Description

Steps to reproduce:

git clone git@github.com:instructure/paseto.git
cargo test

Error message:

running 15 tests
test pae::unit_tests::test_le64 ... ok
test pae::unit_tests::test_pae ... ok
error: process didn't exit successfully: `/Users/kau/Development/paseto/target/debug/deps/paseto-597deab10fb2334f` (signal: 11, SIGSEGV: invalid memory reference)

Hello! I just happened to do a rustup update right after 1.32 was stabilized and found my app was segfaulting. I traced it here and found the tests were doing the same thing. Hopefully you are able to reproduce. Thank you.

Additional Information

Mythra commented 5 years ago

Hey @kennethjeremyau ,

Thanks for the report! I'm currently running this on linux:

running 15 tests
test pae::unit_tests::test_le64 ... ok
test pae::unit_tests::test_pae ... ok
test v1::get_nonce::unit_tests::test_nonce_derivation ... ok
test tokens::builder::unit_test::can_construct_a_token ... ok
test tokens::unit_tests::invalid_enc_token_doesnt_validate ... ok
test v2::local::unit_tests::full_round_paseto ... ok
test v2::local::unit_tests::paseto_non_empty_msg_encrypt_verify ... ok
test v2::local::unit_tests::paseto_empty_encrypt_verify ... ok
test tokens::unit_tests::valid_enc_token_passes_test ... ok
test tokens::unit_tests::invalid_pub_token_doesnt_validate ... ok
test v2::local::unit_tests::paseto_non_empty_footer_encrypt_verify ... ok
test tokens::unit_tests::valid_pub_token_passes_test ... ok
test v1::local::unit_tests::test_v1_local ... ok
test v2::public::unit_tests::paseto_public_verify ... ok
test v1::public::unit_tests::test_v1_public ... ok

test result: ok. 15 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests paseto

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ rustc --version
rustc 1.32.0 (9fda7c223 2019-01-16)
$ git rev-parse HEAD
cfc5c94ec6b0d901e2970f370b56a6a2f43d3b76

And it seems to be running fine. So it could be mac specific, but we've also had a lot weird issues after jemalloc was disabled if you're still running an older commit. Can I get you to verify the git hash (it should match what I have here)?

kennethjeremyau commented 5 years ago

Yes that's the same git hash. cfc5c94ec6b0d901e2970f370b56a6a2f43d3b76 I'll try building on a linux docker container and maybe dig a bit deeper.

Mythra commented 5 years ago

Hmm haven't found any smoking gun yet. Before i dig in tomorrow would it be possible to get:

  1. The C compiler that you're using and it's version (usually found by doing env | grep CC, and if that prints nothing than the version of xcode you have installed).
  2. Any env vars that start with SODIUM that might cause linking with a libsodium that can cause the segfault.
kennethjeremyau commented 5 years ago

env | grep CC shows nothing. env | grep SODIUM shows nothing. Xcode Version 10.1 (10B61)

$ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/c++/4.2.1
Apple LLVM version 10.0.0 (clang-1000.11.45.5)
Target: x86_64-apple-darwin18.2.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

I just kicked off a build targeting x86_64-unknown-linux-musl on ubuntu, so we'll see what that shows. (I am setting SODIUM_STATIC and SODIUM_LIB_DIR there).

While I was investigating, I was disabling unit tests and narrowed down one of the failure paths to underlying_local_paseto in v2. But I ran out of time. Will look more tomorrow as well.

kennethjeremyau commented 5 years ago

Of course, my build failed due to the deprecated env variables. But I re-ran the tests in the docker container and they worked fine.

I traced the problem to: let key_obj = Key::from_slice(key);

This looks like a macOS related issue in sodiumoxide similar to: https://github.com/sodiumoxide/sodiumoxide/issues/302

Mythra commented 5 years ago

Yeah,

This looks to be a bug in v2 local which uses libsodium underneath the hood, and more specificially with the sodiumoxide crate. I see the last comment says:

@sribe-pearson #304 has been merged now, you can try the master branch to see if your problem is solved now. I'm pretty sure it will fix your problem as I encountered the very same issue as you and it worked for me.

Do you wanna try paseto with the master version of sodiumoxide? Should be as simple as changing:

sodiumoxide = { version = "^0.2", optional = true }

to:

sodiumoxide = { git = "https://github.com/sodiumoxide/sodiumoxide.git", optional = true } 

in Cargo.toml? If so that means we should just need to wait for a sodiumoxide release to come out.

kennethjeremyau commented 5 years ago

Yes, that worked. Thanks for your help.

Mythra commented 5 years ago

v1.0.3 has been released which bumps sodiumoxide up to a point where it no longer segfaults on mac-os-x. Sorry this took so long to get the fix merged in :disappointed:

Thanks for reporting this again! We'll look into seeing how we can work better with sodiumoxide so this issue like this isn't a problem for over a month.

kennethjeremyau commented 5 years ago

Thanks, this fix saves me a bunch of compilation time.