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

"bad rdlength" error in dynamic update request #150

Closed
freb opened this issue Nov 8, 2014 · 10 comments
Closed

"bad rdlength" error in dynamic update request #150

freb opened this issue Nov 8, 2014 · 10 comments

Comments

@freb
Copy link

freb commented Nov 8, 2014

I'm porting some dynamic DNS code from python to this dns library and I'm getting an error. I have a small server that listens for DNS updates. The code works fine when requesting with nsupdate, but errors when I try with a FortiGate firewall.

The requests are not identical between the two clients but I am not sure why all other systems I've tested against seem to work fine with the FortiGate. The only difference that I can tell is that the FortiGate sends two updates (summary from wireshark):

testhost.dns.com: type A, class ANY
testhost.dns.com: type A, class IN, addr 3.3.3.3

and nsupdate just sends one:

testhost.dns.com: type A, class IN, addr 3.3.3.3

The part of the library that throws the error is in the UnpackRR function in msg.go where it throws "bad rdlength". The first update from the FortiGate has data length of 0 but I've tried deletes from nsupdate as well that have length 0 with no problem.

I dug into the RFC but didn't find anything that would make me think what the FortiGate is trying to do is illegal.

@freb
Copy link
Author

freb commented Nov 8, 2014

After thinking about this some more I realized that the first update from the FortiGate is just a delete request followed by an update/add request. After reproducing this request with nsupdate I get the same "bad rdlength" error which leads me to believe there is a bug. Here is the exact nsupdate command I used:

$ nsupdate -d -y key.:a2V5
> server 127.0.0.1
> zone dns.com
> update delete testhost.dns.com A
> update add testhost.dns.com 60 A 3.3.3.3
> send

It apears to only happen when you specify the record type for the delete as this will produce the error:

update delete testhost.dns.com A

but this will success:

update delete testhost.dns.com ANY

Thanks.

@andrewtj
Copy link
Collaborator

andrewtj commented Nov 9, 2014

While writing a test case for this I've found RemoveRRset() doesn't work as advertised: it removes a specific RR rather than an RRset. I'm out of time to look at this now but should be able to take another look later.

@miekg
Copy link
Owner

miekg commented Nov 9, 2014

Can you post where it breaks?

@andrewtj
Copy link
Collaborator

andrewtj commented Nov 9, 2014

This sample returns bad rdlength:

 []byte{171, 68, 40, 0, 0, 1, 0, 0, 0, 2, 0, 0, 7, 101, 120, 97, 109, 112, 108, 101, 0, 0, 6, 0, 1, 192, 12, 0, 1, 0, 255, 0, 0, 0, 0, 0, 0, 192, 12, 0, 1, 0, 1, 0, 0, 0, 0, 0, 4, 127, 0, 0, 1}

It should be an update message for the zone "example.", deleting the A RRset "example." and then adding an A record at "example.".

@miekg
Copy link
Owner

miekg commented Nov 9, 2014

Ah, thanks. Test added in update_test.go.

I think this is order dependent :( i.e. what RR are before and/or after, which makes it annoying. I may have some time today to look into it.

@miekg
Copy link
Owner

miekg commented Nov 9, 2014

The fundamental problem here is that unpackStructValue has no concept of rdlength. A better longer term fix would be to rewrite this function entirely and maybe drop reflection: basically adapt something as zscan_rr.go... but Meh.

If have a small patch the makes the current testcase at least parse.

@miekg
Copy link
Owner

miekg commented Nov 9, 2014

See 34f43d3

@andrewtj
Copy link
Collaborator

andrewtj commented Nov 9, 2014

go generate might provide some more options for generating pack functions, maybe even the structs themselves.

More examples can be raised with something like:

package main

import "github.com/miekg/dns"

func main() {
    s := &dns.Server{Addr: ":5030", Net: "tcp"}
    dns.HandleFunc(".", func(w dns.ResponseWriter, r *dns.Msg) {
        m := new(dns.Msg)
        m.SetReply(r)
        w.WriteMsg(m)
    })
    s.ListenAndServe()
}
#!/bin/sh
set -e
(
    echo server 127.0.0.1 5030
    echo zone example.
    for i in `seq 1 4`; do
        echo update delete $i.example. TYPE$i
    done
    echo show
    echo send
    echo answer
) | nsupdate -v

@miekg
Copy link
Owner

miekg commented Nov 10, 2014

[ Quoting notifications@github.com in "Re: [dns] "bad rdlength" error in d..." ]

go generate might provide some more options for generating pack functions, maybe even the structs themselves.

Yeah, from the types and tags most stuff can be inferred. Seems like a good
think to do (and speed up the parsing). I won't have a lot of time for this
though :(

More examples can be raised with something like:

package main

import "github.com/miekg/dns"

func main() {
  s := &dns.Server{Addr: ":5030", Net: "tcp"}
  dns.HandleFunc(".", func(w dns.ResponseWriter, r *dns.Msg) {
      m := new(dns.Msg)
      m.SetReply(r)
      w.WriteMsg(m)
  })
  s.ListenAndServe()
}
#!/bin/sh
set -e
(
   echo server 127.0.0.1 5030
   echo zone example.
   for i in `seq 1 4`; do
       echo update delete $i.example. TYPE$i
   done
   echo show
   echo send
   echo answer
) | nsupdate -v

Hmm. These should all be relatively easy to fix. I'll give it a whack.

#150 (comment)
/Miek

Miek Gieben

@andrewtj
Copy link
Collaborator

andrewtj commented Aug 1, 2015

I've tried to provoke this behaviour again and been unable to—call it fixed?

@miekg miekg closed this as completed Aug 1, 2015
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

No branches or pull requests

3 participants