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

Enchanced TA validation + CLI auth in docker deployment #197

Merged
merged 2 commits into from
Sep 20, 2023
Merged

Conversation

setrofim
Copy link
Collaborator

@setrofim setrofim commented Sep 15, 2023

  • Update corim package dependency to latest, which feature updated trust
    anchor representations that allow for tighter validation based on a
    set of predefined types.
  • In addition to the validation we get "for free" from the above, add
    checks to endorsement decoders to make sure that the TA's are of the
    expected type for the scheme.
  • Upgrade the CLI tools in the docker deployment to the latest versions
    that have auth support. This fixes end-to-end script which did not work
    since the deployment had auth enabled.

@@ -57,7 +57,7 @@ RUN go mod download &&\
go install google.golang.org/protobuf/cmd/[email protected] &&\
go install google.golang.org/grpc/cmd/[email protected] &&\
go install github.com/mitchellh/[email protected] &&\
go install github.com/veraison/corim/cocli@latest &&\
go install github.com/veraison/corim/cocli@eeb7bd48 &&\
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought we strongly enforce revisions now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That hash is assocated with a v2.0.0rc0 tag, however because it's a v2 tag for a modules without a /v2, using it won't work. (conversely, moving to /v2 without publishing a release won't work either), it's a catch22 in the go module versioning system, it seems. So have to use the hash here.

@@ -21,7 +21,7 @@
"environment": {
"class": {
"id": {
"type": "PSA_IOT.impl-id",
"type": "psa.impl-id",
Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande Sep 18, 2023

Choose a reason for hiding this comment

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

is there a specific reason for this change?

This kind of breaks the nice alignment of the Store key-value symmetry for PSA_IOT?

Please reconsider!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is required by corim (I think I mentioned this in our discussions earlier?). For whatever reason, corim templates explect psa. This is hard-coded:

https://github.com/veraison/corim/blob/main/comid/classid.go#L168

I agree that we should align how we represent things in the stores vs the templeas (and better yet, we should get rid of these entirely unnecessary prefixes). However that would require making further changes to corim that orthogonal to what is being implemented here.

Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

In general I am OK, but I have few queries. May be if you have time to discuss we can do it today or on Wednesday (20th Sept)

- Update corim package dependency to latest, which feature updated trust
anchor representations that allow for tighter validation based on a
set of predefined types.
- In addition to the validation we get "for free" form the above, add
checks to endorsement decoders to make sure that the TAs are of the
expected type for the scheme.

Signed-off-by: Sergei Trofimov <[email protected]>
Upgrade the CLI tools in the docker deployment to the latest versions
that have auth support. This fixes end-to-end script which did not work
since the deployment had auth enabled.

Signed-off-by: Sergei Trofimov <[email protected]>
Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

LGTM!

@setrofim setrofim merged commit ae1d91f into main Sep 20, 2023
9 checks passed
@setrofim setrofim deleted the deploy-update branch September 20, 2023 09: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