Conversation
nmattia
left a comment
There was a problem hiding this comment.
Thanks a lot! I'm trying to understand everything that's happening here and have a couple questions. Haven't looked at that code in a while...
| runGit :: [T.Text] -> IO [T.Text] | ||
| runGit args = do | ||
| -- TODO: only clone shallow repository and fetch needed commits to speed up verification | ||
| ensureAncestor :: T.Text -> T.Text -> T.Text -> IO T.Text |
There was a problem hiding this comment.
I'm trying to wrap my head around what's happening here. If I understand --rollback-protection correctly, the goal is to prevent rolling back to an ancestor. So here you ensure that the old rev is an ancestor of the new rev.
Wouldn't that prevent a user from changing branches?
o---o---o---B
/
---o---o---o---A
If the user was on revision A, and changed the branch to B, wouldn't this implementation throw an error?
Would it make sense to invert the check and do something like this?
ensureNotAncestor ... =
exitCode <- readProcessWithExitCode' "git" ["merge-base", "--is-ancestor", newRev, oldRev]
case ExitCode
ExitSuccess -> fail "new rev should not be an ancestor"
_ -> pure ()There was a problem hiding this comment.
Yes, it would prevent branch changes iff the new branch head is not an ancestor of the old rev.
I see that this might be surprising for users that 'just' want to switch branches.
Maybe an additional update flag --ignore-rollback-protection would make sense in these cases?
Could you give an example for when the inverted check would be sensible?
`ensureNotAncestor could for example succeed if I switched from a recent state to an old branch-off, but fail if the old rev was before the branching:
o---o---o---C
/
---A---o---o---o---B
niv update dep # A -> B
niv modify -b fork
niv update dep # B -> C, succeeds
niv modify -b fork
niv update dep # A -> C, fails
adfbb49 to
1085861
Compare
1085861 to
b004911
Compare
This adds rollback protection to the git fetcher.
Motivation
For the moment this merely serves the purpose of making sure no downgrades to running systems happen.
For example users could prohibit accidental Nixpkgs downgrades an thus prevent their server-deployment from receiving an unanticipated software downgrade.
However, when using commit verification such as guix git authenticate or git-verify (of which I'm the author), it is important to verify no rollback-attack is performed. As Nix' functional model can't deal with those itself I implemented them in Niv.