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

Update bootconfig.proto new #168

Merged
merged 9 commits into from
Mar 4, 2024
Merged

Conversation

sachendras
Copy link
Contributor

@sachendras sachendras commented Feb 23, 2024

Update bootconfig.proto to include updating security artifacts aswell part of the SetBootConfigRequest message.

Replaced #163 with the current PR to include changes pushed after the original Pull.

@coveralls
Copy link

coveralls commented Feb 23, 2024

Pull Request Test Coverage Report for Build 8104819386

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 1.254%

Totals Coverage Status
Change from base Build 8070276888: 0.0%
Covered Lines: 166
Relevant Lines: 13241

💛 - Coveralls

@sachendras sachendras marked this pull request as ready for review February 23, 2024 23:23
@sachendras
Copy link
Contributor Author

Moving comments from #163.

by @xw-g:
"gNSI.x client has access to those gNSI namespaces. I don't think we should add those in this API (which is about Bootconfig namespace)"

Response:
gNSI.x Client would allow for reseting only the gNSI artifacts. But gNSI artifacts are referenced by the bootConfig and hence results in commit failures. The change in this pull will provide a single gRPC-based method to reset the boot config including the gNSI artifacts. This also provides parity with the original Bootstrapconfig that is provided by BootZ.

@LimeHat
Copy link

LimeHat commented Feb 26, 2024

gNSI.x Client would allow for reseting only the gNSI artifacts. But gNSI artifacts are referenced by the bootConfig and hence results in commit failures.

Could you please elaborate on this scenario? What steps will lead to a failure?

@sachendras
Copy link
Contributor Author

/gcbrun

@sachendras
Copy link
Contributor Author

gNSI.x Client would allow for reseting only the gNSI artifacts. But gNSI artifacts are referenced by the bootConfig and hence results in commit failures.

Could you please elaborate on this scenario? What steps will lead to a failure?

This is in context to functional test runs at our end and how DUTs (and their different namespaces) should be completely reseted prior to running subsequent functional tests.
In my last comment, I was referring to the possible use of gNSI.x client for an empty rotate operation to delete certs post a certZ test and returning the DUT back to the pool. This action may lead to broken dependencies and subsequent commit failures. However, the proposal in this Pull will helps us to restore bootconfig plus security artifacts via the same API.

@LimeHat
Copy link

LimeHat commented Feb 26, 2024

I mean, the existence of isolated namespaces implies that you could run into dependency issues.. is the answer to define an RPC that merges the two namespaces into a single commit?

Can this be solved on the controller side? Reset the bootconfig first, then reset the gNSI profiles.

Also, I'm not sure that an empty rotate is the way to erase certZ test state.
IMO with the certZ tests, the sequence should look like this:

  1. test creates a new certz profile
  2. reconfigures grpc-servers to use the profile (via gnoi.bootconfig if namespaces are in use)
  3. performs necessary validations
  4. reconfigures the servers back to the previous or default profile
  5. deletes the certz profile

@marcushines marcushines merged commit 6439b98 into openconfig:main Mar 4, 2024
7 checks passed
@sachendras
Copy link
Contributor Author

I mean, the existence of isolated namespaces implies that you could run into dependency issues.. is the answer to define an RPC that merges the two namespaces into a single commit?

Can this be solved on the controller side? Reset the bootconfig first, then reset the gNSI profiles.

Also, I'm not sure that an empty rotate is the way to erase certZ test state. IMO with the certZ tests, the sequence should look like this:

  1. test creates a new certz profile
  2. reconfigures grpc-servers to use the profile (via gnoi.bootconfig if namespaces are in use)
  3. performs necessary validations
  4. reconfigures the servers back to the previous or default profile
  5. deletes the certz profile

For our test workflow (current use case), goal is to have a single RPC that allows for resetting Boot and Security namespaces in a single commit.

@sachendras sachendras deleted the patch-5 branch March 4, 2024 17:49
@LimeHat
Copy link

LimeHat commented Mar 4, 2024

Fair enough (although now the service name is slightly misleading, since it affects more than just a bootConfig).

Having said that, you will not be able to completely reset security namespace using this rpc.
gnsi.Certz certificates field in bootstrapData, which you're reusing here, can contain only certificates since it uses UploadRequest message (same thing is applicable for bootz.proto, but there, as far as I understand, this data was supposed to be used only when attestz/enrollz is not available to rotate from iDevID to oiDevID). It does not provide a capability to manage (delete) multiple profiles or key/cert data from a specific profile.

@sachendras
Copy link
Contributor Author

when attestz/enrollz is not available to rotate from iDevID to oiDevID). It does not provide a capability to manage (delete) multiple profiles or key/cert data from a specific profile.

Right, let's use this Pull as a placeholder for now until we have openconfig/attestz#36 resolved. This problem w/ new ssl profile exists w/ the current BootZ process as well. Therefore, we will have to revisit this pull.

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.

4 participants