Skip to content

Add support for writing to and reading from a slot #6

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 21, 2019
Merged

Add support for writing to and reading from a slot #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae0e5db
Add writing and reading raw data into/from a slot
Jun 11, 2019
634c41f
Cast to a non-const pointer to drop a compiler warning
Jun 19, 2019
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: 37 additions & 1 deletion atecc608a_se.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ static psa_status_t atecc608a_import_public_key(psa_key_slot_number_t key_slot,

ASSERT_SUCCESS_PSA(atecc608a_init());

ASSERT_SUCCESS(atcab_write_pubkey(key_id, pubkey_for_driver(p_data)));
ASSERT_SUCCESS(atcab_write_pubkey(key_id, pubkey_for_driver((uint8_t *) p_data)));
exit:
atecc608a_deinit();
return status;
Expand Down Expand Up @@ -367,6 +367,42 @@ psa_status_t atecc608a_asymmetric_verify(psa_key_slot_number_t key_slot,
return status;
}

psa_status_t atecc608a_write(uint16_t slot, size_t offset, const uint8_t *data, size_t length)
{
psa_status_t status = PSA_ERROR_GENERIC_ERROR;

/* The hardware has slots 0-15 */
if (slot > 15)
{
return PSA_ERROR_INVALID_ARGUMENT;
}

ASSERT_SUCCESS_PSA(atecc608a_init());
ASSERT_SUCCESS(atcab_write_bytes_zone(ATCA_ZONE_DATA, slot, offset, data, length));

exit:
atecc608a_deinit();
return status;
}

psa_status_t atecc608a_read(uint16_t slot, size_t offset, uint8_t *data, size_t length)
{
psa_status_t status = PSA_ERROR_GENERIC_ERROR;

/* The hardware has slots 0-15 */
if (slot > 15)
{
return PSA_ERROR_INVALID_ARGUMENT;
}

ASSERT_SUCCESS_PSA(atecc608a_init());
ASSERT_SUCCESS(atcab_read_bytes_zone(ATCA_ZONE_DATA, slot, offset, data, length));

exit:
atecc608a_deinit();
return status;
}

#define PSA_ATECC608A_LIFETIME 0xdeadbeefU

static psa_drv_se_asymmetric_t atecc608a_asymmetric =
Expand Down
10 changes: 10 additions & 0 deletions atecc608a_se.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,14 @@ psa_status_t atecc608a_init();

psa_status_t atecc608a_deinit();

/** Read from a given slot at an offset. Data zone has to be locked for this
* function to work. */
psa_status_t atecc608a_read(uint16_t slot, size_t offset, uint8_t *data, size_t length);
Copy link

Choose a reason for hiding this comment

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

Why would the driver expose a way to read/write at an offset? The API only provides a way to import/export the key as a whole.

Copy link
Contributor Author

@AndrzejKurek AndrzejKurek Jun 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

There is a possibility to store arbitrary data at any slot (slot 8 in our case), for example a certificate, or a signature. The user might want to write and/or read to a slot, depending on the configuration.

Copy link

Choose a reason for hiding this comment

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

Ok, but that would be exposed through the API via psa_import_key and psa_export_key with the key type PSA_KEY_TYPE_RAW_DATA. These functions read/write the whole slot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I didn't know that that's the plan. I can refactor the import_key and export_key procedures to factor for that, I would just like to get a confirmation from @Patater.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like we don't need extra function pointers in the struct then, since we have import and export function pointers already. Let's use those.

Do we want to register a new driver struct under a different lifetime for handling raw data? or add a switch statement inside the existing driver import/export function implementation?

Copy link
Contributor Author

@AndrzejKurek AndrzejKurek Jun 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The export_key function won't know about the key type unless it's passed to it or if it will check the hardware to determine which method of reading to use.

Copy link

Choose a reason for hiding this comment

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

Oh, you mean the export_key function in the driver? I guess it should receive the key attributes as an argument. Right now it only gets the size and location. Why isn't it enough to know the size and location, by the way?

Copy link
Contributor Author

@AndrzejKurek AndrzejKurek Jun 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The public key is stored fragmented and padded, and has to be read using a dedicated atcab_get_pubkey function that converts it to a readable format. Clear text reads are used for raw data, and exporting a private key is not even possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so the secure element API has to be updated. Should I do it? Will it be a part of a different PR? @Patater - are we planning on switching from https://github.com/Patater/mbed-crypto/tree/secure-element to an official branch?

Copy link

Choose a reason for hiding this comment

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

There are other pending updates to the SE API to cope with driver registration, key registration and updates to the application API. That's on me at the moment, together with https://jira.arm.com/browse/IOTCRYPT-608 and https://jira.arm.com/browse/IOTCRYPT-778


/** Write to a given slot at an offset. If the data zone is locked, offset and
* length must be multiples of a word (4 bytes). If the data zone is unlocked,
* only 32-byte writes are allowed, and the offset and length must be
* multiples of 32. */
psa_status_t atecc608a_write(uint16_t slot, size_t offset, const uint8_t *data, size_t length);

#endif /* ATECC608A_SE_H */