Add support for parsing TrustSignature packets

[arudzitis-stripe]

This is written to address: https://github.com/ProtonMail/go-crypto/issues/86

This change:

• Adds parsing of TrustSignature packets on signatures per RFC 4880 5.2.3.13
• Adds this data to the Signature struct
• Adds a unit test to confirm we can parse a signature with this subpacket and re-serialize it

It does not appear to me that this library models trust, so nothing is actually done with the additional trust information except preserve it on the Signature.

[twiss]

Hey :wave: Thanks for the PR, and apologies for the delay! (I was on vacation last week.)

I have a small nitpick about the API: I think the Signature.TrustSignature naming is a bit confusing (admittedly, this is mostly because the name of the subpacket in the spec is confusing), since it's a property of the signature rather than a new signature (i.e. it's not like the "Embedded Signature" subpacket). So I would drop the "Signature" from the name.

And then, to simplify things more, I would tend to add two properties, Signature.TrustLevel and Signature.TrustAmount, instead of a struct. The revocation reason subpacket is also split into two properties, RevocationReason and RevocationReasonText, so I think that's fine. And then, you could serialize it when TrustLevel != 0, since the spec anyway says that "Level 0 has the same meaning as an ordinary validity signature."

[arudzitis-stripe]

@twiss updated with your feedback!

[twiss]

Thanks! Is there some reason TrustAmount needs to be a pointer? I assume if TrustLevel is 0 then TrustAmount is irrelevant too, so it can just default to 0 as well?

If you want to check that the TrustAmount was set explicitly before generating the subpacket, I think you could also just check that it's != 0 as well, as emitting Level > 0 and Amount == 0 seems fairly nonsensical to me.

[arudzitis-stripe]

I agree with the argument it is nonsensical, but it sounds like from the RFC that 0 is an allowed value:

The trust amount is in a range from 0-255, interpreted such that values less than 120 indicate partial trust and values of 120 or greater indicate complete trust

However, I can make it not-a-pointer, and just assume if level > 0, then Amount was intentionally set to 0 (as is allowed by the RFC).

[arudzitis-stripe]

@twiss updated

[twiss]

Right. The downside of this is that someone might forget to set the TrustAmount but since this is a very low-level API I think this is OK, they just need to be careful to set both :)