6 ms·

this is hilarious

jopsen 6y ago
Makes perfect sense. Defense in depth, if you can grep the source code and not find any references to md5, then you have quickly verified that the code probably doesn't use md5. This you can easily verify again later, you can even make a test for it :) Even if in practice this had no impact, removing md5 usage, will make it harder to accidentally introduce it in the future.
- asddubs 6y ago
  yeah I can see that, what if someone ends up being smart and re-using the verification procedure for a file that does have security impact, DRY right?
- the8472 6y ago
  The issue is not md5. The issue one wants to detect is weak hash functions used in cases where they're not appropriate. The fact that crc32 passed means that any obscure hash function would have passed too, even if it had been used in a context were it isn't appropriate. All it means that the audit is superficial and doesn't catch the error category, just famous examples within that category. That kind of superficial sanning may be worth something when unleashed on security-naive developers or even as optional input for more experienced ones. But "hard compliance rules" and "superficial scans" combine to create a lot of busywork which makes people less motivated to work with auditors instead of against them.
  - meshaneian 6y ago
    Next up: Replace MD5 with BASE64+ROT13. Significantly worse functionality AND performance, but sounds more secure (to a layman) and doesn't trigger the "MD5" alert...
    - cortesoft 6y ago
      You joke, but an ex-security guy at my company literally told me “this file can’t be in plain text on disk. Base64 encode it”
      - skissane 6y ago
        Base64 encoding does protect somewhat against "looking over your shoulder" attacks (Unless the person looking over your shoulder has a really good memory and can remember the Base64, or decode it in their head. Or they have a camera.)
      - mschuster91 6y ago
        Helps against attackers grepping the whole disk (or any folder named "conf" or similar) for "username", "user", "password", "pass", "key" and friends. It's game over anyway if someone has a shell on your server but at least it complicates their life a bit.
  - theon144 6y ago
    Both perspectives are somewhat correct, I feel; the requirement to remove any usage of md5 is beneficial, but the fact that crc32 passed means the audit shows the motivation was misplaced. The resulting situation might of course not be a net benefit though :/
  - dx034 6y ago
    But way more people would use md5 for password hashing than crc32. Of course someone could circumvent these tests, but the risk of someone copying an old tutorial where md5 is used for password hashing can be mitigated.
  - jopsen 6y ago
    > But "hard compliance rules" and "superficial scans" combine to create a lot of busywork which makes people less motivated to work with auditors instead of against them. Absolutely :) The fact is that if you have experienced engineers a security audit is rarely able to find anything. You would basically have to do code reviews, and this is hard / expensive, and even then rarely fruitful. So, superficial scans, hardening, checking for obvious mistakes is really all you can do. Making hard rules is unproductive, but then again, migrating from md5 to crc32 hopefully isn't very expensive. IMO, crc32 is a better choice for testing for changes, and has the benefit of removing any doubt that the hash has any security properties.
- therein 6y ago
  // We use MD5 to check if config files are changed. This is not used anywhere else. typedef DigestMD5 ConfigFileHasher;
  - dyingkneepad 6y ago
    Until someone repurposes that thing to do something that is security-sensitive and forgets to remove the comment, misleading the next auditors. I always assume that people from the future who are going to touch my code are really dumb people, so I try to have as few traps as possible for them.
    - asddubs 6y ago
      i know for a fact the person who's gonna touch my code in the future is really dumb, because it's me
- deleted 6y ago
  [deleted]
Macha 6y ago
I've seen similar rigidity from security audits. Stuff like "version 10.5.2 (released last week) of this software introduced a security bug that was fixed in 11.0 (released today), we need you to update from 10.5.1 (released last week + 1 day) to 11.0 now because our audit tool says so".
- gorkish 6y ago
  Ah yes and also the vendor helpfully changed the API and did a complete rewrite in v11.0. Think about all the neat new things you will get to learn!

this is hilarious

Makes perfect sense. Defense in depth, if you can grep the source code and not find any references to md5, then you have quickly verified that the code probably doesn't use md5. This you can easily verify again later, you can even make a test for it :) Even if in practice this had no impact, removing md5 usage, will make it harder to accidentally introduce it in the future.

asddubs 6y ago
yeah I can see that, what if someone ends up being smart and re-using the verification procedure for a file that does have security impact, DRY right?
the8472 6y ago
The issue is not md5. The issue one wants to detect is weak hash functions used in cases where they're not appropriate. The fact that crc32 passed means that any obscure hash function would have passed too, even if it had been used in a context were it isn't appropriate. All it means that the audit is superficial and doesn't catch the error category, just famous examples within that category. That kind of superficial sanning may be worth something when unleashed on security-naive developers or even as optional input for more experienced ones. But "hard compliance rules" and "superficial scans" combine to create a lot of busywork which makes people less motivated to work with auditors instead of against them.
- meshaneian 6y ago
  Next up: Replace MD5 with BASE64+ROT13. Significantly worse functionality AND performance, but sounds more secure (to a layman) and doesn't trigger the "MD5" alert...
  - cortesoft 6y ago
    You joke, but an ex-security guy at my company literally told me “this file can’t be in plain text on disk. Base64 encode it”
    - skissane 6y ago
      Base64 encoding does protect somewhat against "looking over your shoulder" attacks (Unless the person looking over your shoulder has a really good memory and can remember the Base64, or decode it in their head. Or they have a camera.)
    - mschuster91 6y ago
      Helps against attackers grepping the whole disk (or any folder named "conf" or similar) for "username", "user", "password", "pass", "key" and friends. It's game over anyway if someone has a shell on your server but at least it complicates their life a bit.
- theon144 6y ago
  Both perspectives are somewhat correct, I feel; the requirement to remove any usage of md5 is beneficial, but the fact that crc32 passed means the audit shows the motivation was misplaced. The resulting situation might of course not be a net benefit though :/
- dx034 6y ago
  But way more people would use md5 for password hashing than crc32. Of course someone could circumvent these tests, but the risk of someone copying an old tutorial where md5 is used for password hashing can be mitigated.
- jopsen 6y ago
  > But "hard compliance rules" and "superficial scans" combine to create a lot of busywork which makes people less motivated to work with auditors instead of against them. Absolutely :) The fact is that if you have experienced engineers a security audit is rarely able to find anything. You would basically have to do code reviews, and this is hard / expensive, and even then rarely fruitful. So, superficial scans, hardening, checking for obvious mistakes is really all you can do. Making hard rules is unproductive, but then again, migrating from md5 to crc32 hopefully isn't very expensive. IMO, crc32 is a better choice for testing for changes, and has the benefit of removing any doubt that the hash has any security properties.
therein 6y ago
// We use MD5 to check if config files are changed. This is not used anywhere else. typedef DigestMD5 ConfigFileHasher;
- dyingkneepad 6y ago
  Until someone repurposes that thing to do something that is security-sensitive and forgets to remove the comment, misleading the next auditors. I always assume that people from the future who are going to touch my code are really dumb people, so I try to have as few traps as possible for them.
  - asddubs 6y ago
    i know for a fact the person who's gonna touch my code in the future is really dumb, because it's me
deleted 6y ago
[deleted]

Macha 6y ago

I've seen similar rigidity from security audits. Stuff like "version 10.5.2 (released last week) of this software introduced a security bug that was fixed in 11.0 (released today), we need you to update from 10.5.1 (released last week + 1 day) to 11.0 now because our audit tool says so".

gorkish 6y ago
Ah yes and also the vendor helpfully changed the API and did a complete rewrite in v11.0. Think about all the neat new things you will get to learn!