wrong reverse dns lookups in shadowserver reports cause a crash of the FQDN-validation

[bernhardreiter]

Right now, if a reverse dns returns something bogus, there is a crash and dump
in sanitize()

Happened with shadowserver opendns resolver, and the hostname field was something like
.example.net

 'source_queue': 'shadowserver-parser-dns-open-resolvers-queue',
 'traceback': ['Traceback (most recent call last):',
               '  File "/usr/lib/python3.4/encodings/idna.py", line 165, in '
               'encode',
               '    raise UnicodeError("label empty or too long")',
               'UnicodeError: label empty or too long',
               '',
               'The above exception was the direct cause of the following '
               'exception:',
               '',
               'Traceback (most recent call last):',
               '  File "/usr/lib/python3/dist-packages/intelmq/lib/bot.py", '
               'line 606, in process',
               '    events = list(filter(bool, self.parse_line(line, 
report)))',
               '  File '
               '"/usr/lib/python3/dist-packages/intelmq/bots/parsers/shadowserver/parser.py", '
               'line 150, in parse_line',
               '    event.add(intelmqkey, value)',
               '  File '
               '"/usr/lib/python3/dist-packages/intelmq/lib/message.py", 
line '
               '202, in add',
               '    value = self.__sanitize_value(key, value)',
               '  File '
               '"/usr/lib/python3/dist-packages/intelmq/lib/message.py", 
line '
               '298, in __sanitize_value',
               '    return class_reference().sanitize(value)',
               '  File '
               '"/usr/lib/python3/dist-packages/intelmq/lib/harmonization.py", '
               'line 374, in sanitize',
               "    return value.encode('idna').decode().lower()",
               "UnicodeError: encoding with 'idna' codec failed 
(UnicodeError: "
               'label empty or too long)']}

[dmth]

harmonization.py does this, to validate "FQDN" -Data. As you can see, leading dots are not stripped.

    @staticmethod
    def sanitize(value):
        value = value.rstrip('.')
        if value:
            return value.encode('idna').decode().lower()

Minimalistic example to reproduce the error:

>>> badurl = ".example.com"
>>> goodurl = "example.com"

>>> goodurl.encode("idna")
b'example.com'

>>> badurl.encode("idna")
Traceback (most recent call last):
  File "/usr/lib/python3.4/encodings/idna.py", line 165, in encode
    raise UnicodeError("label empty or too long")
UnicodeError: label empty or too long

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
UnicodeError: encoding with 'idna' codec failed (UnicodeError: label empty or too long)

The data in the report is simply wrong. But how to deal with this?

Reject the dataset (the call of is_valid()) should fail now)?

    @staticmethod
    def sanitize(value):
        value = value.rstrip('.')
        if value[0] == ".":
            return False
        if value:
            return value.encode('idna').decode().lower()

Or alter and sanitize the dataset?

    @staticmethod
    def sanitize(value):
        value = value.rstrip('.')
        value = value.lstrip('.')
        if value:
            return value.encode('idna').decode().lower()

Is an indication of this alteration necessary?

[dmth]

This issue shall only focus on a solution for the Shadowserver-Parser, for instance by creating a validatior in config.py

A universal solution for this problem is pursued in #1030

[ghost]

IMHO we can strip the leading dot always. See also #369 for the discussion on trailing dot

Ping @aaronkaplan

EDIT: s/zero/dot/ :/

[bernhardreiter]

@wagner-certat you are refering to the leading dot (.)?

[ghost]

@bernhardreiter yes :)

[dmth]

We are going to work on that issue, first as validator in the shadowserver-config.
If we create a PR for that , we'll create it against the master branch, as it is a bugfix.

[ghost]

That issue is independent of the source/parser. So IMHO it should go to the sanitation of the harmonization. So if you are keen on fixing this only for shadowserver, that specific fix will be replaced by a generic fix very soon. I'll push a commit fixing this to both branches of course.

[dmth]

I'll push a commit fixing this to both branches of course.

Great I'm looking forward to it.