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.

Sign up for GitHub

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

Jump to bottom

Verify if the key is correct and check the hmac of the payload #45

Closed
jeffli678 opened this issue Feb 26, 2020 · 2 comments · Fixed by #46
Closed

Verify if the key is correct and check the hmac of the payload #45

jeffli678 opened this issue Feb 26, 2020 · 2 comments · Fixed by #46
Labels
enhancement

Comments

@jeffli678
Copy link
Contributor

jeffli678 commented Feb 26, 2020 •
edited

I found this repo after I made my own ad-hoc implementation. The code here is well-structured. However, it seems you do not check if the key supplied by the user is correct before decrypting the file. Also, the hmac of the encrypted payload is not verified.

I have implemented them according to the spec. When I have some time, I will submit a pull request with these two features added.

As for Agile, one can verify if the key is correct using "encryptedVerifierHashInput" and "encryptedVerifierHashValue". And verify the hmac of the payload using "encryptedHmacKey" and "encryptedHmacValue". I did not check other cryptos supported by this repo.
@nolze
Copy link
Owner

nolze commented Feb 27, 2020

Thanks! That would be nice.

FYI, key verification has been implemented for some encryption method, e.g.,

msoffcrypto-tool/msoffcrypto/method/ecma376_standard.py

Line 35 in 36d1d0e

def verifykey(key, encryptedVerifier, encryptedVerifierHash):

I think key verification may be skipped by default but can be enforced in the strict mode or the like (or otherwise, enabled by default and skipped with the force option.)

@jeffli678
Copy link
Contributor Author

jeffli678 commented Feb 27, 2020 •
edited

Thanks for letting me know! I did not notice it. I only searched the code of agile crypto and found nothing. It looks they are quite similar. (Update: the idea is similar, but of course the implementation differs)

My intuition is we check the password and verify the integrity by default. Meantime, we allow the decryption to proceed even if the check fails, which is also the default. I will think about it later.

@jeffli678 jeffli678 mentioned this issue Feb 29, 2020
Merged
@nolze nolze linked a pull request Feb 29, 2020 that will close this issue
add "verify_passwd" and "verify_integrity" option #46
Merged
@nolze nolze closed this as completed in #46 Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add "verify_passwd" and "verify_integrity" option jeffli678/msoffcrypto-tool
3 participants
@nolze @jeffli678 and others