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

Use usedforsecurity=False for md5() calls to make suds work on FIPS enabled systems #72

Merged
merged 1 commit into from
Apr 3, 2022

Conversation

oalbrigt
Copy link
Contributor

No description provided.

@phillbaker
Copy link
Member

phillbaker commented Mar 1, 2022 via email

@oalbrigt oalbrigt force-pushed the FIPS branch 3 times, most recently from 1923912 to 56caa0a Compare March 1, 2022 08:09
@ptoscano
Copy link

ptoscano commented Mar 1, 2022

(part of subscription-manager/virt-who team here)

While this seems OK-ish, I wonder whether the Reader class needs md5 at all. It seems it is used only for caching, so I wonder there a cache system could be implemented without using md5 at all.

@oalbrigt
Copy link
Contributor Author

oalbrigt commented Mar 2, 2022

I think you're right, but I dont see an issue in using it.

suds/reader.py Outdated
h = md5(name.encode()).hexdigest()
try:
h = md5(name.encode()).hexdigest()
except ValueError:
Copy link
Member

Choose a reason for hiding this comment

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

Thanks - I'm wondering if this could have "false positives" of catching ValueErrors (and then trying the version with usedforsecurity) when it shouldn't.

A few other options from looking at how others approach this:

  • Flip the order and catch AttributeError, e.g.
        try:
            # FIPS requires usedforsecurity=False and might not be
            # available on all distros: https://bugs.python.org/issue9216
            h = md5(name.encode(), usedforsecurity=False).hexdigest()
        except AttributeError:
            h = md5(name.encode()).hexdigest()

If this is available in earlier versions of python on RHEL, I can see how checking for python 3.9 doesn't make sense.

No strong opinions on this, but let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds like a better way to do it. I've pushed the update.

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