Closed cpu closed 6 years ago
coveralls
commented
6 years ago | Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
|---|---|---|---|---|---|
| monitor/monitor.go | 11 | 13 | 84.62% | ||
| <!-- | Total: | 35 | 37 | 94.59% | --> |
| Totals | |
|---|---|
| Change from base Build 313: | 0.8% |
| Covered Lines: | 770 |
| Relevant Lines: | 1144 |
rolandshoemaker
commented
6 years ago Generally the code looks good. One major point though, this could, in theory, ignore a split view or rolled back tree (i.e. in the case where a tree is 'recovered' from a DB backup and then extra entries added so the tree is smaller but not a predecessor of the last tree observed).
It seems like since we now have a storage layer for this it would be relatively easy to simply store all of the STHs we've seen and verify that every STH we see matches something already on that list.
cpu
commented
6 years ago It seems like since we now have a storage layer for this it would be relatively easy to simply store all of the STHs we've seen and verify that every STH we see matches something already on that list.
Want to take that as a follow-up issue? As I mentioned in #63 ahead of writing this I was only interested in taking on the immediate fix.
rolandshoemaker
commented
6 years ago I'm fine doing this as a follow-up, although most of this code will end up being reverted in that case. Also there appear to be a handful of debug messages that are left over that should be cleaned up.
@jsha in the future could you hold off on merging when there is still on-going discussion.
jsha
commented
6 years ago Oops, sorry; I missed that there was still an open question. Will do better next time. :-)
If a log gives back a valid STH for a smaller treesize than the previously seen STH (a stale STH) don't overwrite prevSTH and don't check consistency proof. If the stale STH is older than the log's MMD, print an error.
Resolves https://github.com/letsencrypt/ct-woodpecker/issues/61