Skip to content

Upgrade signing key generation#379

Merged
kaczmarczyck merged 2 commits intogoogle:developfrom
kaczmarczyck:submit-upgrade
Sep 15, 2021
Merged

Upgrade signing key generation#379
kaczmarczyck merged 2 commits intogoogle:developfrom
kaczmarczyck:submit-upgrade

Conversation

@kaczmarczyck
Copy link
Collaborator

This PR adds the upgrade signing key, including:

  • generate in setup.sh
  • encoding in build.rs
  • import in ctap/key_material.rs
  • mention in install.md

On top, it adds one missing function to get the partition offset to the interface, and cleans up some unnecessary code after the compiler version jump.
The key is currently unused in key_material.rs. For the bigger picture, please check my upgrade branch.

@kaczmarczyck kaczmarczyck self-assigned this Sep 15, 2021
@google-cla google-cla bot added the cla: yes label Sep 15, 2021
@coveralls
Copy link

coveralls commented Sep 15, 2021

Coverage Status

Coverage increased (+0.6%) to 42.458% when pulling 664bad1 on kaczmarczyck:submit-upgrade into 7a975ac on google:develop.

ia0
ia0 previously approved these changes Sep 15, 2021
Copy link
Member

@ia0 ia0 left a comment

Choose a reason for hiding this comment

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

I also took a quick look at the upgrade branch. I guess you split the partition location in 2 because of the MPU? I actually realized I could probably do the same for the storage (and avoid the MPU hack :D). I'll try next time I have time.


pub const AAGUID: &[u8; AAGUID_LENGTH] =
include_bytes!(concat!(env!("OUT_DIR"), "/opensk_aaguid.bin"));
pub const _UPGRADE_PUBLIC_KEY: &[u8; _UPGRADE_PUBLIC_KEY_LENGTH] =
Copy link
Member

Choose a reason for hiding this comment

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

Last time, after discussion, I realized that #[warn(dead_code)] (and not #[allow(dead_code)]) might be more convenient (no need to change names). As you want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that, and realized that we error on warning. So it's either allow or renaming. And in this case, it wasn't too much work to rename.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right this is not great. We might want to remove that rule in develop branch.

@kaczmarczyck
Copy link
Collaborator Author

@ia0 Yes, the split is to satisfy the MPU. The ugly regex script will make room for new boards eventually though. :)

@kaczmarczyck kaczmarczyck merged commit 596b478 into google:develop Sep 15, 2021
@kaczmarczyck kaczmarczyck deleted the submit-upgrade branch September 15, 2021 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants