Re: [PATCH 3/7] pack-protocol.txt: Mark all LFs in push-cert as required

> On Mon, Jul 6, 2015 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Dave Borowitz <dborowitz@google.com> writes:
>>
>>> The server can munge pkt-lines and reinsert LFs, but it _must_ have
>>> some way of reconstructing the payload that the client signed in order
>>> to verify the signature. If we just naively insert LFs where missing,
>>> we lose the ability to verify the signature.
>>
>> I still do not understand this part.
>>
>> There is no way to "naively" insert, is there?  You have an array of
>> lines (each of which you have already stripped its terminating LF at
>> its end).  How else other than adding one LF at the end of each
>> element do you reconstruct the original multi-line string the client
>> signed?  Are there other ways that makes the result ambiguous??
>
> I think I understand the confusion now. I think you and I are working
> from different assumptions about the client behavior.
>
> My assumption was: the intended behavior for the client is to sign the
> exact payload that was sent over the wire, minus pkt-line headers.
>
> For example, under my assumption, if the client sent:
>
> 0008foo\n
> 0007bar
> 0008baz\n
>
> then this indicates the client signed:
>
> "foo\nbarbaz\n"
>
> Under this assumption, "naively inserting LF" means inserting an LF
> after "bar". Thus the server would record the following in a
> persistent store:
>
> "foo\nbar\nbaz\n"
>
> If we only store this string, and do not remember the fact that the
> client originally omitted one of those LFs, then when we go back to
> verify that signature later, it will fail.
>
> That was my assumption.
>
> Your assumption, IIUC, is that the payload the client signed MUST have
> contained LFs in between each line. When framing the content for the
> wire, the client MUST send one "logical line", which has no trailing
> LF, per pkt-line, and furthermore the pkt-line content MAY contain an
> additional trailing LF.
>
> Under your assumption, the server always has enough information to
> reconstruct the original signed payload.
>
>
> The problem with the documentation, then, is that the documentation
> does not say anything to indicate that the signed payload is anything
> other than what is on the wire.

> So maybe this series should include an explicit description of the
> singed payload outside of the context of a push. Then, in the push
> section, we can describe the set of transformations that the client
> MUST perform (splitting on LF; adding pkt-line headers) and MAY
> perform (adding LFs).
>
>>> If we say the payload the client signs MUST have LFs only in certain
>>> places, then that gives the server enough information to reconstruct
>>> the payload and verify the signature.
>>>
>>> But if we say the signed payload MUST have LFs and the wire payload
>>> MAY have LFs, then now we have two completely different formats, only
>>> one of which is documented.
>>
>> I thought that was what I was saying.  The wire protocol sends the
>> contents of each line (both what is signed and the signature) on a
>> separate packet.  When I say "contents of a line", I do not include
>> the terminating LF as part of the line (iow, LF is not even
>> optional; the terminating LF is not considered as part of "the
>> contents of a line").  It becomes irrelevant that a pkt-line may or
>> may not have a trailing optional LF.  If there is LF at the end of a
>> packet between "push-cert" and "push-cert-end" packets, that LF by
>> definition cannot be part of the "contents of a line" in a
>> certificate.
>>
>> It is just a pkt-line framing artifact you can and should remove if
>> you are doing a "split to array, join with LF" implementation to
>> recover the original string.
>>
>> And that is very much consistent with the way we send other things
>> with pkt-line protocol.  Each packet up to the first flush is
>> expected to have <object name> and <refname> as ref advertisement.
>> The pkt-line framing may or may not add a trailing LF, but LF is not
>> part of <refname>.  It is not even part of the payload; merely an
>> artifact of pkt-line framing.