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

Incorrect fabric ID validation in fabric table #15892

Closed
bzbarsky-apple opened this issue Mar 5, 2022 · 3 comments
Closed

Incorrect fabric ID validation in fabric table #15892

bzbarsky-apple opened this issue Mar 5, 2022 · 3 comments
Assignees
Labels
spec Mismatch between spec and implementation V1.0

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

As far as I can tell, the fabric id validation added in #15829 does not match the spec. It both allows things the spec does not allow, and disallows things the spec allows.

Specifically:

  • If the ICAC has a fabric id of 0, that is invalid per spec, but that code allows it, as long as there is no fabric id in the RCAC.
  • If the RCAC has a fabric id of 0, that is invalid per spec, but that code allows it.
  • If the RCAC has a fabric ID but the ICAC does not, that is valid per spec as far as I can tell, but that code disallows it.

Proposed Solution

Fix the validation.

@emargolis
Copy link
Contributor

I posted #15920 to address some of the comments in this ticket.

Note that checks for validity of the NodeId, FabricId and other DN attributes is performed when
loading certificates for validation (LoadCert()). So there is no need to check validity
again in FabricInfo::VerifyCredentials().

@bzbarsky-apple
Copy link
Contributor Author

If we do the checks elsewhere, great. Looks like #15920 removes the checking for things that are not actually required to be true, so that would be sufficient to resolve this if the other checks are done elsewhere.

@emargolis
Copy link
Contributor

addressed with #15920

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec Mismatch between spec and implementation V1.0
Projects
None yet
Development

No branches or pull requests

2 participants