Tock V2 port - rebased and updated

[kaczmarczyck]

This PR is similar to #580. I made some changes and new observations during testing. This PR is WIP still. Most parts of this PR are ready for review, only the patches/ need more attention (see observations).

Differences (FYI @L0g4n) :

• I removed all configs for the opentitan board, as I can't test on them. Adding them back shouldn't be hard: Changes were in boards/ and deploy.py.
• I reverted to RTT debugging for the dev board, instead of UART.

Observations:

• Binary size decreases by 6 kB.
• If we use RustCrypto instead of our own implementation, binary size increases by 23 kB (compared to Tock V2 with our own crypto). Also usage feels much slower. It was working otherwise, so it should work for any non-embedded implementations.
• The storage driver implementation needs a page-sized buffer and does potentially unnecessary copies on each write.
• The USB driver makes the kernel panic when canceling UP checks. My best guess is the CTAP_CMD_TRANSMIT_OR_RECEIVE command, is this command is the unique source of CTAP HID cancel packets (not to be confused with the USB cancel syscall).

[kaczmarczyck]

The USB driver is fixed. I tested a bit more, and OpenSK worked reliably on the second commit. Desription is updated.

[ia0]

I tested the following on a dev board (clear storage + panic console):

• reset, setup, deploy, configure
• webauthn.io register, authenticate, restart, authenticate
• CTAP2 test tool (see below)

I don't know if it's the test suite or this PR, but some button touch are not taken into account.

The test suite did not finish successfully, I've hit the following error:

Test successful: Tests whether the PIN auth token regenerates after replug.
You have 10 seconds for the next touch after pressing enter.
Please replug the device, then hit enter.

F0503 11:57:45.783972 1074220 command_state.cc:64] Check failed: fido2_tests::Status::kErrNone == device_->Init() CTAPHID initialization failed
[...]
    @     0x5619ac5e0e7a  fido2_tests::CommandState::PromptReplugAndInit()
    @     0x5619ac5e0f9e  fido2_tests::CommandState::Reset()
    @     0x5619ac5e1305  fido2_tests::CommandState::Prepare()
    @     0x5619ac5de469  fido2_tests::BaseTest::Setup()
    @     0x5619ac594a99  fido2_tests::runners::RunTests()
[...]

I've also hit the following warnings and errors during test suite:

[...]
Test successful: Tests if the PIN protocol parameter is checked in MakeCredential.
Expected error code `CTAP2_ERR_PIN_AUTH_INVALID`, got `CTAP1_ERR_INVALID_PARAMETER`.
Test successful: Tests if a PIN auth is rejected without a PIN set in MakeCredential.
[...]
Failed test: Tests if client PIN fails with missing parameters in MakeCredential. (id: make_credential_pin_auth_missing_parameter) - Missing PIN parameters were not rejected when PIN is set.
A prompt was sent unexpectedly.
Expected error code `CTAP2_ERR_PIN_REQUIRED`, got `CTAP2_OK`.
[...]
Test successful: Tests if storing lots of credentials is handled gracefully.
The test for full store errors was aborted after 50 credentials were successfully created.
[...]
Failed test: Tests is user verification set to true is accepted in GetAssertion. (id: get_assertion_option_uv_true) - The user verification option (true) was not accepted.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
The failing error code is `CTAP2_ERR_PIN_AUTH_INVALID`.
[...]
Test successful: Tests if assertions with resident keys work.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
Please touch your security key!
[...]
Test successful: Tests if assertions with non-resident keys work.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
Please touch your security key!
[...]
Test successful: Tests if the PIN protocol parameter is checked in GetAssertion.
Expected error code `CTAP2_ERR_PIN_AUTH_INVALID`, got `CTAP1_ERR_INVALID_PARAMETER`.
Please touch your security key!
[...]
Failed test: Tests if the PIN auth is correctly checked with a PIN set in GetAssertion. (id: get_assertion_pin_auth) - Falsely rejected valid PIN auth.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
The failing error code is `CTAP2_ERR_PIN_AUTH_INVALID`.
[...]

Not sure if any of those should be particularly blocking. LGTM to proceed otherwise.

[kaczmarczyck]

