OpenSSL linking configuration mismatch between oqs (C) and oqs-sys

[BlackHoleFox]

In the primary C library, linking to OpenSSL is disabled on Windows. However, in this crate, it is enabled by default on all platforms. Without --no-default-features, this causes a number of compilation errors on Windows when trying to build oqs in the build script.

Please consider not linking to OpenSSL by default, especially on Windows, although it causes installation headaches on all platforms.

[thomwiggers]

I need the equivalent of https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#platform-specific-dependencies for features but it does not seem to exist yet (rust-lang/cargo#7914 (comment))

[BlackHoleFox]

I need the equivalent of ...

To fix the OpenSSL issue on Windows, you can swap

liboqs-rust/oqs-sys/build.rs

Line 45 in 805ac8e

if cfg!(feature = "openssl") {

out for

if cfg!(all(not(windows), feature = "openssl")) {

Is there a reason why static linking is enabled by default? Is it needed for your use case of liboqs-rust? I am unable to get this crate to compile on Windows, even with the default features disabled (I have no idea how the Windows CI passes currently). I've also asked others to try and they've also encountered difficulties. Note that liboqs itself compiles and the binaries run fine in Visual Studio. I also tested the liboqs.sln project that liboqs-sys generates as part of the build process in Visual Studio and it compiles fine as well.

Every time I compile liboqs-rust, even with OpenSSL disabled, I'm met with a linker error:

Compiling oqs-sys v0.3.0
   Compiling oqs v0.3.0
error: could not find native static library `oqs`, perhaps an -L flag is missing?

error: aborting due to previous error

error: could not compile `oqs-sys`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

The only way the linking bug seems to fix itself is by changing this

liboqs-rust/oqs-sys/build.rs

Line 56 in 805ac8e

println!("cargo:rustc-link-lib=static=oqs");

to not statically link:

println!("cargo:rustc-link-lib=oqs");

Tl;dr I'm having a hard time seeing how something with the build script is not wrong currently. If the stuff above is acceptable and desired, I'll open a PR off my testing branch.

[thomwiggers]

if cfg!(all(not(windows), feature = "openssl")) {

I don't want to force-disable openssl on Windows, because if you do set up everything correctly, it works fine, and gives you access to accelerated versions of a bunch of primitives that make up between 20% to up to 90% of the runtime of some schemes.

I have no idea how the Windows CI passes currently

Presumably it's set up differently.

I'm not sure if println!("cargo:rustc-link-lib=static=oqs"); should indeed be static, I've mainly just copied that from some example somewhere initially and as I only run Linux, it's obviously not been a problem for me. I'd merge a PR that removes static= assuming it passes CI.

[thomwiggers]

I don't want to force-disable openssl on Windows

I should add that if you don't want to have openssl in your crate, you can simply specify no-default-features and use a build.rs that emits the openssl feature where it is supported.

[BlackHoleFox]

I dont want to force disable OpenSSL on Windows

I don’t think it works at all from my testing. This line in the primary C library explodes MSVC if you try to enable OpenSSL. And as stated originally, liboqs unconditionally disables compiling OpenSSL on Windows. None of its CI jobs even attempt to enable it either.

no-default-features

That would only be a solution if the static linking bug was fixed. But, due to the liboqs’ behavior with OpenSSL and Windows I think its not very important, at least until the C code is patched to support MSVC.

[thomwiggers]

This line in the primary C library explodes MSVC if you try to enable OpenSSL.

That file should not be compiled though... It's only for tests against NIST's Known-Answer Tests

[BlackHoleFox]

Well, it seems to be anyway. These are the errors that MSVC gives when you try to compile main on Windows with OpenSSL, sorry I didn't share these sooner:

 liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(36,15): error C2143: syntax error: missing ')' before '(' [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]
  liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(36,24): error C2059: syntax error: ')' [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]
  liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(36,25): error C2059: syntax error: ')' [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]
  liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(37,13): error C2143: syntax error: missing ')' before 'type' [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]
  liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(37,13): error C2091: function returns function [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]
  liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(37,32): error C2085: 'handleErrors': not in formal parameter list [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]
  liboqs-rust\oqs-sys\liboqs\src\common\rand\rand_nist.c(37,32): error C2143: syntax error: missing ';' before '{' [liboqs-rust\target\debug\build\oqs-sys-45a0b0771bd42382\out\build\src\common\common.vcxproj]

I tried changing the function to decorated like this and it looked to of fixed it.

#ifdef OQS_USE_OPENSSL
# ifdef __GNUC__
__attribute__((noreturn))
# elif defined(_MSC_VER)
__declspec(noreturn)

[thomwiggers]

I'm running into problems if I don't statically link liboqs :(

[thomwiggers]

Are you building liboqs directly? How are you building liboqs? You should be passing -DOQS_BUILD_ONLY_LIB=Yes if you want to match what this crate is doing.

[BlackHoleFox]

I’m not building it directly. I am attempting to compile it through this Rust crate. Every error I’ve posted has come from the output of oqs-sys‘ build script using cargo build.

[thomwiggers]

That's definitely weird...

[thomwiggers]

I've figured out why CI was not having problems: CI wasn't running tests on Windows at all 🙈 (#30)

[BlackHoleFox]

That would explain a bit. Can't believe I didn't see that sooner D:

[thomwiggers]

Yeah Github Action's workflow syntax is rather sensitive to these problems where you define matrix vars but don't actually use them

[thomwiggers]

The branch in #30 works on a Windows 10 Dev VM (from Microsoft) that's up-to-date; unfortunately I am having trouble updating the Github Actions VM to use a more recent Windows SDK.

[thomwiggers]

I've managed to figure out the problem in #30, so you could try that branch. Don't forget to make sure you update the submodule if you check out that branch.

[BlackHoleFox]

I checked out the branch and confirmed it fixes the linking issue. My tests also run after configuring the stack size.

[BlackHoleFox]

This is very unrelated, but it would be cool if we could get this liboqs-rust to work with the openssl-sys crate. To get this compiling, the build.rs logic has duplicated some of the searching and link logic of that crate. It also has a vendored feature to get around the headaches of needing OpenSSL present on your system.

Its probably not possible, at least not easily, since liboqs links to it, but it would be a great feature. I tried tinkering with it but the linker on both Linux and Windows isn't happy with the idea.

[thomwiggers]

The main problem is the openssl-sys crate build.rs does not expose the location where it found OpenSSL. Otherwise I could pass it on to liboqs — I did look into this.

[BlackHoleFox]

Ah, that sucks :/ Good to know though.