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

Tag/label data is not being added to DNSTAP output #516

Closed
johnhtodd opened this issue Dec 17, 2023 · 9 comments
Closed

Tag/label data is not being added to DNSTAP output #516

johnhtodd opened this issue Dec 17, 2023 · 9 comments

Comments

@johnhtodd
Copy link

johnhtodd commented Dec 17, 2023

Describe the bug
The tagging information added to other outputs such as JSON file are not being added to DNSTAP relayed messages, or are not recognized. I'm not sure if this is a bug or just if it would be described as "a not implemented expected feature".

To Reproduce
Use configuration below to arbitrarily add tags to messages, and then relay them via DNSTAP to another go-dnscollector instance to validate that they are not being included in the messages.

Expected behavior
I would expect go-dnscollector to send and then recognize its own tagging data in DNSTAP streams, and be able to parse that data as a receiver. This would be for both customized tagging ("atags:") as well as any other fields that go-dnscollector adds during processing, such as "geoip:" and all the other fields that are dynamically or statically created by transformations.

Additional context
Running most recent version, with this simplified configuration (below.) The output file shows the tag "foo:DMARC" as being added in the JSON summaries. I ran another go-dnscollector instance on port 59311 to receive the relayed DNSTAP stream with a very simple single output to JSON, and the tags did not appear either as recognized fields or as data in the "extra:" field. I am fairly certain they are not being sent.

pipelines:

  - name: dnsdist-from-outside
    dnstap:
      listen-ip: 0.0.0.0
      listen-port: 59310
      chan-buffer-size: 131070
    routes: [ split-test ]

  - name: godnsconnector-next
    dnstap:
      remote-address: 127.0.0.1
      remote-port: 59311

  - name: split-test
    dnsmessage:
      matching:
        include:
          dns.qtype: "TXT"
          dns.qname: "^*.dmarc.*$"
      policy: "drop-unmatched"
    transforms:
      atags:
        tags: [ "foo:DMARC" ]
    routes: [ outputfile,godnsconnector-next ]

  - name: outputfile
    logfile:
      file-path:  "/tmp/dnstap.log"
      max-size: 1000
      max-files: 10
      mode: json
@dmachard
Copy link
Owner

It shouldn't be difficult to add.

@johnhtodd
Copy link
Author

Good to hear! Eventually I am hoping that we can add tag data to DNSTAP outputs from dnsdist, and have that data recognized by go-dnscollector for performing matching within go-dnscollector. We already do some tagging in dnsdist, but we can re-structure the format to meet whatever method ends up being implemented by go-dnscollector. I suppose it should also be the case that tag data that is not understood should be transmitted un-modified through the processing stack unless there is some specific syntax that says "drop unparseable tag data in extra field" on the ingestion into go-dnscollector.

@johnhtodd
Copy link
Author

This does raise the issue of how to encode this data, especially when there may be existing data already in the "extra" field from the original DNSTAP producer. Is this a process of making a giant text (json?) blob of the various other fields and inserting it into the extra field? Or is it best to have a new protobuf structure that can understand and incorporate all possible additional fields as they are defined by go-dnscollector over time? This method (new protobuf definition) seems fragile, but protobuf should be able to handle unknown fields if there is version mismatch, though it creates a condition where other DNSTAP consumers would not be able to parse the data - this would become a dnscollector-to-dnscollector only method (which may be OK!) I don't have a very strong opinion on it since I haven't thought about it enough I suppose.

@johnhtodd
Copy link
Author

This has become a blocker for us, specifically, the minimal behavior of passing the "extra" field is necessary even if only as an extremely basic relay function.

The "dnstap-relay" source identifier doesn't seem to work in the most recent versions, either. (I tried this as a workaround.) In fact, I tried using the example in https://github.com/dmachard/go-dnscollector/blob/main/docs/_examples/use-case-12.yml but that doesn't apparently work any more - it fails with "config error: unknown YAML key dnstap-relay in configuration"

I see that the extra field can be minimally ingested by go-dnscollector, but it cannot be transmitted in any dnstap format that I can observe.

@johnhtodd
Copy link
Author

Tested and seems to work, at least in the concept of purely relaying the data using "dnstap-relay"

@dmachard
Copy link
Owner

Thanks to report the issue for the dnstap-relay, it's fixed.
Also the DNSTap extra field is now properly sent with the DNStap logger if it exists in original message.

@dmachard
Copy link
Owner

This does raise the issue of how to encode this data, especially when there may be existing data already in the "extra" field from the original DNSTAP producer. Is this a process of making a giant text (json?) blob of the various other fields and inserting it into the extra field? Or is it best to have a new protobuf structure that can understand and incorporate all possible additional fields as they are defined by go-dnscollector over time? This method (new protobuf definition) seems fragile, but protobuf should be able to handle unknown fields if there is version mismatch, though it creates a condition where other DNSTAP consumers would not be able to parse the data - this would become a dnscollector-to-dnscollector only method (which may be OK!) I don't have a very strong opinion on it since I haven't thought about it enough I suppose.

I am working on a new dedicated protobuf structure which will be stored in the DNStap extra field.

@dmachard dmachard mentioned this issue Jan 17, 2024
4 tasks
@dmachard dmachard added this to the v0.40.0 milestone Jan 17, 2024
@dmachard
Copy link
Owner

The pull request #548 is in progress

@dmachard
Copy link
Owner

feature request completed via #548

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants