close
The Wayback Machine - https://web.archive.org/web/20201114121319/https://github.com/letsencrypt/boulder/pull/4308
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

Switch to more efficient multi nonce-service design #4308

Merged
merged 7 commits into from Jun 28, 2019
Merged

Conversation

@rolandshoemaker
Copy link
Contributor

@rolandshoemaker rolandshoemaker commented Jun 27, 2019

A basically complete re-write/re-design of the forwarding concept introduced in #4297 (sorry for the rapid churn here). Instead of nonce-services blindly forwarding nonces around to each other in an attempt to find out who issued the nonce we add an identifying prefix to each nonce generated by a service. The WFEs then use this prefix to decide which nonce-service to ask to validate the nonce.

This requires a slightly more complicated configuration at the WFE/2 end, but overall I think ends up being a way cleaner, more understandable, easy to reason about implementation. When configuring the WFE you need to provide two forms of gRPC config:

  • one gRPC config for retrieving nonces, this should be a DNS name that resolves to all available nonce-services (or at least the ones you want to retrieve nonces from locally, in a two DC setup you might only configure the nonce-services that are in the same DC as the WFE instance). This allows getting a nonce from any of the configured services and is load-balanced transparently at the gRPC layer.
  • a map of nonce prefixes to gRPC configs, this maps each individual nonce-service to it's prefix and allows the WFE instances to figure out which nonce-service to ask to validate a nonce it has received (in a two DC setup you'd want to configure this with all the nonce-services across both DCs so that you can validate a nonce that was generated by a nonce-service in another DC).

This balancing is implemented in the integration tests.

Given the current remote nonce code hasn't been deployed anywhere yet this makes a number of hard breaking changes to both the existing nonce-service code, and the forwarding code.

Fixes #4303.

@rolandshoemaker rolandshoemaker requested a review from letsencrypt/boulder-developers as a code owner Jun 27, 2019
@rolandshoemaker
Copy link
Contributor Author

@rolandshoemaker rolandshoemaker commented Jun 27, 2019

I think there is probably a path to a more elegant prefix discovery/config solution here, but I think it's worth going with the relatively simple config based approach for now.

Copy link
Contributor

@jsha jsha left a comment

Nicely done! Since nonces now have some structure, it would be nice to define a parseNonce that generates a struct containing a prefix and a body, and errors if the length is wrong, or the prefix doesn't hex decode, or the body has non-b64 characters. Seems like we do this parsing ad-hoc in a couple places where we just strip off the first byte.

cmd/boulder-wfe/main.go Outdated Show resolved Hide resolved
cmd/boulder-wfe/main.go Outdated Show resolved Hide resolved
cmd/boulder-wfe2/main.go Outdated Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
Copy link
Contributor

@cpu cpu left a comment

I like this redesign, thanks @rolandshoemaker 👍

cmd/boulder-wfe/main.go Outdated Show resolved Hide resolved
cmd/boulder-wfe/main.go Outdated Show resolved Hide resolved
cmd/boulder-wfe/main.go Outdated Show resolved Hide resolved
cmd/boulder-wfe2/main.go Outdated Show resolved Hide resolved
cmd/boulder-wfe2/main.go Outdated Show resolved Hide resolved
nonce/nonce.go Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
nonce/nonce.go Show resolved Hide resolved
nonce/nonce_test.go Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
nonce/nonce.go Outdated Show resolved Hide resolved
@rolandshoemaker rolandshoemaker dismissed stale reviews from cpu and jsha via ac6f3ff Jun 28, 2019
nonce/nonce.go Show resolved Hide resolved
@jsha
jsha approved these changes Jun 28, 2019
@cpu
cpu approved these changes Jun 28, 2019
@cpu cpu merged commit af41bea into master Jun 28, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cpu cpu deleted the nonce-prefixing-2 branch Jun 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.