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

Fix verifyServerCertificateChain failing to pass with old nodeId #593

Closed
2 tasks done
tegefaulkes opened this issue Oct 19, 2023 · 4 comments
Closed
2 tasks done
Labels
development Standard development

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Oct 19, 2023

Specification

The verifyServerCertificateChain in client/utils.ts needs to be fixed. By design it should pass verification for a cert chain that contains the desired NodeId anywhere in the valid chain. Currently this is failing.

The function needs to checked and the exact issue found and a fix applied. Ultimately you should be able to verify a server with any valid NodeId inside it's cert chain.

Additional context

Tasks

  • 1. Find the root cause of the verify function failing to verify with an older NodeId within it's chain.
  • 2. Apply a fix.
@tegefaulkes tegefaulkes added the development Standard development label Oct 19, 2023
@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented Nov 9, 2023

Found the problem. At some point the order the certs were provided in had changed. So it was checking the chain backwards. Not the order is expected to be leaf -> root.

I'm applying this fix to all of the verify functions.

I'm also thinking of moving the agent level verify functions from network to nodes domain. They're not actually shared anywhere.

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Nov 9, 2023

Standardise is so that first element of the array is always leaf.

So leaf, intermediate, root.

The only issue is that if you need to add a new leaf it's a unshift operation. But this is rare.

Generally you care about the leaf first so left to right is more regular.

@tegefaulkes
Copy link
Contributor Author

Ok, both the client and nodes verify functions have been fixes. I added tests for each of them showing that they work as expected. I also moved the nodes verify functions from network to the nodes domain.

@CMCDragonkai
Copy link
Member

If the default order of this arrays is always leaf, intermediate, root.

Then make sure when you are printing them out, you need reverse the order. Because when you print in pem chain format, it is always root first, then leaf last.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
Development

No branches or pull requests

2 participants