macOS support? about nitrocli HOT 39 CLOSED

Yeah test_safe works now on my Debian box.

from nitrocli.

Oh sweet! It works now:

Status:
  model:             Storage
  serial number:     0x00050000
  firmware version:  0.52
  user retry count:  3
  admin retry count: 3

  SD card ID:        0x2484a9b6
  firmware:          unlocked
  storage keys:      created
  volumes:
    unencrypted:     active
    encrypted:       inactive
    hidden:          inactive

from nitrocli.

That is really strange. @szszszsz Can you help us with this problem?

from nitrocli.

Thank you for the explanation @szszszsz! I agree that having a separate hidapi-nitrokey would be the ideal solution, but I’m not keen on maintaining something for a platform that I don’t have access to. So for the time being, I’d suggest that macOS users install libnitrokey and then patch nitrokey-sys to use the pre-installed library. Ideally, we find someone who is willing to maintain a Homebrew formula for nitrocli that would automate theses tasks.

But I still don’t understand why the unit tests failed for @philn. As far as I see, he compiled libnitrokey from source, so it should have used the patched libhidapi, right? (Edit: Right, your second comment explained that.) Also, why does it fail on Debian?

@philn Could you please try to install libnitrokey from source on macOS and then run this code:

#include <stdio.h>
#include <libnitrokey/NK_C_API.h>

int main(void)
{
    if (NK_login_auto() == 1) {
        printf("Device found\n");
    } else {
        printf("Device not found\n");
    }
    return 0;
}

from nitrocli.

Ah, you might have to set the DYLD_LIBRARY_PATH environment variable to the libnitrokey directory too (or install libnitrokey to the system library path).

from nitrocli.

I don’t have access to a Mac and @d-e-s-o has to decide whether macOS is a supported platform, but you should be able to run upcoming versions on a Mac. Please try to compile the current development version from source by running:

$ git clone https://github.com/d-e-s-o/nitrocli
$ cd nitrocli/nitrocli
$ cargo build --release
$ ./target/release/nitrocli status

If you don’t want to install the development version, you’ll have to wait a few days until the next version is released.

from nitrocli.

