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

Rust binaries support for Nitrokey 3 cards / Heads support status for Nitrokey 3 dongles #1354

Closed
szszszsz opened this issue Mar 25, 2023 · 11 comments

Comments

@szszszsz
Copy link
Contributor

szszszsz commented Mar 25, 2023

Is your feature request related to a problem? Please describe.

I would like to use Rust-based tooling inside Heads. Was this done before? If so, what are best practices for that?

I expect any binary which would be small and buildable statically using musl would be good enough.
Just to confirm: is the musl use a hard requirement for the non-C tools? I would like to keep the static linking against glibc possibility open if possible for the tool I have in mind.

Describe the solution you'd like

  • A document with a setup description and best practices for adding Rust-based tools. (if there is any already)
  • Pointers to the already added and used Rust-based tools in Heads. (if there is any already)

Edit: I can see from this table, that musl should actually have lower size overhead and be a safer counterpart against glibc:

@tlaurion
Copy link
Collaborator

@szszszsz this would be a first.
Ideally, musl would be reused and binaries produced, depending on libc provided with heads for bloat reduction and all things dynamically linked.

Check for no_cc under Makefile to get hands on on what is passed to the build system today. musl-cross-make might need a version bump

@tlaurion
Copy link
Collaborator

@szszszsz I see rust-lang/rust#31322

@szszszsz
Copy link
Contributor Author

szszszsz commented Mar 25, 2023

Yes, linking against musl should be as easy as providing it as target x86_64-unknown- linux-musl. I already found magic invocation for that, however not all debug information seems to be removed for this triple (as opposed to libc variant), but that seems to be fixable.

This is what I have so far, while building rusb example for listing USB devices (not tested under Heads environment yet):

click to show
~/w/h/rusb (master|✚2) $ env RUST_BACKTRACE=1 TARGET=linux PKG_CONFIG=(which pkg-config) cargo +nightly bloat  -Z build-std=std,core,panic_abort -Z build-std-features=panic_immediate_abort --target=x86_64-unknown-
linux-musl --release  --example list_devices --quiet
Warning: unused arguments left: ["--quiet"].
warning: use of deprecated type alias `libc::time_t`: This type is changed to 64-bit in musl 1.2.0, we'll follow that change in the future release. See #1848 for more info.
  --> src/context.rs:18:24
   |
18 | type Seconds = ::libc::time_t;
   |                        ^^^^^^
   |
   = note: `#[warn(deprecated)]` on by default

warning: `rusb` (lib) generated 1 warning
    Finished release [optimized] target(s) in 0.03s
    Analyzing /tmp/cargo/x86_64-unknown-linux-musl/release/examples/list_devices

File  .text    Size     Crate Name
0.7%  19.7% 19.6KiB [Unknown] _ZN9libunwind10CFI_ParserINS_17LocalAddressSpaceEE20parseFDEInstructionsERS1_RKNS2_8FDE_InfoERKNS2_8CIE_In...
0.4%   9.8%  9.7KiB [Unknown] main
0.2%   4.3%  4.3KiB [Unknown] _ZN9libunwind17DwarfInstructionsINS_17LocalAddressSpaceENS_16Registers_x86_64EE13stepWithDwarfERS1_mmRS2_Rb
0.1%   4.0%  3.9KiB [Unknown] _ZN9libunwind17DwarfInstructionsINS_17LocalAddressSpaceENS_16Registers_x86_64EE18evaluateExpressionEmRS1_R...
0.1%   2.9%  2.9KiB [Unknown] _ZN9libunwind10CFI_ParserINS_17LocalAddressSpaceEE9decodeFDEERS1_mPNS2_8FDE_InfoEPNS2_8CIE_InfoEb
0.1%   2.9%  2.9KiB [Unknown] fmt_fp
0.1%   2.9%  2.8KiB [Unknown] _ZN9libunwind10CFI_ParserINS_17LocalAddressSpaceEE8parseCIEERS1_mPNS2_8CIE_InfoE
0.1%   2.8%  2.8KiB [Unknown] _ZN9libunwind14EHHeaderParserINS_17LocalAddressSpaceEE7findFDEERS1_mmjPNS_10CFI_ParserIS1_E8FDE_InfoEPNS5_...
0.1%   2.6%  2.6KiB [Unknown] __unw_add_dynamic_fde
0.1%   2.5%  2.5KiB [Unknown] _ZN9libunwind10CFI_ParserINS_17LocalAddressSpaceEE7findFDEERS1_mmmmPNS2_8FDE_InfoEPNS2_8CIE_InfoE
0.1%   2.4%  2.3KiB [Unknown] printf_core
0.1%   1.8%  1.8KiB [Unknown] _ZN9libunwind14EHHeaderParserINS_17LocalAddressSpaceEE11decodeEHHdrERS1_mmRNS2_12EHHeaderInfoE
0.1%   1.5%  1.5KiB [Unknown] malloc
0.0%   1.2%  1.2KiB [Unknown] __bin_chunk
0.0%   1.0%    988B [Unknown] _ZN9libunwind17LocalAddressSpace11getEncodedPERmmhm
0.0%   0.9%    873B [Unknown] unwind_phase2
0.0%   0.8%    797B [Unknown] unwind_phase2_forced
0.0%   0.8%    772B [Unknown] _Unwind_RaiseException
0.0%   0.7%    753B      core <core::fmt::builders::PadAdapter as core::fmt::Write>::write_str
0.0%   0.7%    750B [Unknown] __unw_set_reg
1.2%  32.7% 32.4KiB           And 302 smaller methods. Use -n N to show more.
3.7% 100.0% 99.2KiB           .text section size, the file size is 2.6MiB
~/w/h/rusb (master|✚2) $