The first looks like something that happens when the device hasn't booted quickly enough after a reset. The retry loop in the Test Tool doesn't always find the device. Is it reproducible for you? For the warnings and errors, I'll check if any of them is a regression. They could be outdated tests.

[ia0]

The warnings and errors are reproducible, here are the remaining ones:

[...]
Test successful: Tests if Reset actually deletes credentials.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
[...]
RESULTS
Failed test: Tests if client PIN fails with missing parameters in MakeCredential. (id: make_credential_pin_auth_missing_parameter) - Missing PIN parameters were not rejected when PIN is set.
A prompt was sent unexpectedly.
Expected error code `CTAP2_ERR_PIN_REQUIRED`, got `CTAP2_OK`.
Failed test: Tests is user verification set to true is accepted in GetAssertion. (id: get_assertion_option_uv_true) - The user verification option (true) was not accepted.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
The failing error code is `CTAP2_ERR_PIN_AUTH_INVALID`.
Failed test: Tests if the PIN auth is correctly checked with a PIN set in GetAssertion. (id: get_assertion_pin_auth) - Falsely rejected valid PIN auth.
A prompt was expected, but not performed. Sometimes it is just not recognized if performed too fast.
The failing error code is `CTAP2_ERR_PIN_AUTH_INVALID`.
Passed 63 out of 66 tests.
To re-run tests that failed, supply the following flag:
--test_ids=make_credential_pin_auth_missing_parameter,get_assertion_option_uv_true,get_assertion_pin_auth
[...]

This time I gave plenty of time between resetting the board and hitting enter in the test suite. The tool did not fail.

Regarding the button issue, I'm even more unsure it's the PR. I used to use the next message from the tool as feedback that the button was pressed, but sometimes the tool takes a considerable amount of time before showing the next line. I tried to check whether the LEDs were blinking and could confirm that some of the times when I didn't get feedback in the tool, the LEDs actually stopped blinking which means the button touch worked. However, it's hard to keep an eye on both the tool and the LEDs at the same time, so not sure if all cases can be explained with this theory. Suggestion would be to improve the tool by printing when the button is touched.

I also noticed that when the tool was saying "don't touch although LEDs are blinking", it happened once that they were not blinking (or were blinking after some non-negligible amount of time). Not sure if expected.

Apart from that, no major testing issues identified on my side.

[zhalvorsen]

Have you checked boot time differences on the dev board between Tock V1 and Tock V2? It sounds like you are seeing the same thing I am.

I'm about 1/4 the way through reviewing. I'll finish today.

[kaczmarczyck]

From looking at the debug output, it looks like Tock boots in under 1 second for me on the dev board. No big difference between V1 and V2.

[kaczmarczyck]

Also the Test Tool outputs from before and after this PR are identical. Not sure about the other observations you made, but most tests consist of more than one call to the authenticator. So there can be delay between starting the test and the blinking. In this specific case, there is an easier explanation: After the long warning message that you shouldn't touch, the Tool waits 2 seconds. This is because people tend to just spam press the button to pass all tests, and it's hard to react to the warning message otherwise.

So looks to me like everything is working so far? Also no problems on the dongle for me.

[ia0]

I didn't review, only tested as described in #620 (comment) and #620 (comment). No blocking issues on my side. However, I think we should wait at least @zhalvorsen or @jmichelp to approve before merging.

[zhalvorsen]

Bringing a question from the previous PR here.

Is there a specific reason this syscall driver is here instead of in third_party/libtock-drivers? I think it is the only one that lives outside

[kaczmarczyck]

Historic reasons. I could move it, this is just the smallest diff solution. I can see why that could make things cleaner.

[zhalvorsen]

If we do it, it would probably be better in a different PR anyway. So I'm good to move on.

[kaczmarczyck]

Sounds good. I'll look into it tomorrow and merge if @jmichelp has no objections.

[zhalvorsen]

LGTM but I didn't look too closely at the patches

[kaczmarczyck]

MacOS CI didn't run on the last small commit, but they were green on all other commits.

[L0g4n]

Thanks for the work of all contributors 👍🏾

[kaczmarczyck]

Thanks for your initiative! If you want to add your board, feel free to open another PR. :)