Verifying a nil commitment silently passes

[AMarcedone]

https://github.com/google/keytransparency/blob/master/core/client/kt/verify.go#L70:

// VerifyCommitment verifies that the commitment in `in` is correct for userID.
func (Verifier) VerifyCommitment(userID, appID string, in *tpb.GetEntryResponse) error {
	if in.Committed != nil {
		entry := new(tpb.Entry)
		if err := proto.Unmarshal(in.GetLeafProof().GetLeaf().GetLeafValue(), entry); err != nil {
			return err
		}
		if err := commitments.Verify(userID, appID, entry.Commitment, in.Committed); err != nil {
			return err
		}
	}
	return nil
}

If in.committed == nil the verification silently passes (no error is thrown). Is this intended? If so, why?
I would expect an error to be raised when Committed == nil

[AMarcedone]

Related: the VerifyGetEntryResponse function from the same file does not have any test in verify_test.go.

[gdbelvin]

Good catch - this was trying to test for the case where we were returning a proof of absence.
Instead, we should be skipping this this only if entry.Commitment is nil.

[AMarcedone]

Wouldn't it be better to handle this at an higher level during verification?
I.e. if there is no committed data, then do not call the commitment verification at all?

[gdbelvin]

Indeed it does. I'm making this change in #716