Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for absent parameters field in PSS keys #1583

Closed

Conversation

JanSlabon
Copy link
Contributor

I just played with CSR creation and had an example created by OpenSSL (as per this simple guide) which ended in AlgorithmIdentifiers without parameters.

This seems to be ok, in view to this:

[...]The parameters may be either absent or present when used as subject public key information.[...]

@terrafrost
Copy link
Member

Looks good! Just make the update that I mentioned and I'll merge it in!

@terrafrost
Copy link
Member

Oh - also, why do you have Dss in the unit test names? Feels like Pss might be more appropriate? Dss, to me, implies digital signature scheme, aka DSA.

@JanSlabon
Copy link
Contributor Author

Oh - also, why do you have Dss in the unit test names? Feels like Pss might be more appropriate? Dss, to me, implies digital signature scheme, aka DSA.

Sure, this is a typo!

@@ -151,8 +151,6 @@ public static function load($key, $password = '')
$result['MGFHash'] = str_replace('id-', '', $params['maskGenAlgorithm']['parameters']['algorithm']);
if (isset($params['saltLength'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the isset check as well. saltLength is always going to be there, even if the orig key didn't explicitly specify it.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, because $params is an empty array if there are no parameters.

@terrafrost
Copy link
Member

I squashed your commits and cherry-picked them into the 3.0 branch:

1314599

I then merged that into master.

Thanks!

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.

None yet

2 participants