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

[openssl] Use mainline ECC code and drop fips patches. Fixes JB#54628 #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shanechko
Copy link

No description provided.

# The hobble_openssl is called here redundantly, just to be sure.
# The tarball has already the sources removed.
sh %{SOURCE1} > /dev/null

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove that?

Copy link
Author

Choose a reason for hiding this comment

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

To use code from original ec_curve.c

@Thaodan
Copy link
Contributor

Thaodan commented Jun 21, 2021

You should document the changes and not just remove the lines.
E.g. see in:
https://github.com/sailfishos/openssl/blob/master/rpm/openssl.spec#L447

@saukko
Copy link
Contributor

saukko commented Jun 22, 2021

Hi, did a quick look and it seems that this patch contains:

  1. dropping fips patches
  2. dropping ecc patches
  3. dropping openssl-hobble
  4. dropping default value patch
  • maybe something else that I missed?

Still need to do more detailed review, but would be better if this would be in 4 parts and having reasonings for all of those.

In the commit message it is not clear which parts of the commit are done for which reasoning. Best would be to have all as separate commits, but at minimum would have some explanation why each thing was done to understand better what is aimed with all of this.

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