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

DNS normalizer fails when option is present #2805

Open
dekkers opened this issue Apr 8, 2024 · 2 comments
Open

DNS normalizer fails when option is present #2805

dekkers opened this issue Apr 8, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@dekkers
Copy link
Contributor

dekkers commented Apr 8, 2024

The DNS normalizer fails when an option is present. For example the following DNS data:

RESOLVER: 1.1.1.1
id 24462
opcode QUERY
rcode NOERROR
flags QR RD RA
edns 0
payload 1232
option Generic 15
;QUESTION
example.com. IN SOA
;ANSWER
example.com.

The problem seems to be that dnspython to_text generates the option Generic 15, but the from_text method isn't able to parse option.

Traceback (most recent call last):
  File "/app/boefjes/boefjes/app.py", line 247, in _start_working
    handler.handle(p_item.data)
  File "/app/boefjes/boefjes/job_handler.py", line 198, in handle
    results = self.job_runner.run(normalizer_meta, raw)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/local.py", line 76, in run
    return self._parse_results(normalizer_meta, results)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/local.py", line 79, in _parse_results
    parsed: list[NormalizerResult] = [self._parse(result) for result in results]
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/local.py", line 79, in <listcomp>
    parsed: list[NormalizerResult] = [self._parse(result) for result in results]
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/plugins/kat_dns/normalize.py", line 50, in run
    responses.append(from_text("\n".join(lines[1:])))
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/dns/message.py", line 1352, in from_text
    return reader.read()
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/dns/message.py", line 1308, in read
    line_method(section_number)
  File "/usr/local/lib/python3.11/site-packages/dns/message.py", line 1159, in _header_line
    raise UnknownHeaderField
dns.message.UnknownHeaderField: The header field name was not recognized when converting from text
    into a message.

As far as I can see there are two ways to solve this:

  1. Add support for parsing option to dnspython from_text
  2. Change our raw files to use the format used by to_wire and from_wire. This saves the queries in the binary DNS protocol wire format. We can use prepend_length to save multiple queries/answers to a single file.
    If we change to the new format, we should probably create a new raw file type and keep the old normalizer so we can still parse old raw files.
@dekkers dekkers added the bug Something isn't working label Apr 8, 2024
@dekkers dekkers self-assigned this Apr 8, 2024
@dekkers dekkers added this to KAT Apr 8, 2024
@github-project-automation github-project-automation bot moved this to Incoming features / Need assessment in KAT Apr 8, 2024
@dekkers dekkers moved this from Incoming features / Need assessment to To be discussed in KAT Apr 8, 2024
@dekkers
Copy link
Contributor Author

dekkers commented Apr 18, 2024

Discussion conclusion: Try out if to_wire and from_wire doesn't result in any new problems.

@dekkers dekkers moved this from To be discussed to Todo (In this Sprint) in KAT Apr 18, 2024
@dekkers dekkers mentioned this issue Aug 22, 2024
9 tasks
@underdarknl
Copy link
Contributor

I agree, we should just move to the Wire protocol for this. it's better fitting to the OpenKAT philosophy and might fix these issues, as dnspython clearly Can parse these records from the underlying wire data directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog / To do
Development

No branches or pull requests

2 participants