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

Remove last trace of FIPS stuff #5326

Closed
wants to merge 1 commit into from
Closed

Remove last trace of FIPS stuff #5326

wants to merge 1 commit into from

Conversation

richsalz
Copy link
Contributor

No description provided.

@richsalz richsalz added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Feb 12, 2018
@t-j-h
Copy link
Member

t-j-h commented Feb 12, 2018

-1 please hold off on this until we sort out the forward strategy

crypto/o_fips.c Outdated
return 1;
CRYPTOerr(CRYPTO_F_FIPS_MODE_SET, CRYPTO_R_FIPS_MODE_NOT_SUPPORTED);
return 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

This is a no-can-do until 1.2. (you didn't do a make update, btw)

@richsalz richsalz changed the title Remove last trace of FIPS stuff WIP: Remove last trace of FIPS stuff Feb 12, 2018
@richsalz
Copy link
Contributor Author

Forgot the WIP tag

@levitte levitte added this to the 1.2.0 milestone Feb 12, 2018
@levitte levitte added the 1.2.0 label Feb 12, 2018
@richsalz richsalz removed the 1.2.0 label Feb 12, 2018
@richsalz
Copy link
Contributor Author

I will restore the API.

@richsalz richsalz changed the title WIP: Remove last trace of FIPS stuff Remove last trace of FIPS stuff Feb 12, 2018
@richsalz
Copy link
Contributor Author

API restored, taking off WIP. I know it's going to need a vote, but this is just... waiting.

@nickthetait
Copy link
Contributor

I'm not familiar with the justification for removal of FIPS. Is it because these components are being removed because they are no longer used?

@richsalz
Copy link
Contributor Author

1.1.0 is not FIPS capable. We will be starting a new FIPS project after 1.1.1; see the last section of https://www.openssl.org/blog/blog/2018/01/18/f2f-london/

@t8m
Copy link
Member

t8m commented Mar 5, 2018

Removal of the ...NON_FIPS_ALLOW flags is a borderline API/ABI break. The flags at some point of time existed and applications could set them.

@richsalz
Copy link
Contributor Author

richsalz commented Mar 5, 2018

They are not removed, they are set to zero. No breakage.

@t8m
Copy link
Member

t8m commented Mar 5, 2018

The breakage would happen if you reused the previous values for anything else. So is there any point in setting them to zero rather than keeping the pre-reserved values?

@richsalz
Copy link
Contributor Author

richsalz commented Mar 5, 2018

How would you get those values? Shrug. It doesn't matter because @t-j-h put a -1 on this, and I don't have the energy/interest to take this through a vote so this won't happen.

@t8m
Copy link
Member

t8m commented Mar 5, 2018

The application compiled against older openssl using these defines would set the flags to the old values (and not 0).

@richsalz
Copy link
Contributor Author

richsalz commented Mar 5, 2018

Yes, and zero means setting no flag bits, which means no action, which is what the current behavior is. We've done this "set to zero for no-op" before.

@t8m
Copy link
Member

t8m commented Mar 5, 2018

Maybe I am not clear enough. Imagine application compiled with openssl-1.1.0 that uses one of these defines - it means that it will try to set some value of the flag to non-zero. If you then later reuse this non-zero value in openssl-1.1.x to mean something else than these FIPS flags, you have an ABI breakage. (not API breakage though)

@richsalz
Copy link
Contributor Author

richsalz commented Mar 5, 2018

I understand. I think the FIPS work will at least require a recompile. Maybe I'm wrong. But as I said, Tim put a -1 on this and I'm not working to take it forward past that.

@richsalz richsalz modified the milestones: 1.2.0, Post 1.1.0 May 6, 2018
@richsalz richsalz closed this May 6, 2018
@richsalz richsalz deleted the rm-rest-of-fips branch May 6, 2018 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants