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

FIPS module checksums: add scripts and Makefile rule #8871

Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented May 3, 2019

This adds the following scripts:

util/lang-compress.pl:

Compress source code, which language is determined by the first argument.
For the moment, we know 'perl' (perlasm source code), 'C' (C source code)
and 'S' (Assembler with C preprocessor directives).
This removes comments and empty lines, and compresses series of horizontal
spaces to one single space in the languages where that's appropriate.

util/fips-checksums.sh:

Takes source file names as arguments, pushes them through
util/lang-compress.pl and unifdef with FIPS_MODE defined, and calculates
the checksum on the result.

Fixes #13130

@levitte levitte added the branch: master Merge to master branch label May 3, 2019
@levitte levitte added this to In progress in 3.0 New Core + FIPS via automation May 3, 2019
Configurations/unix-Makefile.tmpl Show resolved Hide resolved
util/c-compress-test.pl Show resolved Hide resolved
util/fips-checksums.sh Show resolved Hide resolved
@levitte levitte force-pushed the calculate-fips-source-checksums branch from fae606d to adcea0f Compare October 22, 2019 11:21
@levitte
Copy link
Member Author

levitte commented Oct 22, 2019

Ping @mattcaswell, @t-j-h, @paulidale

@levitte levitte changed the title FIPS module checksums: add scripts and Makefile rule WIP: FIPS module checksums: add scripts and Makefile rule Nov 1, 2019
@mattcaswell mattcaswell self-assigned this Nov 1, 2019
@kroeckx kroeckx added this to the 3.0.0 beta1 milestone Oct 7, 2020
@levitte levitte force-pushed the calculate-fips-source-checksums branch from adcea0f to e67641c Compare November 6, 2020 07:03
@romen romen added the triaged: OTC evaluated This issue/pr was triaged by OTC label Nov 10, 2020
@paulnelsontx paulnelsontx added this to Ready to merge in 3.0.0 estimator Dec 1, 2020
@mspncp
Copy link
Contributor

mspncp commented Jan 12, 2021

FWIW: I was able to build unifdef using Microsoft Visual Studio Professional 2019.

After cloning,

git clone http://dotat.at/git/unifdef.git

you need to run the reversion.sh script (e.g. using git bash) as follows,

msp@msp-nbw10 MINGW64 /c/src/unifdef (master)
$ ./scripts/reversion.sh

before building the solution

unifdef\win32\unifdef.sln

otherwise Visual Studio will complain about a missing version.h file.

@levitte levitte force-pushed the calculate-fips-source-checksums branch from e67641c to 11eba1e Compare February 3, 2021 23:14
@mattcaswell mattcaswell removed their assignment Mar 24, 2021
@levitte levitte force-pushed the calculate-fips-source-checksums branch from 11eba1e to 566ed74 Compare April 27, 2021 09:07
@levitte
Copy link
Member Author

levitte commented Apr 27, 2021

I've done a major rework of this branch. Instead of magically figuring out the source files for any configuration, we only collect source files for the default and the no-asm configuration and calculates checksums over those files, period.

This should be more or less done at this point. I'll watch the CIs for confirmation, then I'll make this non-WIP.

@levitte levitte marked this pull request as ready for review April 27, 2021 09:11
@mattcaswell
Copy link
Member

Hmmm...the "make update" CI failure in fips-sources.checksums is presumably because of changes in the master branch??

@levitte
Copy link
Member Author

levitte commented Apr 27, 2021

Ah! I was wondering what was going on...

@levitte levitte force-pushed the calculate-fips-source-checksums branch from 614bd7b to 3a1245f Compare April 27, 2021 09:47
@levitte
Copy link
Member Author

levitte commented Apr 27, 2021

BTW, how come we have some sources from ssl/ in the FIPS module?

@levitte
Copy link
Member Author

levitte commented Apr 27, 2021

Rebasing wasn't enough, of course, I should have re-generated the checksums [facepalm]

@levitte levitte force-pushed the calculate-fips-source-checksums branch from 3a1245f to f9b333a Compare April 27, 2021 09:52
@levitte
Copy link
Member Author

levitte commented Apr 27, 2021

Yay, check-update much happier now.

Quick, approve before master changes again! 😉

@mattcaswell
Copy link
Member

BTW, how come we have some sources from ssl/ in the FIPS module?

Because there is now some "padding" related code which we now do provider side. Partly for the reason that it needs to be inside the FIPS boundary. However we can't entirely remove the same code from libssl, e.g. for backwards compatibility in case we are using an engine and no provider is involved.

Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

I suppose we do not want to merge this earlier than just before the beta1 release. Otherwise we would have to update the checksums whenever we touch the fips module code, which still can commonly happen in PRs before beta1.

Configurations/unix-Makefile.tmpl Outdated Show resolved Hide resolved
3.0 New Core + FIPS automation moved this from In progress to Needs review Apr 27, 2021
@mattcaswell
Copy link
Member