Edit: Thank you for the pointers!

@tlaurion
Copy link
Collaborator

@szszszsz I guess this is for nitrokey 3 cards?
I would suggest to rename this issue, it was asked under heads slack/matrix channel at https://matrix.to/#/!pAlHOfxQNPXOgFGTmo:matrix.org/$7w1jE1EI3GVk-p-8WNBwAeYVTs2J2_T0H3Zgvg1X9wA?via=matrix.org&via=nitro.chat&via=talk.puri.sm

@tlaurion tlaurion changed the title Rust binaries support Rust binaries support for Nitrokey 3 cards Mar 29, 2023
@saper
Copy link
Contributor

saper commented Apr 1, 2023

If we talk about Nitrokey 3 support instead, the question would be what do those Rust binaries do?

I have a Nitrokey 3 sitting here quite idle, since it can do almost ... nothing, maybe I could test/work on something. Even webauthn magically stopped working with the token.

Rant

I can't even build Nitrokey 3 tooling from source because we seem to rely on some interesting binary components only. This puts me off from ever working on anything with that device.

@tlaurion
Copy link
Collaborator

tlaurion commented Jun 7, 2023

@szszszsz Example of questions from community directed at me:

I recently purchased Nitrokey 3 NFC and tried to use it with my X230, but when I tried to change HOTP, the system wrote that the Librem Key device was not found, it did not seem to recognize the N3K. Problem in NK3 or in Heads?

AFAIK HOTP is not functional on Nitrokey 3. Please send pointers to this issue so the community can find their own answers without contacting me through private channels. Thank you!

Answer I can give for the moment:

  • firmware needs to support reverse HOTP. Not sure if this is the case and which firmware version+ it needs to be.
  • hotp-verification module will need to be updated to support newer keys and their IDs. As of today, the module for sure doesn't support newer keys.

@tlaurion tlaurion changed the title Rust binaries support for Nitrokey 3 cards Rust binaries support for Nitrokey 3 cards / Heads support status for Nitrokey 3 dongles Jun 7, 2023
@szszszsz
Copy link
Contributor Author

szszszsz commented Jun 7, 2023

Hi @tlaurion !

Sorry for not replying, this ticket got out of my sight.

  1. Please connect to this ticket for synchronization:
  2. To my knowledge new Heads release is required for x230 to work with it, as it uses new tool version with a new transport protocol. New models are released with it and tested, and if I am not mistaken, this will be backported.
  3. In the meantime it was decided against Rust to avoid introducing potential build complexity and its reproducibility issues, and instead the old C tool was extended. It's 50kB lighter as well than the Rust build I've managed to make.

@tlaurion
Copy link
Collaborator

tlaurion commented Jun 7, 2023

@szszszsz a PR pointing modules/hotp-verification to tested commit would be needed under Heads!
Ideally the PR would explain what are the changes and simply point to the newer commit.

We talked about upstreaming and reducing Heads forks complexity in a recent call with Nitrokey!
If Nitrokey has new hardware, and users are buying new dongles, then Heads on master should reflect that new offer.
I understand forks for rebranding, but other features should eventually land on master as well and even rebranding should be reflected in board configurations, not in forks.

@szszszsz
Copy link
Contributor Author

szszszsz commented Jun 7, 2023

@tlaurion I understand and agree. As far as I know this work should be upstreamed soon (certainly the updated modules/hotp-verification module), and it was just finalized this (last?) week, so that's a fresh thing.

cc @daringer : Please keep this one in mind.

@szszszsz
Copy link
Contributor Author

Connected: #1417

@szszszsz
Copy link
Contributor Author

I think with that we can close this ticket, as I do not see anything to act on. Please reopen otherwise.

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