Upgrade goxmldsig from v1.4.0 to v.1.5.0

[Pe-te]

github.com/russellhaering/goxmldsig v1.4.0
github.com/russellhaering/goxmldsig v1.5.0

When upgrading goxmldsig from v1.4.0 to v1.5.0 the following two tests fail:

service_provider_test.go:1436: assertion failed:
expected error "cannot validate signature on Assertion: Signature could not be verified",
got "cannot validate signature on Assertion: crypto/rsa: verification error"
--- FAIL: TestXswPermutationSevenIsRejected (0.00s)

service_provider_test.go:1467: assertion failed:
expected error "cannot validate signature on Assertion: Signature could not be verified",
got "cannot validate signature on Assertion: crypto/rsa: verification error"
--- FAIL: TestXswPermutationEightIsRejected (0.00s)

This also happens in the automated dependabot pull request here: #607

Any idea what is causing this or how to fix it?

Differences between the two versions can be seen here:

https://github.com/russellhaering/goxmldsig/compare/v1.4.0...v1.5.0?diff=split&w

I tried some debugging, but the stack trace where the errors happen is completely different in v1.4.0 and v1.5.0.

v1.4.0:

github.com/russellhaering/goxmldsig@v1.4.0/validate.go:L265

	if !bytes.Equal(digest, decodedDigestValue) {
		return nil, errors.New("Signature could not be verified")
	}

v1.5.0:

crypto/internal/fips140/rsa/pkcs1v15.go:L125

	if !bytes.Equal(em, expected) {
		return ErrVerification
	}

The data for em and expected looks like this, it's not even the same length:

Line 265 in v1.4.0 is moved to line 356 in v1.5.0:
https://github.com/russellhaering/goxmldsig/compare/v1.4.0...v1.5.0?diff=split&w#diff-bbf4d9531c9f596bdac5b426ac47babf2ad4d4b0a2196e8f013de35ab8515886R356

But it never gets to line 356 in v1.5.0 because the new code in line 288 returns the rsa error:

	err = cert.CheckSignature(algo, canonicalSignedInfoBytes, decodedSignature)
	if err != nil {
		return nil, err
	}

This is going too deep for me, so I'm giving up and hope someone can use the information to find a fix.
I'm not exactly sure what permutations are and what the purpose of the test is.
But I would guess it should fail at the same place, not be cut off earlier by something else?

• maybe the rsa key is not good enough and needs and update
• might be we just need to change the expected error string
• something completely different

good luck! 😄

[Pe-te]

have updated the question with more info, hope that helps!

[Pe-te]

I just saw that in pull request 662 the two assertions are more lax and only check if the first parts of the error messages exist instead of comparing the whole error messages:
https://github.com/crewjam/saml/pull/662/changes#diff-31eefa9f7c0eb84c75ccbacb2a3f5d2233fea91929557d419f481607f2cb7b81

When/If the pull request 662 or something similar gets merged, this issue can be closed.