Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
write some words about lease renewal secrets #1118
write some words about lease renewal secrets #1118
Changes from all commits
78f70d6
b6173ee
11a0b8d
bb63331
3ba379c
8fe9532
78a1d65
a864bd5
bb57fcf
7219291
ee22430
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole section would be a lot clearer if there was also some code included to demonstrate (with type annotations).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like this?
I'm not really sure where to put it, though, or what kind of quality control to try to apply to it ... I suppose I could try to observe a few derived renewal secrets from the actual client software and then capture them as test vectors ... But that code isn't exactly easy to invoke.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that is what I was thinking. You could just put it after the prose section.
I was able to validate that implementation by looking at existing code, other than the logic that calculates
peer_id
. But... that logic doesn't match the prose spec anyway, since it's supposed to be based on x509 certificate, not tub ID? So if you fix that line I think it's fine.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tubid is the sha1 hash of the x509 certificate. So I think it works out correctly the way I implemented it above. It's true that "tubid" is a Foolscap concept and the code that computes it is in Foolscap.
I didn't feel like grabbing
cryptography
and making the certificate an input to this function. The tubid is currently quite convenient to find (it's in every storage fURL) and that won't immediately change even after we drop Foolscap (because Tahoe will have to be capable of accepting fURLs for a long, long time).So I'd like to leave this as-is is for now but I would be happy to see it revisited in the near future to scrub all traces of Foolscap from the spec.