Re: [openpgp] Should signatures be rejected if the embedded hash prefix does not match?

[Justus Winter <[e-mail]>]

Andrew Gallagher <andrewg=[e-mail]> writes:

> On 27/02/2023 17:27, Justus Winter wrote:
>> Andrew Gallagher <andrewg=[e-mail]> writes:
>> 
>>> Interop failures can in principle lead to security issues, yes. But do
>>> we know that this particular interop failure does? If so, then there
>>> would be a potential sig forgery attack against github* (amongst
>>> others), which is a *lot* more serious than anyone has previously suggested.
>> 
>> Imagine a setup where a git hoster uses a lenient implementation to
>> check that every commit has a valid signature, and a client that uses a
>> strict implementation that only allows updating if all the commits have
>> a valid signature.
>> 
>> Now, someone pushes a commit with a bad signature that the lenient
>> implementation on the server will accept, but the client will refuse.
>> This is a denial of service attack against the system.
>
> That's exactly the scenario that I'm proposing we should avoid by making 
> the lenient interpretation of v4 sigs standard behaviour. If we can 
> reduce the risk of DoS without increasing the risk of forgery, is that 
> not a good trade?

This is the race to the bottom I'm trying to avoid here.

>>> No, I'm proposing to accept that this particular kind of malformed
>>> artifact should be accepted because the cost/benefit ratio of doing
>>> anything about it now is too high, and to instead concentrate on making
>>> sure it doesn't happen again.
>> 
>> So you are saying we should accept Github's signatures that are
>> malformed in two ways (the digest prefix is wrong and the MPI encoding
>> is wrong), because the benefit is high.
>
> No, I said the *cost/benefit ratio* of *not* accepting malformed 
> prefixes is high. The benefit of either accepting or not accepting them 
> is low either way, because their only value is saving a few clock 
> cycles. The cost of not accepting them is potentially high, because it 
> leads to the DoS scenario you outlined above. The low-cost (and thus 
> low-cost/benefit) option is to accept that v4 prefix bytes cannot be 
> trusted and just ignore them. Which is what your software already does, BTW.

I just love it when other people explain my software to me, using the
results of the tests I wrote.

>> What is the benefit exactly?  Who has access to the key?  Who can create
>> signatures with that key?  Everybody!  What does a signature on a commit
>> from Github even mean?
>
> It's a statement that github validated your identity by some other 
> means. That might not be worth much, but that's not sufficient reason to 
> break things.
>
>> Github has been notified about this issue in July 2022.  I just asked
>> Github to create a signature, and it is malformed.  I don't think they
>> care, I don't think they will do the right thing going forward.
>
> If you don't believe they're going to do the right thing anyway, then 
> what is the point in hitting them with the breaking-change stick? If 
> they do decide to implement v6, then they'll have to interop with 
> clients that follow the spec. This may encourage them to fix their v4 
> code. Or it may not, in which case we're still no worse off than now.

I'm not breaking anything.  It is already broken.  And for some reason
you are trying to take the only instrument away that could possibly
convince them to repair their service.

>>> If implementers
>>> did not fully understand this issue until relatively recently, it is
>>> unreasonable to expect downstream users to have made an informed
>>> choice.
>> 
>> I don't follow.  What is it we the implementers did not understand until
>> recently?  The spec says how to form signatures.  The spec says put the
>> digest prefix there.  If you don't put the digest prefix there, you
>> didn't form a signature.
>
> And yet many implementations - including yours - will happily declare 
> those non-signatures valid. So either they didn't fully understand the 
> interop issues, or they did and chose to ignore them. Either way, it's 
> not downstream's responsibility.

"My implementation" does rely on the digest prefix.  The fact that it
does not reject signatures over data if the digest prefix isn't correct
doesn't change that.  In fact, the ongoing discussion convinces me more
and more that its current behavior is a bug, and that I should make it
more strict.

Anyway, I think we made our points, I'll let someone else chime in.

Best,
Justus