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

next_steps_nitrokey.md: extracted steps for Nitrokey integration #61

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/next_steps_nitrokey.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Propose and document new API design based on CTAPHID

With CTAPHID used instead of CoAP, human-readable names of endpoints should be
changed to numeric IDs. CoAP status reporting also has to be modified to account
for different interface.

# Implement functions specific to Fobnail in libnitrokey

This is temporary solution to allow testing until full Fobnail support is added
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krystian-hebel I'm not sure if I fully get it.
How is Implement functions specific to Fobnail in libnitrokey different from adding Fobnail support to libnitrokey ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libnitrokey is used for older Nitrokey models. For latest Nitrokey 3--which we are aiming here--we use pynitrokey instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably @krystian-hebel was looking for a C library to have something without Python.

Is pynitrokey pure Python? If we are considering heads, we may have quite limited flash space to include Python interpeter and some Python modules, depending on the specific device.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Python is not suitable in this case, I suggest to write a new core library in Rust. This is something we were considering anyways but didn't had a need to yet.

Copy link
Contributor Author

@macpijan macpijan Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads has been looking at including Python: linuxboot/heads#689
But it is not there yet, and may be difficult to fit in some platforms/configs AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are confident that Rust will result in total smaller footprint than Python, then it might be a way to go.
I saw you have made some Rust work in heads already linuxboot/heads#1354 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for Python, Heads seems to also be headed in that direction. While I'm just a little worried about space issue for the interpreter itself, its modules tend to be bigger that C counterparts. I also prefer to get errors during compilation instead of runtime, especially for something that requires reflashing, but I can live with that.

Heads can increase amount of space available by using maximized builds, however this may prevent us from using DRTM later, because TXT probably requires ME to be functional.

I tried to build current CPython locally, and after disabling all optional features listed by ./configure --help and enabling optimizations I ended up with over 130 MiB to be installed, which after default XZ compression got down to 23MiB, which is still way too much. Note that this was with default gcc/libc on my host instead of musl used by Heads, but I don't expect it to miraculously get 10x smaller.

How is Implement functions specific to Fobnail in libnitrokey different from adding Fobnail support to libnitrokey?

Sorry, pushed wrong version, it was described better but I forgot to save the file. Based on the above discussion I'll try to rewrite this part in a way that doesn't makes any choice definitive.

to libnitrokey.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing aspect is always quite important in these applications. We could at least mention that the library will be covered with unit test to ensure the proper implementation of the documented API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.


# Rewrite Attester application to be compatible with Fobnail Token changes

This task consists of supporting new communication interface. All uses of CoAP
will be removed, and API provided by libnitrokey will be used instead.

# Change the way metadata is obtained

Currently, metadata is created from SMBIOS tables and MAC address of primary
network controller, as described [here](https://fobnail.3mdeb.com/fobnail-api/#adminprovisionidmeta).
Accessing SMBIOS requires root privileges, and reliably determining proper
network interface proved to be difficult, especially with USB network adapters
or systems with no network drivers like Heads. A new way of obtaining metadata
macpijan marked this conversation as resolved.
Show resolved Hide resolved
is required, preferably one that can easily be ported between various operating
systems.

# Port Attester application to Windows

Apart from metadata mentioned earlier, accesses to TPM and HID interface also
are done in a way specific to operating system. On top of that, building
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
are done in a way specific to operating system. On top of that, building
are done in a way specific to the operating system. On top of that, building

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

applications for Windows requires different different tools and procedures,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
applications for Windows requires different different tools and procedures,
applications for Windows requires different tools and procedures,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

which also will be done in this task.

## Update documentation

Top level description wasn't changed since first PoC. Project evolved
significantly since then, and information located there no longer reflects
current state of the project.