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

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

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

nsmethwick-fastly
Copy link
Contributor

@nsmethwick-fastly nsmethwick-fastly commented Jun 18, 2024

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 [email protected] "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
Copy link
Owner

@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
Copy link
Contributor Author

@alexdalitz updated!

@alexdalitz alexdalitz merged commit 4be7085 into alexdalitz:master Jun 20, 2024
4 checks passed
@alexdalitz
Copy link
Owner

@nsmethwick-fastly Merged - thank you!

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

Successfully merging this pull request may close these issues.

2 participants