lysann/passforios-gopenpgp
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Wrap the cause of signature verification errors.

Browse source 
Instead of swallowing the cause of verification errors,
we use error wrapping to communicate the cause to the caller.
This commit is contained in:
M. Thiercelin 2023-03-06 13:52:17 +01:00
parent 58dbea76e7
commit e9fca4d62f
No known key found for this signature in database
GPG key ID: 29581E7E24EBEC0A
5 changed files with 48 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

5
CHANGELOG.md
View file

@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## Unreleased
### Changed
- The `SignatureVerificationError` struct now has a `Cause error` field, which is returned by the the Unwrap function. The cause is also included in the error message.
NB: If the caller was relying on the exact message of the error, it might break the flow.
## [2.6.1] 2023-03-22
### Security fix

4
crypto/mime_test.go
View file

@ -303,7 +303,7 @@ func TestMessageVerificationNotSignedNoVerifier(t *testing.T) {
func TestMessageVerificationNotSignedFailed(t *testing.T) {
callbackResults := runScenario(t, "testdata/mime/scenario_13.asc")
var expectedErrors = []SignatureVerificationError{newSignatureNotSigned(), newSignatureFailed()}
var expectedErrors = []SignatureVerificationError{newSignatureNotSigned(), newSignatureFailed(nil)}
compareErrors(expectedErrors, callbackResults.onError, t)
expectedStatus := []int{3}
compareStatus(expectedStatus, callbackResults.onVerified, t)
@ -335,7 +335,7 @@ func TestMessageVerificationNoVerifierNoVerifier(t *testing.T) {
func TestMessageVerificationNoVerifierFailed(t *testing.T) {
callbackResults := runScenario(t, "testdata/mime/scenario_23.asc")
var expectedErrors = []SignatureVerificationError{newSignatureNoVerifier(), newSignatureFailed()}
var expectedErrors = []SignatureVerificationError{newSignatureNoVerifier(), newSignatureFailed(nil)}
compareErrors(expectedErrors, callbackResults.onError, t)
expectedStatus := []int{3}
compareStatus(expectedStatus, callbackResults.onVerified, t)

26
crypto/signature.go
View file

@ -29,23 +29,33 @@ var allowedHashes = []crypto.Hash{
type SignatureVerificationError struct {
Status int
Message string
Cause error
}
// Error is the base method for all errors.
func (e SignatureVerificationError) Error() string {
if e.Cause != nil {
return fmt.Sprintf("Signature Verification Error: %v caused by %v", e.Message, e.Cause)
}
return fmt.Sprintf("Signature Verification Error: %v", e.Message)
}
// Unwrap returns the cause of failure.
func (e SignatureVerificationError) Unwrap() error {
return e.Cause
}
// ------------------
// Internal functions
// ------------------
// newSignatureFailed creates a new SignatureVerificationError, type
// SignatureFailed.
func newSignatureFailed() SignatureVerificationError {
func newSignatureFailed(cause error) SignatureVerificationError {
return SignatureVerificationError{
Status: constants.SIGNATURE_FAILED,
Message: "Invalid signature",
Cause: cause,
}
}
@ -108,7 +118,7 @@ func verifyDetailsSignature(md *openpgp.MessageDetails, verifierKey *KeyRing) er
return newSignatureNoVerifier()
}
if md.SignatureError != nil {
return newSignatureFailed()
return newSignatureFailed(md.SignatureError)
}
if md.Signature == nil ||
md.Signature.Hash < allowedHashes[0] ||
@ -238,21 +248,25 @@ func verifySignature(
_, err = signatureReader.Seek(0, io.SeekStart)
if err != nil {
return nil, newSignatureFailed()
return nil, newSignatureFailed(err)
}
sig, signer, err = openpgp.VerifyDetachedSignatureAndHash(pubKeyEntries, origText, signatureReader, allowedHashes, config)
}
}
if err != nil || sig == nil || signer == nil {
return nil, newSignatureFailed()
if err != nil {
return nil, newSignatureFailed(err)
}
if sig == nil || signer == nil {
return nil, newSignatureFailed(errors.New("gopenpgp: no signer or valid signature"))
}
if verificationContext != nil {
err := verificationContext.verifyContext(sig)
if err != nil {
return nil, newSignatureFailed()
return nil, newSignatureFailed(err)
}
}

2
crypto/signature_collector.go
View file

@ -110,7 +110,7 @@ func (sc *SignatureCollector) Accept(
case errors.Is(err, pgpErrors.ErrUnknownIssuer):
sc.verified = newSignatureNoVerifier()
default:
sc.verified = newSignatureFailed()
sc.verified = newSignatureFailed(err)
}
} else {
sc.verified = newSignatureNoVerifier()

36
crypto/signature_test.go
View file

@ -75,11 +75,25 @@ func TestVerifyTextDetachedSig(t *testing.T) {
}
}
func checkVerificationError(t *testing.T, err error, expectedStatus int) { //nolint: unparam
if err == nil {
t.Fatalf("Expected a verification error")
}
castedErr := &SignatureVerificationError{}
isType := errors.As(err, castedErr)
if !isType {
t.Fatalf("Error was not a verification errror: %v", err)
}
if castedErr.Status != expectedStatus {
t.Fatalf("Expected status to be %d got %d", expectedStatus, castedErr.Status)
}
}
func TestVerifyTextDetachedSigWrong(t *testing.T) {
fakeMessage := NewPlainMessageFromString("wrong text")
verificationError := keyRingTestPublic.VerifyDetached(fakeMessage, textSignature, testTime)
assert.EqualError(t, verificationError, "Signature Verification Error: Invalid signature")
checkVerificationError(t, verificationError, constants.SIGNATURE_FAILED)
err := &SignatureVerificationError{}
_ = errors.As(verificationError, err)
@ -342,9 +356,7 @@ func Test_VerifyDetachedWithUnknownCriticalContext(t *testing.T) {
0,
)
// then
if err == nil || !errors.Is(err, newSignatureFailed()) {
t.Fatalf("Expected a verification error")
}
checkVerificationError(t, err, constants.SIGNATURE_FAILED)
}
func Test_VerifyDetachedWithUnKnownNonCriticalContext(t *testing.T) {
@ -423,9 +435,7 @@ func Test_VerifyDetachedWithWrongContext(t *testing.T) {
verificationContext,
)
// then
if err == nil || !errors.Is(err, newSignatureFailed()) {
t.Fatalf("Expected a verification error")
}
checkVerificationError(t, err, constants.SIGNATURE_FAILED)
}
func Test_VerifyDetachedWithMissingNonRequiredContext(t *testing.T) {
@ -481,9 +491,7 @@ func Test_VerifyDetachedWithMissingRequiredContext(t *testing.T) {
verificationContext,
)
// then
if err == nil || !errors.Is(err, newSignatureFailed()) {
t.Fatalf("Expected a verification error")
}
checkVerificationError(t, err, constants.SIGNATURE_FAILED)
}
func Test_VerifyDetachedWithMissingRequiredContextBeforeCutoff(t *testing.T) {
@ -553,9 +561,7 @@ func Test_VerifyDetachedWithMissingRequiredContextAfterCutoff(t *testing.T) {
verificationContext,
)
// then
if err == nil || !errors.Is(err, newSignatureFailed()) {
t.Fatalf("Expected a verification error")
}
checkVerificationError(t, err, constants.SIGNATURE_FAILED)
}
func Test_VerifyDetachedWithDoubleContext(t *testing.T) {
@ -581,7 +587,5 @@ func Test_VerifyDetachedWithDoubleContext(t *testing.T) {
verificationContext,
)
// then
if err == nil || !errors.Is(err, newSignatureFailed()) {
t.Fatalf("Expected a verification error")
}
checkVerificationError(t, err, constants.SIGNATURE_FAILED)
}