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

Support DeviceCert model as well as the AliasCert model #83

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

alistair23
Copy link
Collaborator

We currently only support AliasCert modes.

This PR implements DeviceCert support, with AliasCert still the default. This makes the requester more dynamic and adds the option --certificate-model device to responders to specify the DeviceCert model.

The slot_id{} check is outdated, we don't seem to ever create the file
so let's remove it.

Signed-off-by: Alistair Francis <[email protected]>
Move the current certificates under the alias directory and also support
device certificates.

Signed-off-by: Alistair Francis <[email protected]>
@alistair23 alistair23 force-pushed the alistair/cert-model branch 2 times, most recently from 6978819 to e6bc4f0 Compare July 15, 2024 06:13
@alistair23 alistair23 force-pushed the alistair/cert-model branch from e6bc4f0 to b008282 Compare July 15, 2024 06:16
} else if certificate_model == "device" {
CertModel::Device
} else {
panic!("Unsupported certificate model");
Copy link
Contributor

@twilfredo twilfredo Jul 15, 2024

Choose a reason for hiding this comment

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

the else can be removed, since certificate_model default to "alias". That part should be unreachable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need the else otherwise Rust throws us

error[E0317]: `if` may be missing an `else` clause
   --> src/main.rs:408:20
    |
408 |               } else if certificate_model == "device" {
    |  ____________________^
409 | |                 CertModel::Device
    | |                 ----------------- found here
410 | |             };
    | |_____________^ expected `CertModel`, found `()`
    |
    = note: `if` expressions without `else` evaluate to `()`
    = help: consider adding an `else` block that evaluates to the expected type

Plus it protects us if we support more models in the future

Copy link
Contributor

@twilfredo twilfredo left a comment

Choose a reason for hiding this comment

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

looks good, were you able to test this over the sockets?

@alistair23
Copy link
Collaborator Author

looks good, were you able to test this over the sockets?

Yeah, it works with both Alias and Device certificates

@alistair23 alistair23 merged commit 706a519 into master Jul 16, 2024
2 checks passed
@alistair23 alistair23 deleted the alistair/cert-model branch July 16, 2024 03:52
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