robrichards / xmlseclibs

A PHP library for XML Security
BSD 3-Clause "New" or "Revised" License
387 stars 181 forks source link

Add RSA MGF1 signature types #200

Open liamdennehy opened 5 years ago

liamdennehy commented 5 years ago

I have no idea what I am doing! Someone with crypto expertise please check my work! :smile:

It seems OAEP and MGF1 refer to the same (or are simply included) underlying functions, so simply extending the XMLSecurityKey class with the new definitions in RFC6931#Section-2.3.10 does the job, but maybe it's not that simple.

Tested to correctly verify against sample in #199, unable to test generation, so no units included either.

peter279k commented 4 years ago

The Travis CI build is failed because the default dist has been changed.

According to the Travis CI blog post, the default dist change into xenial, not trusty.

To let php-5,4 to php-5.6 versions be supported correctly, defining the matrix setting and let these versions on that. Then setting the dist is trusty.

liamdennehy commented 4 years ago

Fixed the trusty issue, but this requires more work:

$ if [[ $EXECUTE_COVERAGE != 'true' ]]; then phpunit tests; fi
PHPUnit 7.0.2 by Sebastian Bergmann and contributors.

This version of PHPUnit is supported on PHP 7.1 and PHP 7.2.
You are using PHP 7.0.27 (/home/travis/.phpenv/versions/7.0.27/bin/php).
The command "if [[ $EXECUTE_COVERAGE != 'true' ]]; then phpunit tests; fi" exited with 1.

No idea why the version of phpunit that is included in a travis vm is incompatible with the version of php in the same vm.

peter279k commented 4 years ago

Hi @liamdennehy, thanks for your concern.

The PHPUnit version problem is about internal Travis CI environment.

Some useful discussion is here.

To avoid this internal Travis problem, I suggest we can consider using the vendor/bin/phpunit version directly rather than using complicated condition to dispatch different works for phpunit at this moment.

liamdennehy commented 4 years ago

Last three commits have been addressing that, 4857aeb provides for composer install when required and fe4c990 includes better script readability. Build times have unfortunately increased, but still tolerable.

https://travis-ci.org/robrichards/xmlseclibs/builds/585488348 is now a pleasant shade of green...

liamdennehy commented 4 years ago

Note This PR still has no tests, and especially has only been made to validate, but not produce the required signature in my own exemplar.

liamdennehy commented 4 years ago

rather than using complicated condition to dispatch different works for phpunit

I don't think the new scripts complicate things too much, though this is the first dependency in the project's travis build so I only require when actually necessary. I can propose a move to composer's phpunit exclusively, though I don't know how that intersects with the coverage tests in 7.0.27, and is outside the scope of this PR's purpose.

peter279k commented 4 years ago

rather than using complicated condition to dispatch different works for phpunit

I don't think the new scripts complicate things too much, though this is the first dependency in the project's travis build so I only require when actually necessary. I can propose a move to composer's phpunit exclusively, though I don't know how that intersects with the coverage tests in 7.0.27, and is outside the scope of this PR's purpose.

Yes. I think it's about another PR and issue.

7118path commented 3 years ago

I could not get this to work correctly on an external created signature, therefore used this code as base for another pullrequest #222