Extend FIDO2 BLE support also for Linux

[akemnade]

For Windows it was already added via gh#336,
so let's also add it for Linux.
Unpaired devices are ignored, the user has to pair independently of libfido use using the bluetooth manager provided by the desktop environment.

[LDVG]

Thank you for your contribution! Before having a more in-depth look, may I ask what authenticator you are testing this against?

[akemnade]

This was tested with:

eSecu FIDO2 pro (passkey printed on device, 20 byte control point size, fragmentation often needed, bluetooth turned off after a short time
of inactivity)
AirID2 Mini (full Numerical comparison for pairing, >200 byte control point size, so fragmentation rarely needed),

[akemnade]

about the ci-failures:
cifuzz: it does not install dependencies correctly, and they are not specified in this repo, so I cannot do anything there easily.
The other fuzzer:

==16136==WARNING: MemorySanitizer: use-of-uninitialized-value
5096
    #0 0x7ff5d26bb286  (/lib/x86_64-linux-gnu/libsystemd.so.0+0x4b286) (BuildId: e45f7492c0f62251620378d7224ad0371a8d1f98)
5097
    #1 0x7ff5d26b2ae7 in sd_bus_start (/lib/x86_64-linux-gnu/libsystemd.so.0+0x42ae7) (BuildId: e45f7492c0f62251620378d7224ad0371a8d1f98)
5098
    #2 0x7ff5d26b2e88 in sd_bus_open_system_with_description (/lib/x86_64-linux-gnu/libsystemd.so.0+0x42e88) (BuildId: e45f7492c0f62251620378d7224ad0371a8d1f98)
5099
    #3 0x7ff5d26ba157 in sd_bus_default_system (/lib/x86_64-linux-gnu/libsystemd.so.0+0x4a157) (BuildId: e45f7492c0f62251620378d7224ad0371a8d1f98)

How to debug this? sd_bus_default_system just takes a pointer to a variable on the stack to write some output into this, so nothing scary. Can this be resolved to source code lines?

[LDVG]

How to debug this? sd_bus_default_system just takes a pointer to a variable on the stack to write some output into this, so nothing scary. Can this be resolved to source code lines?

MemorySanitizer requires the whole application to be instrumented -- libsystemd included.

We'd likely need to mock a lot of these calls to sensibly fuzz your implementation.

[LDVG]

This is starting to take shape!

I have left a set of comments which mostly aim to get this PR into a shape that is consistent with our existing code base. Note that some of the comments apply to more than one occurrence. I'd like to generally mention:

• We try to follow a basic KNF/style(9) format for our code. While not critical, I left a couple of nitpicking comments on this.
• Some of the functions have conditionals/loops that nest very deeply, we'd prefer if we could split some of the logic out to separate functions to alleviate this.
• We'd like for most return values to be checked and bubbled up to the calling function. Where it makes sense, we also like to log a debug message indicating what went wrong.

Once again, I'd like to thank you for your work so far!

[akemnade]

CMakeFiles/fido2_shared.dir/ble_linux.c.o.d -o CMakeFiles/fido2_shared.dir/ble_linux.c.o -c /__w/libfido2/libfido2/src/ble_linux.c
327
In file included from /__w/libfido2/libfido2/src/ble_linux.c:2:
328
/usr/include/elogind/systemd/sd-bus.h:96:43: error: ISO C restricts enumerator values to range of 'int' [-Wpedantic]
329
   96 |         SD_BUS_CREDS_UNIQUE_NAME        = 1ULL << 31,
330
      |                                           ^~~~
331
make[2]: *** [src/CMakeFiles/fido2.dir/build.make:569: src/CMakeFiles/fido2.dir/ble_linux.c.o] Error 1

seems like a problem in alpine + libelogind in the ci run

[LDVG]

Thank you for the updates. I've added a set of new comments, and also left a set of (potentially?) unresolved comments from the previous round.

[akemnade]

opened an issue for the alpine trouble:
https://gitlab.alpinelinux.org/alpine/aports/-/issues/15102

By adding the include path, the alpine build got broken. Don't ask me questions...

[akemnade]

A summary of the alpine problem:
later versions of sd-bus.h in systemd/elogind use extension keyword to allow 64bit enums even in pedantic mode. So this is solved in alpine edge already and next release in November. Without extension these things are tolerated in system includes (not something added via -I). E.g. debian does not add -Isomething in pkg-config --cflags for libsystemd, so that gets ignored, alpine does, but has a copy of the header files in /usr/include/systemd besides of /usr/include/elogind, so things are found in any case.

So what can we do now:

• just accept the alpine problem until it gets fixed
• remove addition of include path for systemd in CMakeLists.txt
• remove -pedantic
• any hacks in the alpine build process (like removing /usr/include/elogind in the alpine container)

[jo-bitsch]

I'm very interested in that functionality and would be willing to help fixing the remaining issues.

The alpine issue referenced above should be fixed in the current stable (as of the new release of alpine v.3.19 released last December 7, 2023). As the pipeline refers to latest (

libfido2/.github/workflows/alpine_builds.yml

Line 20 in f03f4c4

container: alpine:latest

) this might just work.

Unfortunately, I can't inspect the fuzzing issues, as the log expired.

[DavidZemon]

I forked @akemnade's repo and rebased it on top of Yubico's main, then installed on my local Ubuntu 24.04 system. I then tested it with one of Token's devices (a Token BioStick - https://tokenring.com) and IT WORKED! Well done @akemnade.

The UX isn't where I'd like it just yet. Our device is very small and does not remain on and connected at all times. So what I need is for this BLE feature to do a quick scan of nearby BLE devices, check if any of those devices are advertising the FIDO service UUID (0xFFFD), and if so, attempt to connect if not already connected. I don't know if I'll be given permission to work on this as part of work hours, but will definitely keep you updated if I'm able to put any time toward it.

[akemnade]

I am still interested in having this merged. I am a bit puzzled about how to proceed with this fuzzy test issues. As I understand correctly, there are no actual problems seen with this code but fuzzing has to be set up correcly.

What I am wondering about is: To me the idea would be to basically bombard the code with random dbus input and look if something explodes. To achieve that I would think that functions like sd_bus_call_method() would have to be mocked. But then there are other functions like sd_bus_message_read_basic() which are just processing such input further. To me it feels a bit strange if such functions have to be mocked, too.

Regarding UX: I think a bluetooth gatt profile has to be registered, so auto-connection is enabled to paired devices having that feature. I have some experience in doing such stuff, so I would do that, if the basic stuff would get in.

BTW: there is some mac-wip branch providing bluetooth support for libfido2 on macOS.

[akemnade]

so, lets rebase it here too, to also see some recent ci results