Re: GPG-Signed Commits Exploit

[Jim Hyslop]

Alexander Taler wrote:
> The design document implies that the GPG signature is made on the
> full file as it is committed.  As a developer this bothers me,
> because it means I'm signing other people's code, not just my
> own.  Chewing on this nagging doubt uncovered an exploit:
>
> The attacker, Eve, needs shell (non-cvs) access to the
> repository, (which can be assumed, since CVS has not been fully
> audited for security), as well as commit access, possibly through
> a compromised key.  Eve commits malicious code in revision 1.18
> of file foo.c, signed with Aaron's key which she has compromised.
> Beth, an honest developer, commits revision 1.19 of file foo.c,
> signed with her uncompromised key.  Eve then returns to the scene
> of the crime, and modifies revision 1.18 to be merely an
> innocuous change.  Later on, it is discovered that Aaron's key
> was compromised, and all of his commits are audited, and found to
> be acceptable.  But the change lives on in Beth's commit.

If we assume that the deltas do not get signed, the malicious change can 
still be detected. The auditors should not only look at each of Aaron's 
commits, but also the first revision after his commits that is signed 
with a valid key, in this case Beth's revision 1.19.

Your scenario assumes that the compromised key is discovered. A more 
likely scenario, I think, is that somebody notices the exploit Eve 
introduced, and determines that it was added in rev 1.19 by Beth. Unless 
Beth can demonstrate that Aaron's key was compromised, she'll take the 
fall and Eve will go unpunished.

Earlier, we discussed mechanisms for allowing loginfo access to the 
signature, so that the signatures can be stored in a secure area. 
Unfortunately for Beth, without signed deltas this mechanism pretty much 
convicts her of introducing the malicious code: Aaron checks in revision 
1.18, its signature is stored in the secure area. Eve modifies rev 1.18 
and changes the signature. At this point, the tampering could be 
detected *if* someone compares the signature in the repository against 
the signature in the secure area. Let's assume that nobody does this, 
though, and the rest of your scenario is followed: Beth commits her 
changes, then Eve completes her attack.

At this point, the signature for rev1.18 in the repository will compare 
correctly against the stored signature, and will validate that rev 1.18 
is the revision that was checked in by Aaron. Eve has not touched Beth's 
revision, so her signature is still valid.

> This scenario would be avoided if the actual diff being committed
> were signed instead of/as well as the complete file.

I think it would have to be "as well as."  Any use of 'cvs admin -o' 
will render delta signatures invalid.

I suppose the client could issue a diff command and sign the resulting 
output, and send that signature to the server as well as the revision 
signature.

--
Jim