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

Clarify documentation on TLS cert usage #7

Merged
merged 4 commits into from
Dec 21, 2023

Conversation

jenia-grunin
Copy link
Contributor

@jenia-grunin jenia-grunin commented Dec 12, 2023

The PR reflects updated requirements agreed in https://b.corp.google.com/issues/311141279, https://b.corp.google.com/issues/309421084, https://buganizer.corp.google.com/issues/285110110, https://buganizer.corp.google.com/issues/304566614 as well as 2023 NCP Switch Requirements go/ncp-security-switch-requirements-2023.

README.md Outdated
4. Active card notifies switch owner infra that a new control card is inserted.
5. Switch owner initiates enrollz and attestz workflow for the new standby card. Once attestz succeeds, switch owner issues to the standby control card its own TLS credentials/cert.

Although this is highly undesirable, if per-control-card TLS cert is temporary unsupported and instead both control cards must share the same prod TLS key pair and cert, then the primary/active control card can only sync the TLS credentials/cert to the newly inserted standby/secondary card after the standby card have been enrolled and attested by the switch owner.
Copy link
Contributor

Choose a reason for hiding this comment

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

the previous section seems to conflict with this one? either it is supported or it isn't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think "if per-control-card TLS cert is temporary unsupported" suggests any contradictions with previous sections, but I am also ok with removing this paragraph altogether. If some of the vendors end up not being able to deliver "per-control-card TLS cert support" by the set timelines, we can deal with them on a case by case basis and consider suggesting a phased-out approach similar to the one offered in this paragraph. How does that sound?

Copy link
Contributor

Choose a reason for hiding this comment

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

@morrowc - Thoughts?

I am not clear this is actually the only model we should support from an openconfig perspective. This makes several assumptions about the only security models that are supported for OC.

I think we should make sure this more broadly encompasses all workflows that operators may want to utilize for securing thier devices.

We can suggest this is this preferred method of operation but i do think there are other scenarios which are still valid and actually more inline with current vendor hardware.

Significant amount of existing vendor hardware doesn't even have TPM's and do not have a mode which doesn't copy credentials between control cards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Marcus. The thing is that we already discussed this, which is why go/ncp-security-switch-requirements-2023 has been finalized and shared with all vendors. If you want to bring these concerns again, please raise them in that doc or directly with authors.

This README is only authoritative when it comes TPM enrollment and attestation, while Prod TLS cert issuance procedure is a separate workflow and is outside of the scope of this README (which is mentioned in the doc). If you want, I can highlight it in bold that this doc is not authoritative on TLS issuance workflow/APIs mechanics, but that it does mandate that "switch-owner-issued production TLS credentials/certs can only be accessible to control cards that have been TPM enrolled and attested by switch owner".

This paragraph ("Although this is highly undesirable, if per-control-card TLS cert is temporary unsupported...") is there to address cases where vendors cannot support "per-control-card TLS cert" requirement in the shorter term future. I am also ok with removing it completely as I suggested in the previous comment.

"Significant amount of existing vendor hardware doesn't even have TPM's". Since this repo is about TPM enrollment and attestation, it would not be relevant to such vendors.

Please let me know what you think and how the wording can be improved to better align with our broader agreement and long term goals.

@marcushines marcushines merged commit 0a919d0 into openconfig:main Dec 21, 2023
2 checks passed
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