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

Use feature-complete SE050 backend #378

Merged
merged 4 commits into from
Dec 1, 2023
Merged

Use feature-complete SE050 backend #378

merged 4 commits into from
Dec 1, 2023

Conversation

sosthene-nitrokey
Copy link
Collaborator

@sosthene-nitrokey sosthene-nitrokey commented Nov 13, 2023

@sosthene-nitrokey
Copy link
Collaborator Author

It doesn't yet supports RSA 4096 bit key generation when the SE050 backend is enabled. We will need to make it a runtime flag for that to work.

@sosthene-nitrokey sosthene-nitrokey force-pushed the se050-backend branch 3 times, most recently from b7f968f to a0c7d47 Compare December 1, 2023 13:27
Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

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

LGTM, not tested on hardware yet. I think it makes more sense to do a big test round once everything is merged.

request,
resources,
)
}
Extension::Se050Manage => ExtensionImpl::<
Copy link
Member

Choose a reason for hiding this comment

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

Not now, but for the future: We should rename this to *Info or *Test so that we don’t use the same name for two different things.

Comment on lines 49 to +52
"fido" => self.fido.field(key),
"opcard" => self.opcard.field(key),
Copy link
Member

Choose a reason for hiding this comment

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

for the future: we should try to always use App::CLIENT_ID in the Config implementations

#[derive(Debug, Default, PartialEq, Deserialize, Serialize)]
pub struct OpcardConfig {
#[cfg(feature = "se050")]
#[serde(default, rename = "t", skip_serializing_if = "is_default")]
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe use s instead of t for the rename to keep at least some connection between the key and the field? or maybe we should just assign numbers …

se050-test-app = ["se050", "admin-app/se050"]
se050 = ["trussed-se050-backend", "dep:se05x"]
se050 = ["dep:se05x", "trussed-se050-backend"]
Copy link
Member

Choose a reason for hiding this comment

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

My understanding was that we can keep the test app, can’t we?

@sosthene-nitrokey sosthene-nitrokey merged commit a788f3c into main Dec 1, 2023
8 checks passed
@sosthene-nitrokey sosthene-nitrokey deleted the se050-backend branch December 1, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants