Skip to content

fix: [tools] Validation of mac-eui-64, mac-address and SHA256 ssh-fingerprint values - #1504

Open
bacescuandrei wants to merge 1 commit into
MISP:mainfrom
bacescuandrei:fix/avt-strict-validity
Open

bacescuandrei wants to merge 1 commit into
MISP:mainfrom
bacescuandrei:fix/avt-strict-validity

Conversation

@bacescuandrei

Copy link
Copy Markdown

These are differences from MISP's app/Lib/Tools/AttributeValidationTool.php:

  • mac-eui-64 values never validated. modifyBeforeValidation kept only the first 6 octets, and MAC_EUI_64_RE had an extra : after ff:fe. MISP uses ^([a-f0-9]{2}:){3}ff:fe(:[a-f0-9]{2}){3}$ and keeps all octets.
  • Long mac-address values were silently truncated. The same normalisation cut mac-address values with more than 6 octets down to 6, so invalid values were accepted and altered. MISP rejects them.
  • Unpadded SHA256 fingerprints were rejected. SHA256: SSH fingerprints, as printed by OpenSSH, have no base64 padding, and b64decode rejected them. They are now padded and decoded strictly, as MISP's strict base64_decode does. Whitespace inside the fingerprint is ignored, as in MISP. Non-base64 characters, which b64decode used to discard silently, are now rejected, as in MISP.

These problems were not caught because _should_be_valid used assertTrue, which also passes for the error message returned by validate. The test helper now checks assertIs(..., True). That stricter check showed that two existing test values were not valid (AS '1.1' before normalisation, and a 60-character onion address), so they are corrected.

tests/test_attributevalidationtool.py: 31 passed. Without the change, 3 tests fail (mac, mac-eui-64 in test_validate_networking, ssh-fingerprint). mypy is clean on the changed file.

…gerprint values

mac-eui-64 values were truncated to 6 octets and checked with a wrong
regex, so they never validated; mac-address values longer than 6 octets
were truncated and accepted; SHA256 ssh-fingerprints in the unpadded
form printed by OpenSSH were rejected. Whitespace in SHA256 fingerprints
is ignored, as in MISP. The tests now require validate() to return True
instead of any truthy value.

This branch has not been deployed

No deployments
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.

1 participant