On 6.7.2018 00:06, Pieter Wuille wrote:> The only case where "malicious"
conflicting values can occur is when
> one of the Signers produces an invalid signature, or modifies any of
> the other fields already present in the PSBT for consumption by
> others. If this were an issue, it would be an issue regardless of the
> Combiner's operation, as in topology A no Combiner is even present.
> This is generally true I think - Combiners can always be replaced with
> just a different (and possibly less parallel) topology of data flow.

This is an interesting thesis, and also an unspoken assumption ISTM. It
seems worth adding something like this to the spec:
"""
In general, the result of Combiner combining two PSBTs from independent
participants A and B should be functionally equivalent to a result
obtained from processing the original PSBT by A and then B in a sequence.
or, for participants performing fA(psbt) and fB(psbt):
Combine(fA(psbt), fB(psbt)) == fA(fB(psbt)) == fB(fA(psbt))
"""

(...)

> The bottom line is that a Combiner which picks arbitrarily in case of
> conflicts will never end up with something worse than what you already
> need to deal with. If you disregard the case of invalid fields
> (because the result will just be an invalid transaction), then any
> choice the Combiner makes is fine, because all the values it can pick
> from are valid.

This sounds reasonable and IMHO it would be good to have a summary of
this argument in the Rationale section.


> If you're worried about attack surface, I don't believe rejecting
> invalid fields ever matters. An attacker can always drop the fields
> you don't understand before giving you the PSBT, making your behavior
> identical to one where you'd have ignore those fields in the first
> place.

Modifying the PSBT requires an active attacker. A passive attacker could
possibly sniff the invalid signatures and misuse them.
Where an active attacker can likely do more than drop fields.


In general, this comes down to a philosophical difference again. I'm
reluctant to sign an input with unknown data, on the premise that there
could be *anything* in that data; the fact that right now I can't come
up with a field that would be problematic does not mean that tomorrow
won't bring one. (in particular, a potential failure here is silent,
invisible to the user)

We are most likely to implement the "do not sign with unknown fields"
rule in any case (technically a whitelist of "known OK" field types),
and resolve potential problems as they arise. I raised this point mainly
because I think discussing this explicitly in the spec is beneficial: a
distinction between mandatory and optional fields is one way, mentioning
or prescribing possible signing strategies is another.

regards
m.