Fix touch responsiveness and USB HID issues

[pwnall]

This PR addresses two responsiveness issues that may cause the device to hang or drop connections:

1. Touch State Leak: Implemented Drop for Touch to correctly clear the STATE when a user presence request is interrupted (e.g., by a USB packet or timeout). Added safe extraction in State::touch to handle queued hardware button interrupts (bounces) without hitting unreachable!() and panicking.
2. USB HID Communication Fixes:
   • Updated MessageAssembler::parse_packet to allow any valid initialization packet (not just CTAPHID_INIT) to abort a hung transaction, in accordance with the CTAP 2.1 specification.
   • Fixed a bug where a new initialization packet arriving immediately after a transaction timeout was incorrectly dropped while returning the MsgTimeout error.

These fixes prevent the firmware from entering unrecoverable states where the LED fails to blink or the device stops responding to the host.

[coveralls]

Coverage is 97.262% — pwnall:fix-artemis-touch-usb into google:develop. No base build found for google:develop.

[ia0]

Thanks for the PR! I'll only comment on the Touch issue and let @kaczmarczyck address the second point.

[ia0]

We should not reach unreachable!(). I wonder if you had to add this because you unconditionally reset the STATE in Drop for Touch. In theory, state.button.stop() will start dropping future events and clear any existing pending events. If that's not the case it's a bug in Wasefire.

That said, the applet could be resilient against Wasefire bugs. I don't think it's a good idea because (at least for development) we want to fail as soon as something impossible happens. We could have a resilient cargo feature to avoid crashes as much as possible, which would be used for production keys (there could be security concerns though, it's a trade-off with availability). @kaczmarczyck what do you think?

[pwnall]

Thank you very much for explaining!

I removed this diff. I don't want to make development more difficult 😄

[kaczmarczyck]

We should double check how we deal with parallel connections, and that we don't miss anything else. We didn't test enough since the Wasefire refactoring.

Touches should never be used twice. CTAP2 library code should already ensure this, but the combination with CTAP1 can work in one direction: First requesting on CTAP1, then going into the CTAP2 UP loop. We should drop CTAP1 then. But doesn't need to be done here.

[kaczmarczyck]

And to answer the question, once it is impossible, we can fail when the impossible happens.

[ia0]

LGTM for src/touch.rs. Is there a reason the PR is still a draft?

[kaczmarczyck]

Approving for HID library code

[kaczmarczyck]

Thanks for reporting this bug in a very old part of the code. I wrote a test to check the new behavior, feel free to add it here, or I will later:

    #[test]
    fn test_timed_out_new_init() {
        let mut env = TestEnv::default();
        let mut assembler = MessageAssembler::default();
        assert_eq!(
            assembler.parse_packet(
                &mut env,
                &zero_extend(&[0x12, 0x34, 0x56, 0x78, 0x81, 0x00, 0x40]),
                None,
            ),
            Ok(None)
        );
        env.clock().advance(TIMEOUT_DURATION_MS);
        assert_eq!(
            assembler.parse_packet(
                &mut env, 
                &zero_extend(&[0x12, 0x34, 0x56, 0x78, 0x81, 0x00, 0x40]),
                None
            ),
            Ok(None)
        ); 
        assert_eq!(
            assembler.parse_packet(
                &mut env,
                &zero_extend(&[0x12, 0x34, 0x56, 0x78, 0x00]),
                None
            ),
            Ok(Some(Message {
                cid: [0x12, 0x34, 0x56, 0x78],
                cmd: CtapHidCommand::Ping,
                payload: vec![0x00; 0x40]
            }))
        );
    }