Bug 834740 - fedpkg is depending on md5, which is not allowed in fips mode
fedpkg is depending on md5, which is not allowed in fips mode
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: rpkg (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: David Cantrell
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-23 00:27 EDT by Paul Wouters
Modified: 2013-01-10 01:50 EST (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-07-10 13:39:25 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to mark rpkg's use of md5 as valid (969 bytes, patch)
2012-07-03 11:55 EDT, Toshio Ernie Kuratomi
no flags Details | Diff

  None (edit)
Description Paul Wouters 2012-06-23 00:27:39 EDT
fedpkg won't work in fips mode because it checks the file download cheksums in the sources file using md5.

md5 is weak, and we should not trust it to verify network obtained files.

It would be good if we can migrate away from md5 in the sources file to sha1 or sha256. As this migration will probably take a while, due to packagers needing to update their sources files, it should start sooner rather then later.
Comment 1 Jesse Keating 2012-06-23 00:46:33 EDT
The use of md5 to validate downloads isn't for cryptographic reasons.  We're just making sure that A) the download didn't get corrupted on the way down, and B) we have a unique path to store archives of the same name.  Just because md5 is weak cryptographically doesn't mean it's not useful outside of crypto.

If you want to change how the files are checksummed please open a ticket with Fedora Infrastructure.

Note, md5 functionality will still be required to create (s)rpms for RHEL5 or older targets.
Comment 2 Paul Wouters 2012-06-25 10:21:53 EDT
My understanding was that in the pre-RHEL6 days there was a meeting with releng that sha256 was needed in the tool chain top to bottom and that md5 would be phased out to prevent these issues.

I do think that validating a file that came in over the network qualifies as an md5 usage that is forbidden in fips (unlike say, the md5 cacheid object ids in squid). 

But regardless, if you conbsider it an allowed case, then fedpkg should probably use the private_MD5_Init() instead of MD5Init() to signify this was a conscious decision to override the fips limitation.

eg see: http://www.openssl.org/gitweb.cgi?p=geoff/openssl.git;a=commitdiff_plain;h=e25ed35d50091b675364514b4f7413a9e87a30da
Comment 3 Jesse Keating 2012-06-25 11:42:05 EDT
We're just using python's hashlib to get this done.  If you think that kind of feature should be exposed in hashlib to python callers, file a bug there and get that done.  Until then, I cannot change how it uses the md5 library.
Comment 4 Toshio Ernie Kuratomi 2012-06-26 10:33:43 EDT
That may well be a good bug to open although it does open up the need to do more auditing.  IIRC, python currently does not provide md5 hashing if FIPS mode is enabled.  However, md5 is being provided by python as a standard library routine for other applications to consume.  With md5 not present, many other applications (like fedpkg) break.  Since there are valid uses of md5, it likely makes sense for python to use the private_MD5_Init() as you suggest.  However, that means that python scripts would need to have their usage of md5 audited to see if we should be denying them as opposed to the current behaviour where all uses of md5 are denied.
Comment 5 Paul Wouters 2012-06-26 11:27:43 EDT
python should not per default use the "workaround md5" function. It should just fail with md5, so people are informed and can make a conscious decision about whether or not their md5 usage falls within FIPS or not. Only when it falls outside of FIPS restrictions, should they use a newly exposed md5 function that works in fips mode.
Comment 6 Paul Wouters 2012-06-26 11:28:44 EDT
I think the best way is for fedpkg to migrate its md5 use to sha256/sha512. Then it does not have to worry about md5 in upstream python/hashlib
Comment 7 Toshio Ernie Kuratomi 2012-06-26 11:50:29 EDT
(In reply to comment #5)
> python should not per default use the "workaround md5" function. It should
> just fail with md5, so people are informed and can make a conscious decision
> about whether or not their md5 usage falls within FIPS or not. Only when it
> falls outside of FIPS restrictions, should they use a newly exposed md5
> function that works in fips mode.

Then the bug should have a patch to python to add a new md5 hashlib class to python that does not fail when FIPS mode is on (or add a flag to the current md5 hashlib class that enables it to work when FIPS mode is on).  Currently, there's nothing that people can do when FIPS mode is on and they have to use md5 to communicate with other protocols.
Comment 8 Toshio Ernie Kuratomi 2012-07-03 10:47:35 EDT
Looks like dmalcolm is working on such a patch.  It doesn't look like he's making use of private_MD5_Init() does the proposed patch do the equivalent?

http://bugs.python.org/issue9216

Note that in Fedora this patch looks to have been backported to F17+'s python2 package.  It's only been proposed for python3 upstream (I'm guessing it won't go in earlier than python-3.4+ at this point in the release cycle for python-3.3).  So fedpkg can probably get away with making use of it but other packages whose upstreams are used on more than Fedora might have to carry local patches in their rpm to enable this.

(I'm working on a patch to make use of this functionality).
Comment 9 Toshio Ernie Kuratomi 2012-07-03 11:55:23 EDT
Created attachment 596001 [details]
Patch to mark rpkg's use of md5 as valid

Patch marks fedpkg's use of md5 as valid even under FIPS.

Tested with fedpkg sources on F16 (where usedforsecurity is not available) and F17 (where usedforsecurity is available).  On F16 it works with FIPS mode off and fails (SIG abort, which I believe is the reason dmalcolm continued to work on FIPS mode patches) with FIPS mode on.  No python traceback.  On F17 it works with FIPS mode both on and off.

Tested building RHEL5 SRPMS under FIPS mode using this.  On F17, this fails to produce an RPM that can be built on RHEL5: http://kojipkgs.fedoraproject.org//work/tasks/6457/4216457/root.log    I'm guessing that rpmlib would need to mark some portion of what it does as being a valid use of md5 in order for this to work (and that might *not* be a valid use so....)

Have not tested uploading new sources which is another area that could fail with this.

Scratch build with this patch applied:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4216420
Comment 10 Toshio Ernie Kuratomi 2012-07-03 11:57:44 EDT
Reassigning to rpkg as there's now a patch for that pkg.  Not sure if that's where we should be making changes, though, so not removing python maintainers from the CC just yet.
Comment 11 Miloslav Trmač 2012-07-10 11:50:10 EDT
(In reply to comment #7)
> (In reply to comment #5)
> Then the bug should have a patch to python to add a new md5 hashlib class to
> python that does not fail when FIPS mode is on (or add a flag to the current
> md5 hashlib class that enables it to work when FIPS mode is on).  Currently,
> there's nothing that people can do when FIPS mode is on and they have to use
> md5 to communicate with other protocols.

"Have to use MD5 to communicate with other protocols" is not a sufficient justification to override the FIPS-140 requirements in FIPS mode.  The whole purpose of FIPS mode is to enforce the FIPS-140 requirements on cryptography, even if that means reducing functionality.  (Yes, this might end up meaning that fedpkg just will not run in FIPS mode.)

The private MD5 APIs are there _only_ for non-cryptographic uses of MD5, and "verifying that data was not tampered with" is a cryptographic use.

(In reply to comment #1)
> The use of md5 to validate downloads isn't for cryptographic reasons.  We're
> just making sure that A) the download didn't get corrupted on the way down,
> and B) we have a unique path to store archives of the same name.

Unless I am mistaken, the integrity of the repository is enforced as follows: git's use of SHA-1 provides cryptographically secure identification of commits (allowing us to use tags or otherwise refer to specific versions of non-lookaside files that make a SRPM).  One of these files "protected" by git's SHA-1 is the "sources" file, which lists the look-aside files to be included in the SPRM, and authenticates them using the included hash.  In that case, the use of the hash within "sources" is definitely a "cryptographic" use, and the hash algorithm should not be MD5 nowadays (FIPS-140 or not, MD5 is just too broken); this also means that using the "private" MD5 API for this purpose would not be appropriate.
Comment 12 Miloslav Trmač 2012-07-10 11:55:37 EDT
(In reply to comment #11)
> (In reply to comment #1)
> > The use of md5 to validate downloads isn't for cryptographic reasons.  We're
> > just making sure that A) the download didn't get corrupted on the way down,
> > and B) we have a unique path to store archives of the same name.
> 
> Unless I am mistaken, <concerns about repository integrity>

Alternatively, since packagers download the lookaside files using plain HTTP from 
> lookaside = http://pkgs.fedoraproject.org/repo/pkgs
an attacker could modify the data in transit from pkgs.fedoraproject.org to the packager, replacing one tarball with another that has the same MD5 hash but attacks the packager's machine.  The hash in the "sources" file is the only thing that authenticates the data.
Comment 13 Jesse Keating 2012-07-10 12:21:35 EDT
That's a nice argument, but regardless, fedpkg/rpkg can't do anything about this until the Fedora infrastructure changes.  Push your agenda there, and fedpkg will follow.
Comment 14 Toshio Ernie Kuratomi 2012-07-10 13:27:04 EDT
I think mitr is arguing that fedpkg shouldn't work in FIPs mode if md5 is required :-)  Which is good information as it means my patch to rpkg to use md5 should not be applied.

Dennis and I took a look at switching over to sha256sum this morning .  We think it'll be pretty easy to push this as a flag day event.  Somewhat harder if we want to preserve backwards compatibility.  In either case, we're ready to do the work needed. It'll require changes to the upload.cgi and fedpkg when it talks to upload.cgi.  It'll also require changes to fedpkg to generate the sources file with the sha256sum and to construct buildroots based on the sources file.  koji just makes use of fedpkg to construct the buildroot so no changes needed there.

From mitr's comment, I think that using fedpkg to generate srpms for RHEL5 (where md5 is the only choice of hash algorithm) under FIPs mode should remain a CANTFIX.

Does that cover all the bases?
Comment 15 Jesse Keating 2012-07-10 13:39:25 EDT
I think that covers it.

This bug itself should be closed NOTABUG because it's really not a bug.  The dependence is real.

Note You need to log in before you can comment on or make changes to this bug.