Otherwise we would have to update the checksums whenever we touch the fips module code, which still can commonly happen in PRs before beta1.

Is that really too much of a burden? I'd prefer to get this in as soon as possible.

@levitte
Copy link
Member Author

levitte commented Apr 27, 2021

I suppose we do not want to merge this earlier than just before the beta1 release. Otherwise we would have to update the checksums whenever we touch the fips module code, which still can commonly happen in PRs before beta1.

I'm ambivalent. After all, it's not a bad idea to exercise this a few times before things go to stillness...

@levitte levitte force-pushed the calculate-fips-source-checksums branch from ba096fa to d7c0012 Compare April 29, 2021 09:31
OpenSSL::Config::Query is a configuration querying tool that's meant
to make it easier to query the diverse configuration data for info.
That's much easier than to dig through all the parts of %unified_info.
This file will be the basis for the FIPS module checksum calculation
This adds the following scripts:

util/lang-compress.pl:

Compress source code, which language is determined by the first argument.
For the moment, we know 'perl' (perlasm source code), 'C' (C source code)
and 'S' (Assembler with C preprocessor directives).
This removes comments and empty lines, and compresses series of horizontal
spaces to one single space in the languages where that's appropriate.

util/fips-checksums.sh:

Takes source file names as arguments, pushes them through
util/lang-compress.pl and unifdef with FIPS_MODE defined, and calculates
the checksum on the result.
This is required for 'make update' and fips checksums
@levitte levitte force-pushed the calculate-fips-source-checksums branch from d7c0012 to 9613054 Compare April 29, 2021 10:39
3.0 New Core + FIPS automation moved this from Needs review to Reviewer approved Apr 29, 2021
@t8m
Copy link
Member

t8m commented Apr 29, 2021

LGTM. I would wait with merging this at least for discussion in OTC as any two PRs touching the FIPS source files will inevitably have merge conflict.

openssl-machine pushed a commit that referenced this pull request May 4, 2021
OpenSSL::Config::Query is a configuration querying tool that's meant
to make it easier to query the diverse configuration data for info.
That's much easier than to dig through all the parts of %unified_info.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #8871)
openssl-machine pushed a commit that referenced this pull request May 4, 2021
This file will be the basis for the FIPS module checksum calculation

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #8871)
openssl-machine pushed a commit that referenced this pull request May 4, 2021
This adds the following scripts:

util/lang-compress.pl:

Compress source code, which language is determined by the first argument.
For the moment, we know 'perl' (perlasm source code), 'C' (C source code)
and 'S' (Assembler with C preprocessor directives).
This removes comments and empty lines, and compresses series of horizontal
spaces to one single space in the languages where that's appropriate.

util/fips-checksums.sh:

Takes source file names as arguments, pushes them through
util/lang-compress.pl and unifdef with FIPS_MODE defined, and calculates
the checksum on the result.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #8871)
openssl-machine pushed a commit that referenced this pull request May 4, 2021
This is required for 'make update' and fips checksums

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #8871)
openssl-machine pushed a commit that referenced this pull request May 4, 2021
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #8871)
@levitte
Copy link
Member Author

levitte commented May 4, 2021

Merged

841a438 Add OpenSSL::Config::Query and use it in configdata.pm
27ca03e Unix build file: Add a target to create providers/fips.module.sources
be22315 FIPS module checksums: add scripts and Makefile rule
49f699b GitHub CI: ensure that unifdef is installed
f97bc7c [TEMPORARY] make 'make update' verbose in ci.yml

@levitte levitte closed this May 4, 2021
3.0 New Core + FIPS automation moved this from Reviewer approved to Done May 4, 2021
@levitte levitte deleted the calculate-fips-source-checksums branch May 4, 2021 09:35
@h-vetinari
Copy link
Contributor

Perhaps one of the most august 3.0 PRs merged 🥳
Even managed to celebrate its 2nd birthday yesterday 😅

@t8m t8m mentioned this pull request May 4, 2021
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
OpenSSL::Config::Query is a configuration querying tool that's meant
to make it easier to query the diverse configuration data for info.
That's much easier than to dig through all the parts of %unified_info.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#8871)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
This file will be the basis for the FIPS module checksum calculation

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#8871)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
This adds the following scripts:

util/lang-compress.pl:

Compress source code, which language is determined by the first argument.
For the moment, we know 'perl' (perlasm source code), 'C' (C source code)
and 'S' (Assembler with C preprocessor directives).
This removes comments and empty lines, and compresses series of horizontal
spaces to one single space in the languages where that's appropriate.

util/fips-checksums.sh:

Takes source file names as arguments, pushes them through
util/lang-compress.pl and unifdef with FIPS_MODE defined, and calculates
the checksum on the result.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#8871)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
This is required for 'make update' and fips checksums

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#8871)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from openssl#8871)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch triaged: OTC evaluated This issue/pr was triaged by OTC
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Source checksum script
9 participants