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

[WIP] Provide alternatives to md5 signatures #3447

Closed
wants to merge 3 commits into from

Conversation

mwichmann
Copy link
Collaborator

@mwichmann mwichmann commented Sep 16, 2019

Work in progress (see TODO)

Broadly, two changes

  1. in the Visual Studio area, use uuid (Python standard library) to generate GUID identifiers instead of specifically using md5. Simplifies things as uuid lib can provide already formatted strings and that does not have to be done in place. TODO: some of the wired-in GUIDs in tests need to change to what is being produced now.
  2. For use by file hashes, fall back to sha1 if md5 is not available (rare, but documented in Python documentation as being possible in a "FIPS Compliant" build of Python, and actually encountered by an scons user); while considering hashslib itself always present. Like uuid, this is Python standard library since well before the oldest supported Python version (both added in 2.5). Util.py now describes the hash being used in Util.md5, this is used to distinguish expected values in tests. Except for tests, nobody should need to look inside: Util.md5 will still evaluate True.

Security comment: the hashes are not used for any security purpose whatsoever, only for file-change detection, which in turn is used only for rebuild calculations, not for tampering detection. Thus the alternative is sha1, which also is "not secure", but which is as fast (in recent Py3 apparently even faster) as md5 hash computation. An additional fallback, should anyone develop problems with sha1 is sha256.

Signed-off-by: Mats Wichmann [email protected]

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

@bdbaddog
Copy link
Contributor

One larger note.. the function names should be changed from MD5 to Hash or SConsHash or something like that?

@mwichmann
Copy link
Collaborator Author

Sure, a more considered rework could do that.

@bdbaddog
Copy link
Contributor

So if you build with no md5 and then rebuild with, it's going to rebuild everything right?

@bdbaddog
Copy link
Contributor

Should we store the hash type with the hash in the sconsign?

"md5:",etc.. ?

@mwichmann
Copy link
Collaborator Author

Pushed a rebase of this to get a fresh build - I had previously updated the tests to get the new guid strings, but must have lost that set of changes when I pulled patches into this branch. I'll fix that up, as well as the problem in the sconsign tests.

mwichmann added a commit to mwichmann/scons that referenced this pull request Jan 10, 2020
Different expected guid in MSVS tests
Initialize SCons.Util.md5 to a string as expected for sconsign tests
One sider complaint; ignoring the one about star imports as
UtilTests.py use 30+ symbols from SCons.Util.

Signed-off-by: Mats Wichmann <[email protected]>
Work in progress (see TODO)

Broadly, two changes

(1) in the Visual Studio area, use uuid (Python standard library)
to generate GUID identifiers instead of specifically using md5.
Simplifies things as uuid lib can provide already formatted strings
and that does not have to be done in place.
TODO: some of the wired-in GUIDs in tests need to change to what
is being produced now.

(2) For use by file hashes, fall back to sha1 if md5 is not available
(rare, but documented in Python documentation as being possible in a
"FIPS Compliant" build of Python, and actually encountered by an scons
user); while considering hashslib itself always present.  Like uuid,
this is Python standard library since well before the oldest supported
Python version (both added in 2.5).  Util.py now describes the hash
being used in Util.md5, this is used to distinguish expected values in
tests. Except for tests, nobody should need to look inside: Util.md5
will still evaluate True.

Security comment: the hashes are not used for any security purpose
whatsoever, only for file-change detection, which in turn is used only for
rebuild calculations, not for tampering detection. Thus the alternative
is sha1, which also is "not secure", but which is as fast (in recent
Py3 apparently even faster) as md5 hash computation. An additional
fallback, should anyone develop problems with sha1 is sha256.

Signed-off-by: Mats Wichmann <[email protected]>
Different expected guid in MSVS tests
Initialize SCons.Util.md5 to a string as expected for sconsign tests
One sider complaint; ignoring the one about star imports as
UtilTests.py use 30+ symbols from SCons.Util.

Signed-off-by: Mats Wichmann <[email protected]>
New way of generating gives different values,
so update "expected" values

Signed-off-by: Mats Wichmann <[email protected]>
mwichmann added a commit to mwichmann/scons that referenced this pull request May 1, 2020
In the Visual Studio area, use uuid (Python standard library)
to generate GUID identifiers instead of specifically using md5.
Simplifies things as uuid lib can provide already formatted strings
of the form that we need.

This is split off from PR SCons#3447 by request.

Signed-off-by: Mats Wichmann <[email protected]>
@bdbaddog
Copy link
Contributor

bdbaddog commented Dec 14, 2020

Can you prune this down to just the MSVS UUID as I'm planning merging @grossag 's hashing pr #3709 shortly?
Hmm. or is all the non change default hash logic already merged with #3634 ?

@mwichmann
Copy link
Collaborator Author

Withdrawing, the intent of these changes has been incorporated through two other merged PRs.

@mwichmann mwichmann closed this Dec 14, 2020
@mwichmann mwichmann deleted the wip-md5less branch June 29, 2024 16:52
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.

2 participants