Yeah, MacOS is currently unsupported (also see issue #1) and that is unlikely to change unless somebody with a Mac steps up to take over this part. I don't have one.

The "device not found" problem is also what I encountered back in the day when trying it out (again, issue #1). You can try with a more recent development snapshot as @robinkrahl
suggested. Since the "backend" library has changed there is a chance that this problem may have been fixed. If you try it out, please let us know your findings @philn.

from nitrocli.

Thanks for the answers! Unfortunately libnitrokey doesn't seem to build with Apple's clang:

$ cargo run --release -- status
...
   Compiling nitrokey-sys v3.4.1 (/Users/phil/dev/rust/nitrocli/nitrokey-sys)                                                                                                                                                              
error: failed to run custom build command for `nitrokey-sys v3.4.1 (/Users/phil/dev/rust/nitrocli/nitrokey-sys)`                                                                                                                           
process didn't exit successfully: `/Users/phil/dev/rust/nitrocli/nitrocli/target/release/build/nitrokey-sys-2fea02ddcc4e2e43/build-script-build` (exit code: 101)
--- stdout
TARGET = Some("x86_64-apple-darwin")
OPT_LEVEL = Some("3")
HOST = Some("x86_64-apple-darwin")
CXX_x86_64-apple-darwin = None
CXX_x86_64_apple_darwin = None
HOST_CXX = None
CXX = None
CXXFLAGS_x86_64-apple-darwin = None
CXXFLAGS_x86_64_apple_darwin = None
HOST_CXXFLAGS = None
CXXFLAGS = None
DEBUG = Some("false")
running: "c++" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "libnitrokey-v3.4.1/libnitrokey" "-Wall" "-Wextra" "-o" "/Users/phil/dev/rust/nitrocli/nitrocli/target/release/build/nitrokey-sys-2b1e66ef680a387a/out/libnitrokey-v3.4.1/DeviceCommunicationExceptions.o" "-c" "libnitrokey-v3.4.1/DeviceCommunicationExceptions.cpp"
cargo:warning=In file included from libnitrokey-v3.4.1/DeviceCommunicationExceptions.cpp:22:
cargo:warning=libnitrokey-v3.4.1/libnitrokey/DeviceCommunicationExceptions.h:32:7: error: exception specification of overriding function is more lax than base version
cargo:warning=class DeviceCommunicationException: public std::runtime_error
cargo:warning=      ^
cargo:warning=libnitrokey-v3.4.1/libnitrokey/DeviceCommunicationExceptions.h:32:7: note: while declaring the implicit destructor for 'DeviceCommunicationException'
cargo:warning=/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/stdexcept:106:13: note: overridden virtual function is here
cargo:warning=    virtual ~runtime_error() _NOEXCEPT;
cargo:warning=            ^
cargo:warning=libnitrokey-v3.4.1/DeviceCommunicationExceptions.cpp:24:55: error: expected ';' after top level declarator
cargo:warning=std::atomic_int DeviceCommunicationException::occurred {0};
cargo:warning=                                                      ^
cargo:warning=                                                      ;
cargo:warning=2 errors generated.
exit code: 1

from nitrocli.

Which clang version do you use? The libnitrokey builds on Xcode 8.2 (clang-800.0.42.1) and Xcode 9.1 (clang-900.0.38). This seems to be an issue with newer clang versions. Anyway, you should file an issue against the upstream project. – As the nitrokey-sys crate manually compiles libnitrokey (without using libnitrokey’s build scripts), could you please check whether you can compile libnitrokey from source?

from nitrocli.

libnitrokey builds fine, CMake detected AppleClang 10.0.0.10001145. I'm a bit confused now :)

from nitrocli.

Ah, nitrokey-sys doesn't build with C++14 enabled, I started a patch.

from nitrocli.

Great, thanks for investigating that! In the mean time, you should be able to fix the problem by manually installing libnitrokey and replacing the build.rs file in the nitrokey-sys directory with

fn main() {
    println!("cargo:rustc-link-lib=nitrokey");
}

to use the installed library. (Future versions of nitrokey-sys will provide a mechanism to use the system library.)

from nitrocli.

From #42 :

This only fixes the build, my device is still not detected.

Could you please try running the libnitrokey unit tests as described here?

from nitrocli.

@philn I integrated your patches into the upstream next branch. Until July, libnitrokey only used version numbers with two components (i. e. v3.4) so I decided to use the same version as libnitrokey and to use the last component for the crate version. Unfortunately, we now have libnitrokey v3.4.1 so I have no elegant solution to provide a new crate version. I’ll think about possible solutions.

from nitrocli.

I'll check the unit tests tomorrow.
About https://git.ireas.org/nitrokey-sys-rs/commit/?h=next&id=a47058981611bacae464ed6deac39c6686357f3a I'm not sure it's correct to use cfg!(target_os =...) in build scripts. It might only be problematic when cross-compiling though. I'll double-check tomorrow!

from nitrocli.

I'll check the unit tests tomorrow.

I reduced test1.cc to only check the device connection (I didn't want to change my device passwords) and even that fails:

./test1             

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test1 is a Catch v2.3.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
Slot names are correct
-------------------------------------------------------------------------------
/Users/phil/dev/libnitrokey/unittest/test1.cc:46
...............................................................................

/Users/phil/dev/libnitrokey/unittest/test1.cc:49: FAILED:
  REQUIRE( connected == true )
with expansion:
  false == true

===============================================================================
test cases: 1 | 1 failed
assertions: 1 | 1 failed

I also tested nitrocli on my Debian Testing box, without success either (device not detected).

About https://git.ireas.org/nitrokey-sys-rs/commit/?h=next&id=a47058981611bacae464ed6deac39c6686357f3a I'm not sure it's correct to use cfg!(target_os =...) in build scripts. It might only be problematic when cross-compiling though. I'll double-check tomorrow!

I was wrong about this :)

from nitrocli.

Okay, then it is an upstream issue. Would you mind filing an issue against the libnitrokey project?

from nitrocli.

nitrokey-app supports MacOS, right? Does it not use libnitrokey on it, too? I am a bit puzzled by this finding. Or is nitrokey-app still rolling its own logic everywhere (I believe it was doing so in the past, but that was quite some time ago; my details are a bit fuzzy on this front)?

from nitrocli.

nitrokey-app uses libnitrokey (AFAIK since version 1.0) and macOS is listed as a supported platform in the README. But I don’t know if it is tested regularly (except the automated unit tests).

from nitrocli.

The nitrokey-app works fine for me on macOS and Debian. Sorry I should have clarified that earlier!

from nitrocli.

Hi!
Regarding macOS, direct cause lies in linking against libhidapi from brew. For Nitrokey App and libnitrokey, we use fixed fork under Windows and macOS (CMake QMake). If you would find it handy, I can release it as a separate hidapi library (say hidapi-nitrokey; by moving proper make instructions there), installable to system.
However, in case nitrocli would use already installed, system libnitrokey (which is built by default with the custom hidapi), the problem will probably go away, though asking user for installing it separately would probably not result in the best UX. Similar connection issue might occur under Windows.
Last time I have checked (and confirmed just now again), the original hidapi project is not maintained anymore, hence the need of fork.
Changes in the hidapi-nitrokey (against signal11's master, mac directory): hidapi-nitrokey-diff.txt

from nitrocli.

Regarding C++ tests, these are not ready to use for the end-user yet, but rather used internally during the development, and are not device agnostic. test1.cc connects only to Nitrokey Pro, and test2.cc tests only Nitrokey Storage. I will:

• add simple C++ connection tests for such cases like this one, and
• describe the C++ tests state properly in the readme.

About release testing, each libnitrokey release is tested on supported platforms (Windows, macOS, Ubuntu/Fedora) and against current firmwares, using general Python tests with pytest, in /unittest directory:

• Pro: pytest -v test_pro.py
• Storage: pytest -v test_pro.py && pytest -v test_storage.py

Note: these require standard PINs set, and checks all features, including user data removal. Except that, they are safe to run on the end-user device, e.g. in case one would suspect hardware malfunction.

Additionally, currently CI builds each pushed commit on macOS and Ubuntu: https://travis-ci.org/Nitrokey/libnitrokey

from nitrocli.

@philn I have added a simple connection test as well to the libnitrokey's master branch: test_safe. It contains only status check and will not change the data on the device any way. Will work on any device.
@robinkrahl You are welcome! Feel free to call me for support. I have macOS and could run tests periodically in case you would decide to support it. I will keep in mind releasing the hidapi-nitrokey then - let me know, if you would need it earlier.

I am not experienced in Homebrew unfortunately, though we do have Nitrokey App buildable via brew cask already: Nitrokey/nitrokey-app#291.

from nitrocli.

test_safe results on my Debian box (Storage key is plugged in):

./test_safe 

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_safe is a Catch v1.12.1 host application.
Run with -? for options

-------------------------------------------------------------------------------
C API connect
-------------------------------------------------------------------------------
/home/phil/dev/libnitrokey/unittest/test_safe.cpp:31
...............................................................................

/home/phil/dev/libnitrokey/unittest/test_safe.cpp:37: FAILED:
  REQUIRE( login != 0 )
with expansion:
  0 != 0
with messages:
  This test set assumes either Pro or Storage device is connected.
  Here should be implemented only tests not changing the device's state, and
  safe to user data.

-------------------------------------------------------------------------------
Check retry count
-------------------------------------------------------------------------------
/home/phil/dev/libnitrokey/unittest/test_safe.cpp:41
...............................................................................

/home/phil/dev/libnitrokey/unittest/test_safe.cpp:43: FAILED:
  REQUIRE( login != 0 )
with expansion:
  0 != 0
with message:
  This test assumes your PINs' attempt counters are set to 3

-------------------------------------------------------------------------------
Status command for Pro or Storage
-------------------------------------------------------------------------------
/home/phil/dev/libnitrokey/unittest/test_safe.cpp:56
...............................................................................

/home/phil/dev/libnitrokey/unittest/test_safe.cpp:57: FAILED:
  REQUIRE( login != 0 )
with expansion:
  0 != 0

-------------------------------------------------------------------------------
Device serial
-------------------------------------------------------------------------------
/home/phil/dev/libnitrokey/unittest/test_safe.cpp:73
...............................................................................

/home/phil/dev/libnitrokey/unittest/test_safe.cpp:74: FAILED:
  REQUIRE( login != 0 )
with expansion:
  0 != 0

-------------------------------------------------------------------------------
Firmware version
-------------------------------------------------------------------------------
/home/phil/dev/libnitrokey/unittest/test_safe.cpp:82
...............................................................................

/home/phil/dev/libnitrokey/unittest/test_safe.cpp:83: FAILED:
  REQUIRE( login != 0 )
with expansion:
  0 != 0

===============================================================================
test cases: 6 | 1 passed | 5 failed
assertions: 7 | 2 passed | 5 failed

from nitrocli.

Is it normal to link against the system-wide libhidapi-libusb?

ldd test_safe
	linux-vdso.so.1 (0x00007ffe1679d000)
	libnitrokey.so.3 => /home/phil/dev/libnitrokey/build/libnitrokey.so.3 (0x00007fa2da6a4000)
	libhidapi-libusb.so.0 => /usr/lib/x86_64-linux-gnu/libhidapi-libusb.so.0 (0x00007fa2da457000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fa2da2d4000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa2da151000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fa2da137000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa2d9f76000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fa2d9f6a000)
	libusb-1.0.so.0 => /lib/x86_64-linux-gnu/libusb-1.0.so.0 (0x00007fa2d9d51000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa2d9d30000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa2da874000)
	libudev.so.1 => /lib/x86_64-linux-gnu/libudev.so.1 (0x00007fa2d9d0a000)

from nitrocli.

from nitrocli.

Ah! I'm running the test uninstalled directly from the build directory. I wasn't aware of the udev rules. I'll check this again tomorrow, do I really need to install the library to run the tests? It's a bit unusual :)

from nitrocli.

Here is the culprit then :-) Hardware access for the user is regulated via the Udev service, and installation places the Udev rules file in your system. You can copy it manually, if you would like to, and it would work just as good.

from nitrocli.

@philn Did you mange to get test_safe to work under macOS?

from nitrocli.

I haven't tried yet. Tonight maybe, sorry for the delay :)

from nitrocli.

No worries! I just wanted to make sure you aren’t waiting for feedback from us.

from nitrocli.

Also works on macOS! So what's left for this then? Seems like it all boils down to a bundling problem for nitrocli to use the right libnitrokey library.

from nitrocli.

Did you also test nitrocli? If not, change the nitrokey-rs/build.rs file to the following before compiling nitrocli:

fn main() {
    println!("cargo:rustc-link-lib=nitrokey");
    println!("cargo:rustc-link-search=native=/path/to/libnitrokey");
}

If nitrocli works, we have two options:

• Wait for the hidapi-nitrokey library and add it as a dependency to upstream nitrokey-sys.
• Create a homebrew package that applies the patch when compiling nitrocli.

from nitrocli.

Running `target/release/nitrocli status`
dyld: Library not loaded: @rpath/libnitrokey.3.dylib
  Referenced from: /Users/phil/dev/rust/nitrocli/nitrocli/target/release/nitrocli
  Reason: image not found

I need to fiddle with install_name_tool it seems.

from nitrocli.

Awesome that we got that working!

@philn , all, can we close this issue? It appears all problems have been eliminated and nitrocli is working on Mac now. It also seems as if nothing needs to be fixed on the nitrocli side itself.

from nitrocli.

Right, the problem is related with how to use the hidapi nitrokey fork from the Rust nitrokey bindings.

from nitrocli.

@philn In nitrokey-sys v3.4.3, you can skip the build by setting the environment variable USE_SYSTEM_LIBNITROKEY when compiling. So you don’t have to patch the source anymore with upcoming nitrocli versions.

@d-e-s-o Could you please update the bundled nitrokey-sys before the next release (but not the version requirement in Cargo.toml – we only need 3.4.* – Edit: nitrokey-sys obviously is a nitrokey dependency)? And should we mention this workaround for macOS in the readme?

from nitrocli.

@d-e-s-o Could you please update the bundled nitrokey-sys before the next release (but not the version requirement in Cargo.toml – we only need 3.4.* – Edit: nitrokey-sys obviously is a nitrokey dependency)?

Done.

And should we mention this workaround for macOS in the readme?

We probably should :-)

from nitrocli.

Thanks! I’ll prepare a patch for the readme.

from nitrocli.