gla-rad / SECOMLib

A Java library facilitating the development of SECOM-compliant web interfaces
Apache License 2.0
8 stars 3 forks source link

Issues with validating signatures when client and server have mismatching signature algorithms #8

Closed kaspernielsen closed 2 months ago

kaspernielsen commented 2 months ago

I've run into a problem where a signature could not be validated properly.

I believe the cullprint is SecomSignatureFilter around line 130

            // First decide on the signature algorithm
            final DigitalSignatureAlgorithmEnum digitalSignatureAlgorithm = Optional.ofNullable(this.signatureProvider)
                    .map(SecomSignatureProvider::getSignatureAlgorithm)
                    .orElse(DigitalSignatureAlgorithmEnum.DSA);

The code in question takes the signature algorithm from the signature provider of the client. When in reality it should extract the signature algorithm from the received request.

For example, I use SHA3_384_WITH_ECDSA but the NavUI uses SHA2_256_WITH_ECDSA which doesn't work. If I change my own signature algorithm to SHA2_256_WITH_ECDSA it works fine

nvasta commented 2 months ago

The point made is indeed correct.

The problem is that not in all requests with signatures, is the algorithm specified. So far we have the following envelopes:

Only the last two have SECOM metadata which contain the algorithm.

nvasta commented 2 months ago

As a fix, we now check if the incoming request has a metadata field and then we use the specified algorithm.

Otherwise the code will revert back to the once provided by the signature provider.

final DigitalSignatureAlgorithmEnum digitalSignatureAlgorithm = Optional.of(obj)
                    .map(EnvelopeSignatureBearer::getEnvelopeSignatureAlgorithm)
                    .orElseGet(() -> Optional.of(this.signatureProvider)
                            .map(SecomSignatureProvider::getSignatureAlgorithm)
                            .orElse(DigitalSignatureAlgorithmEnum.DSA));
kaspernielsen commented 2 months ago

Works fine here now