seed: ReadSystemEssentialAndBetterEarliestTime #10005
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
based on #10004 |
pedronis
force-pushed
the
seed-better-earliest-time
branch
from
8f889ad to
e12e040
Compare
pedronis
changed the title
have a variant of CommitTo that supports a callback to consider each assertion immediately after it has been added to the database, at which point it also verified
ReadSystemEssentialAndBetterEarliestTime retrieves in one go information about the model and essential snaps of the given types for the Core 20 recovery system seed specified by seedDir and label (which cannot be empty). It can operate even if current system time is unreliable by taking a earliestTime lower bound for current time. It returns as well an improved lower bound by considering appropriate assertions in the seed.
pedronis
force-pushed
the
seed-better-earliest-time
branch
from
e12e040 to
6dc91da
Compare
anonymouse64
approved these changes
Mar 16, 2021
seed/seed.go
Outdated
| // and more recent than snap-declarations anyway, | ||
| // other assertions are ignored as they might be added | ||
| // with unreliable times | ||
| if a.Type() == asserts.SnapRevisionType { |
There was a problem hiding this comment.
as mentioned on IRC, we could also use the snap-declaration timestamp here, since the checkConsistency method also verified that the snap-declarations are store-signed too, the case is unlikely where the snap-declaration is newer than the snap-revision, but it could happen and I think it's okay to be slightly more accepting of assertion types here
seed/seed.go
Outdated
Comment on lines
177
to
180
| // we consider only snap-revisions as they are stored-signed | ||
| // and more recent than snap-declarations anyway, | ||
| // other assertions are ignored as they might be added | ||
| // with unreliable times |
There was a problem hiding this comment.
Suggested change
| // we consider only snap-revisions as they are stored-signed | |
| // and more recent than snap-declarations anyway, | |
| // other assertions are ignored as they might be added | |
| // with unreliable times | |
| // we consider only snap-revision and snap-declaration | |
| // assertions here as they must be store-signed, see | |
| // checkConsistency for each type | |
| // other assertions are ignored as they might have a root | |
| // of trust that does not include the brand key that signed | |
| // the model assertion; thereby allowing an attacker with an | |
| // account-key that was signed by canonical to inject new | |
| // assertions that are not rooted in trust with the store directly | |
| // or with the brand key directly |
There was a problem hiding this comment.
I used a slightly different explanation in the end
seed/seed.go
Outdated
| return nil, nil, time.Time{}, err | ||
| } | ||
|
|
||
| // consider model own timestamp as well |
There was a problem hiding this comment.
Suggested change
| // consider model own timestamp as well | |
| // consider the model's timestamp as well - it must be signed | |
| // by the brand so is safe from the attack detailed above |
asserts/batch_test.go
Outdated
|
|
||
| // this is the order they needed to be added | ||
| c.Check(seen, DeepEquals, []*asserts.Ref{ | ||
|
|
seed/seed20_test.go
Outdated
| c.Check(essSnaps, DeepEquals, t.expected) | ||
| c.Check(betterTime.Equal(improvedTime), Equals, true, Commentf("%v expected: %v", betterTime, improvedTime)) | ||
| } | ||
|
|
thanks @anonymouse64 also remove some spurious blank lines here and in asserts
mvo5
approved these changes
Mar 18, 2021
| } | ||
|
|
||
| improve := func(a asserts.Assertion) { | ||
| // we consider only snap-revision and snap-declaration |
| // containing unreliable time | ||
| var tstamp time.Time | ||
| switch a.Type() { | ||
| default: |
There was a problem hiding this comment.
(nitpick^99) most of the switch/case I have seen has the default as the last entry but that is really probably my most nitpicky comment yet.
mvo5
pushed a commit
to mvo5/snappy
that referenced
this pull request
Mar 26, 2021
ReadSystemEssentialAndBetterEarliestTime retrieves in one go information about the model and essential snaps of the given types for the Core 20 recovery system seed specified by seedDir and label (which cannot be empty). It can operate even if current system time is unreliable by taking a earliestTime lower bound for current time. It returns as well an improved lower bound by considering appropriate assertions in the seed. * asserts: Batch.CommitToAndObserve have a variant of CommitTo that supports a callback to consider each assertion immediately after it has been added to the database, at which point it also verified
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ReadSystemEssentialAndBetterEarliestTime retrieves in one go
information about the model and essential snaps of the given types
for the Core 20 recovery system seed specified by seedDir and label
(which cannot be empty).
It can operate even if current system time is unreliable by taking
a earliestTime lower bound for current time.
It returns as well an improved lower bound by considering appropriate
assertions in the seed.