search

alexdalitz / dnsruby

Dnsruby is a feature-complete DNS(SEC) client for Ruby, as used by many of the world's largest DNS registries and the OpenDNSSEC project
Other
194 stars 77 forks source link

Raises ArgumentError when trying to parse an invalid CAA record #194

Closed nsmethwick-fastly closed 2 weeks ago

nsmethwick-fastly commented 3 weeks ago

When a CAA record is incorrectly formatted dnsruby throws a NoMethod error.

kdig +bufsize=1280 @8.8.8.8 -t CAA buda.com
;; ->>HEADER<<- opcode: QUERY; status: NOERROR; id: 44583
;; Flags: qr rd ra ad; QUERY: 1; ANSWER: 15; AUTHORITY: 0; ADDITIONAL: 1
;; EDNS PSEUDOSECTION:
;; Version: 0; flags: ; UDP size: 512 B; ext-rcode: NOERROR
;; QUESTION SECTION:
;; buda.com.                IN  CAA
;; ANSWER SECTION:
buda.com.               451 IN  CAA 0 comodoca.com "issue"
buda.com.               451 IN  CAA 0 comodoca.com "issuewild"
buda.com.               451 IN  CAA 0 digicert.com "issue"
buda.com.               451 IN  CAA 0 digicert.com "issuewild"
buda.com.               451 IN  CAA 0 issue "comodoca.com"
buda.com.               451 IN  CAA 0 issue "digicert.com; cansignhttpexchanges=yes"
buda.com.               451 IN  CAA 0 issue "letsencrypt.org"
buda.com.               451 IN  CAA 0 issue "pki.goog; cansignhttpexchanges=yes"
buda.com.               451 IN  CAA 0 issuewild "comodoca.com"
buda.com.               451 IN  CAA 0 issuewild "digicert.com; cansignhttpexchanges=yes"
buda.com.               451 IN  CAA 0 issuewild "letsencrypt.org"
buda.com.               451 IN  CAA 0 issuewild "pki.goog; cansignhttpexchanges=yes"
buda.com.               451 IN  CAA 0 letsencrypt.com "issue"
buda.com.               451 IN  CAA 0 letsencrypt.com "issuewild"
buda.com.               451 IN  CAA 0 platform@buda.com "iodef"

It does look like a thread error that appears to be related seems to get raised. I am not sure if anything additional needs to be added to this to handle that error but I am happy to provide more information if needed:

resolver.query("buda.com", "CAA")

<Thread:0x0000000116149828 /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:69 run> terminated with exception (report_on_exception is true):
/Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:625:in `block in send_exception_to_client': undefined method `client_queue' for nil:NilClass (NoMethodError)
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:624:in `synchronize'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:624:in `send_exception_to_client'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:569:in `rescue in get_incoming_data'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:513:in `get_incoming_data'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:280:in `block in process_ready'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:275:in `each'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:275:in `process_ready'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:219:in `do_select'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:69:in `block (2 levels) in initialize'
/Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/resource/CAA.rb:47:in `from_string': undefined method `[]' for nil:NilClass (NoMethodError)
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/resource/RR.rb:113:in `initialize'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/resource/CAA.rb:68:in `new'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/resource/CAA.rb:68:in `decode_rdata'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/decoder.rb:171:in `block in get_rr'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/decoder.rb:53:in `get_length16'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/decoder.rb:171:in `get_rr'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/message.rb:574:in `block (2 levels) in decode'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/message.rb:573:in `times'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/message.rb:573:in `block in decode'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/decoder.rb:20:in `initialize'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/message.rb:567:in `new'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/message/message.rb:567:in `decode'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:514:in `get_incoming_data'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:280:in `block in process_ready'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:275:in `each'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:275:in `process_ready'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:219:in `do_select'
    from /Users/nsmethwick/.gem/ruby/3.0.5/gems/dnsruby-1.72.1/lib/dnsruby/select_thread.rb:69:in `block (2 levels) in initialize'

Let me know if I did this right or if you'd prefer to handle this in a different way.

alexdalitz commented 2 weeks ago

@nsmethwick-fastly Thank you very much for this! It looks good - the only thing I would ask you, please, to change would be to raise a Dnsruby DecodeError rather than an ArgumentError, to keep it in line with the rest of the RR parsing errors. Thanks!

nsmethwick-fastly commented 2 weeks ago

@alexdalitz updated!

alexdalitz commented 2 weeks ago

@nsmethwick-fastly Merged - thank you!