hannob / snallygaster

Tool to scan for secret files on HTTP servers
BSD Zero Clause License
2.07k stars 228 forks source link

dns.resolver.query() causes deprecation warning when using dnspython >=2.0.0 #54

Closed mgollo closed 4 years ago

mgollo commented 4 years ago

In dnspython 2.0.0, dns.resolver.query() has been deprecated, dns.resolver.resolve() should be used instead. Therefore, with dnspython >=2.0.0, snallygaster causes deprecation warnings. However, using dns.resolver.resolve() would make the tool incompatible with dnspython <2.0.0. If that is not an issue, the following path would remove the warnings:

diff --git a/snallygaster b/snallygaster
index 8f32ff6..2dcdb3c 100755
--- a/snallygaster
+++ b/snallygaster
@@ -215,7 +215,7 @@ def dnscache(qhost):
     except OSError:
         pass
     try:
-        dnsanswer = dns.resolver.query(qhost, 'A')
+        dnsanswer = dns.resolver.resolve(qhost, 'A')
     except (dns.exception.DNSException, ConnectionResetError):
         dns_cache[qhost] = None
         return None
@@ -738,7 +738,7 @@ def test_wpdebug(url):
 @HOSTNAME
 def test_axfr(qhost):
     try:
-        ns = dns.resolver.query(qhost, 'NS')
+        ns = dns.resolver.resolve(qhost, 'NS')
     except (dns.exception.DNSException, ConnectionResetError):
         return
     for r in ns.rrset:

If you want to apply this, I can create a PR.

hannob commented 4 years ago

Meh, why do people do this? If they deprecate something they should wait for a while til the replacement is widely deployed.

I think we'll have to do some magic to let it work with both versions. My "compatibility requirement" is usually "needs to work with what's shipped in debian stable". Which likely means we'll have to deal with this for a while.

hannob commented 4 years ago

What do you think about something like:

if 'resolve' in dir(dns.resolver):
    ns = dns.resolver.resolve(qhost, 'NS')
else:
    ns = dns.resolver.query(qhost, 'NS')

? And a big comment that once dnspython 2.0 or above is widely deployed this should be simplified again.

mgollo commented 4 years ago

I agree that this kind of hard deprecations is not really the nicest way. Your suggestion is probably the only way to get rid of the warnings and still remain compatible with older versions.

hannob commented 4 years ago

Just to quickly record this here: It seems to me no matter what I do right now, the axfr test fails with dnspython 2.0. It may be the axfr functionality has some subtle changes as well. Have to look more into this.

About the other use of dnspython which is only called from the invalidsrc test: I'm not entirely sure if we need this at all, I don't remember my intentions when I implemented this, but it looks like it's just a fallback if the normal socket function does not work.

hannob commented 4 years ago

Ok, I'm not happy with the amount of changes required to make dnspython 2.0 happy, but here it is: https://github.com/hannob/snallygaster/commit/4a02f0fe26dd94513984926d60541101cef80afd

It seems it no longer likes to do axfr queries on a hostname, so we need to first resolve IPs. I changed it to use multiple IPs on NS and also do v4/v6, so this also makes the test more thorough.

Can you please test?

mgollo commented 4 years ago

I did some quick tests and didn't get any warnings or errors/exceptions anymore.

hannob commented 4 years ago

Ok, will consider this as fixed. Obviously if you see any other issues please open another bug.