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

Investigate and change the use of MD2, MD4, MD5, or SHA1 hash functions in wodles #10116

Closed
Tracked by #2330
mcarmona99 opened this issue Sep 14, 2021 · 2 comments
Closed
Tracked by #2330
Assignees
Labels
type/enhancement New feature or request type/test

Comments

@mcarmona99
Copy link
Contributor

mcarmona99 commented Sep 14, 2021

Description

With the test created in the issue wazuh/wazuh-qa#1615, some possible code flaws were found by Bandit.

In this issue we specify flaws regarding the use of insecure hash functions.

Investigate if it is possible to use SHA256 or a different secure cryptograpthic algorithm.
Algorithms which are not considered secure with alternatives: https://security.stackexchange.com/questions/78/what-cryptographic-algorithms-are-not-considered-secure

This is not a problem since the data handled are not critical and are only used internally by the wodle, so there is no decoding possibility. Despite this, we could investigate and use secure hash functions, similar to the same possible flaws found in framework.

Vulnerabilities found in:

More info.: https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html#b303-md5

Once changes are done, pass the test to check that these flaws were deleted from the known flaws JSON file of framework.

@noise-kngdm
Copy link
Contributor

noise-kngdm commented Oct 13, 2021

Issue update

We've undergone an investigation to determine if there are any security issues related to the use of the MD5 hashing algorithm in the Azure module. As a summary, we've concluded that there are no problems with our use of the algorithm, an extended investigation report is below.

Hashing algorithms best practices

It is known that MD5 is a hashing algorithm that's cryptographically broken since 2004, as stated in this article, and some of its weaknesses had been described in the late 90s. Despite that, it's a widely used algorithm for integrity checking when there's no third party that could tamper with the file, and any changes would be network-related. It's still in use because it's faster than SHA2 and SHA3 algorithms and it's vastly supported.

Better hashing algorithm options

Although there is no problem with the use of MD5 in some cases, we should stop using it in favor of hashing algorithms like BLAKE2, which is at least as fast, is cryptographically secure, and can have the same digest sizes -more info here-. It's available for use in the hashlib library too, where you can choose between BLAKE2b, optimized for 64-bits platforms, and BLAKE2s, optimized for 32-bits platforms.
According to this paper,

Each one is portable to any CPU, but can be up twice as fast when used on the CPU size for which it is optimized

Given this information, it would be pretty interesting to use BLAKE2 as an MD5 replacement in most cases, or at least in every new development that needs a fast hashing algorithm.

Is that a vulnerability?

In the Azure module, the MD5 algorithm is being used to hash the Log Analytics URL provided by the user, to then save that alongside the date of the last log fetched from Azure in a .json file. The digest is then being used as a sort of primary key of a file that emulates a database. Since no user can tamper with the URL apart from the Wazuh administrator, there is no problem in using the MD5 digest, and in fact, there is no use in hashing the URL, using the raw URL would accomplish the same goal too.

Since there is no risk in the use of the MD5 hashing algorithm, and changing the algorithm to another one would imply some migration efforts, we propose not to change it. In the future, it would be interesting to use a well-designed SQLite database to address this storing necessity in a more efficient and standardized way.

@noise-kngdm
Copy link
Contributor

Issue update

We opened wazuh/wazuh-qa#2338 to mark the security flaw reported by this issue as a false positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request type/test
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants