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

NK_build_aes_key clears OTP slots #102

Open
d-e-s-o opened this issue Apr 24, 2021 · 4 comments
Open

NK_build_aes_key clears OTP slots #102

d-e-s-o opened this issue Apr 24, 2021 · 4 comments
Assignees
Milestone

Comments

@d-e-s-o
Copy link

d-e-s-o commented Apr 24, 2021

Despite claims to the contrary:

In case only PWS or EV is to be erased, it is actually faster to call just the AES generation. Then the OTP slots should stay intact.

It appears that NK_build_aes_key actually clears all OTP slots on my Nitrokey Storage.

#include <assert.h>
#include "libnitrokey/NK_C_API.h"

int main(void)
{
        assert(NK_login_auto() == 1);
        assert(NK_build_aes_key("12345678") == 0);
        assert(NK_logout() == 0);
        return 0;
}
$ nitrocli otp status
alg     slot    name
totp    0       foobar
totp    1       foobaz
$ ./a.out
$ nitrocli otp status
alg     slot    name

Has something changed?

(I haven't checked out the implementation, so it's very well possible that the issue is rooted somewhere in the firmware, in which case the issue probably should be transferred over to the appropriate repository)

@robinkrahl
Copy link

Apparently, build_aes_keys issues the NEW_AES_KEY command on the Nitrokey Pro and the GENERATE_NEW_KEYS command on the Nitrokey Storage. As far as I can see from the firmware, the Nitrokey Storage does not support the NEW_AES_KEY command. So probably GENERATE_NEW_KEYS clears the OTP slots, but I don’t see where.

@szszszsz szszszsz self-assigned this Apr 26, 2021
@szszszsz
Copy link
Member

szszszsz commented Apr 26, 2021

Hi! Thank you for the report. This should not happen by design.
Just confirmed the same with a Nitrokey Storage v0.55/.52, and Nitrokey App v1.4.0. Looking further.

Edit: Nitrokey Pro v0.7/.11 works as expected.

szszszsz referenced this issue in Nitrokey/libnitrokey Apr 26, 2021
@szszszsz
Copy link
Member

Confirmed OTP erasure is called inside the AES key generation:
BuildStorageKeys_u32() -> EraseLocalFlashKeyValues_u32(),
which contains OTP flash pages clearing (excerpts below). EraseLocalFlashKeyValues_u32() is called as well during Factory Reset operation.

u32 BuildStorageKeys_u32 (u8 * AdminPW_pu8)
{
u32 Ret_u32;
u8 MasterKey_au8[AES_KEYSIZE_256_BIT];
// Check admin password
// Unlock smartcard for sending master key
if (FALSE == LA_OpenPGP_V20_Test_SendAdminPW (AdminPW_pu8))
{
#ifdef LOCAL_DEBUG
CI_TickLocalPrintf ("AdminPW wrong\r\n");
#endif
return (FALSE);
}
Ret_u32 = EraseLocalFlashKeyValues_u32 ();
if (FALSE == Ret_u32)
{
return (FALSE);
}

for (i = 0; i < 10; i++)
{
flashc_erase_page (OTP_FLASH_START_PAGE + i, TRUE);
}

@szszszsz
Copy link
Member

Moving ticket to Nitrokey Storage firmware project

@szszszsz szszszsz transferred this issue from Nitrokey/libnitrokey Apr 26, 2021
@szszszsz szszszsz added this to the v0.56 milestone Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants