Populate MessageDetails.SignatureError when unverified

[emersion]

It's invalid for library users to look at MessageDetails.SignatureError
before the OpenPGP message is fully read. Populate it with an error
(cleared once the message is fully read).

This is technically a breaking change, but code looking at
SignatureError before the full message is read is broken anyways.

References: #92

[emersion]

Hi @twiss, did you have the chance to have a look at this patch?

[twiss]

Hi @emersion, sorry for the delay, and thanks for the PR!

The patch looks reasonable, the only thing I dislike about it is that it sets an error when the message is signed and unverified, but not when the message is unsigned (and thus also unverified). The former is not necessarily worse than the latter, so that seems non-ideal to me.

My proposal in #92 to add an IsVerified property would be symmetric across these two scenarios, but another way to do this would be to also add a "not signed" error, for example, so that IsVerified would be equivalent to SignatureError == nil. However, IsVerified seems simpler to me (compared to setting an error on initialization), also because it defaults to false (which is what we want) without any extra work.

[emersion]

I don't like IsVerified because this is yet another bit in MessageDetails which users can forget to check and/or misunderstand.

[twiss]

Hey 👋 Sorry for the delay.
It shouldn't be "yet another bit to check", it should be the only bit to check, if what you care about is whether a signature was verified 😅 I.e. md.IsVerified would replace md.SignedBy != nil && md.SignatureError != nil (in your proposal) which in turn replaces md.SignedBy != nil && md.SignatureError != nil && (message has been read).
In my opinion, having an IsVerified property is also clearer than having an API contract about which properties to check in order to make sure that the message has been verified, if that makes sense.

[emersion]

In short:

• "We have too many confusing fields in this struct, users can't easily tell which one to use."
• "Let's add a new one to fix that!"

Which sounds not too great from a security PoV. Anyways, I'm no longer interested in fixing this library.