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

make HMAC work in strict FIPS mode #355

Merged
merged 2 commits into from
Sep 11, 2019
Merged

make HMAC work in strict FIPS mode #355

merged 2 commits into from
Sep 11, 2019

Conversation

tomato42
Copy link
Member

@tomato42 tomato42 commented Sep 5, 2019

on new FIPS compliant Python HMAC is implemented as a thin wrapper around
openssl implementation of HMAC, that makes it impossible to add a
field to the hmac object or use MD5 for hmac-ing in FIPS mode

so re-implement HMAC in pure python (still use native hash implementations)

leave implementation of fallback for later: #356


This change is Reviewable

@tomato42 tomato42 added enhancement new feature to be implemented maintenance issues related to project maintenance, CI, documentation, etc. labels Sep 5, 2019
@tomato42 tomato42 self-assigned this Sep 5, 2019
@tomato42 tomato42 force-pushed the fips-hmac branch 2 times, most recently from 606516b to 6f0327e Compare September 5, 2019 18:10
@tomato42 tomato42 changed the title don't redefine block_size for HMAC objects make HMAC work in strict FIPS mode Sep 5, 2019
on new FIPS compliant Python HMAC is implemented as a thin wrapper around
openssl implementation of HMAC, that makes it impossible to add a
field to the hmac object, but they actually do have the block_size
field and set it to correct values
@The-Mule
Copy link
Collaborator

The-Mule commented Sep 9, 2019

I am still getting the following error:

  File "/tmp/tmp.a5QyuKjZzh/tlsfuzzer/tlslite/mathtls.py", line 632, in createHMAC
    h.block_size = digestmod().block_size
AttributeError: attribute 'block_size' of '_hmacopenssl.HMAC' objects is not writable

@tomato42
Copy link
Member Author

tomato42 commented Sep 9, 2019

@The-Mule that function should never get the '_hmacopenssl.HMAC' object if the second patch is applied...

@The-Mule
Copy link
Collaborator

The-Mule commented Sep 9, 2019

Sorry for false alarm. It was human error, thankfully. Anyway, fixes seem to be working fine. Thanks! First commit is simple enough for me to review it. As for second commit (HMAC implementation), is it completely new code or something already reviewed elsewhere? (noticed 2015 in licence headed).

as new python uses OpenSSL to implement HMAC (so that it is FIPS
compliant), using MD5 for HMAC will not work

as we need it for older ciphers (and to verify that those old ciphers
don't work in FIPS mode) we need to overwrite that limitation
@tomato42
Copy link
Member Author

As for second commit (HMAC implementation), is it completely new code or something already reviewed elsewhere? (noticed 2015 in licence headed).

it's new code, fixed

Copy link

@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.

LGTM

@tomato42 tomato42 merged commit d82944f into master Sep 11, 2019
@tomato42 tomato42 deleted the fips-hmac branch September 11, 2019 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new feature to be implemented maintenance issues related to project maintenance, CI, documentation, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants