Skip to content

Conversation

@liamdennehy
Copy link

@liamdennehy liamdennehy commented Sep 9, 2019

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

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.

liamdennehy added a commit to liamdennehy/eidas-certificate-parse that referenced this pull request Sep 9, 2019
Relies on PR robrichards/xmlseclibs#200
so explicitly depending on commit in the meantime
@peter279k
Copy link

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
Copy link
Author

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
Copy link

peter279k commented Sep 16, 2019

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link

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
Copy link

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

@liamdennehy liamdennehy closed this by deleting the head repository Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants