MiniDNS / minidns

DNS library for Android and Java SE
Other
215 stars 61 forks source link

Getting DnssecValidationFailedException for non existing domain #121

Open rekire opened 2 years ago

rekire commented 2 years ago

I'm playing with this lib and got an unexpected error in my test code. I get an DnssecValidationFailedException for non existing domain "github.xxx".

Here is my test code:

class DnsTest {
    enum class AddressStatus {
        valid,
        notRegistered,
        noMxRecord,
        unknown
    }

    @Test
    fun evaluating() {
        val cases = mapOf(
            "github.com" to AddressStatus.valid,
            "github.io" to AddressStatus.noMxRecord,
            "github.xxx" to AddressStatus.notRegistered,
        )
        cases.forEach { (domain, expected) ->
            val result = checkDomain(domain)
            assertEquals("Unexpected result for $domain", expected, result)
        }
    }

    private fun checkDomain(domain: String) : AddressStatus {
        return try {
            val result = DnssecResolverApi.INSTANCE.resolve<MX>(Question(domain, Record.TYPE.MX))
            when {
                result.wasSuccessful() && result.answersOrEmptySet.isEmpty() -> AddressStatus.noMxRecord
                result.wasSuccessful() && result.answersOrEmptySet.isNotEmpty() -> AddressStatus.valid
                result.responseCode == RESPONSE_CODE.NX_DOMAIN -> AddressStatus.notRegistered
                else -> AddressStatus.unknown
            }
        } catch (err: DnssecValidationFailedException) {
            println("Error: ${err.message}")
            AddressStatus.unknown
        } catch (t: IOException) {
            AddressStatus.unknown
        }
    }
}

I get the exception: Unexpected result for github.xxx expected:<notRegistered> but was:<unknown>

When I run dig github.xxx +dnssec I get the expected NXDOMAIN status:

; <<>> DiG 9.10.6 <<>> github.xxx +dnssec
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 51158
;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 8, ADDITIONAL: 1
Flowdalic commented 2 years ago

It's hard to tell much without more information. As general rule of thumb, always log full exception, including its stacktrace. Then present that in bug reports. Right now, I don not even now which return site is returning AddressStatus.unknown.

rekire commented 2 years ago

I tried to keep it short, but you are right again. I skipped the Error: log statement above. So the DnssecValidationFailedException is thown as implied by the title.

[removed non relevant parts]

And here is the relevant third execution with github.xxx:

Error: Validation of 1 NSEC3 record failed: Signature is invalid.

Unexpected result for github.xxx expected:<notRegistered> but was:<unknown>
Expected :notRegistered
Actual   :unknown
<Click to see difference>

java.lang.AssertionError: Unexpected result for github.xxx expected:<notRegistered> but was:<unknown>
    at org.junit.Assert.fail(Assert.java:89)
    at org.junit.Assert.failNotEquals(Assert.java:835)
    at org.junit.Assert.assertEquals(Assert.java:120)
    at eu.rekisoft.android.editmail.DnsTest.evaluating(DnsTest.kt:35)
Flowdalic commented 2 years ago

Thanks, feel free to reduce the noise by manually filtering out the irrelevant exceptions. Proper bug reports where only the relevant information is shown increase the chances that someone looks at them.

java.lang.AssertionError: Unexpected result for github.xxx expected: but was: at org.junit.Assert.fail(Assert.java:89) at org.junit.Assert.failNotEquals(Assert.java:835) at org.junit.Assert.assertEquals(Assert.java:120) at eu.rekisoft.android.editmail.DnsTest.evaluating(DnsTest.kt:35)

This appears to only show the junit AssertionError. If there is a underlying excpetion then I don't see it. And since I also don't know whats at DnsTest.kt:35 I am stuck again helping here.

rekire commented 2 years ago

I added a err.printStackTrace():

private fun checkDomain(domain: String) : AddressStatus { // domain is here github.xxx
    return try {
        val result = DnssecResolverApi.INSTANCE.resolve<MX>(Question(domain, Record.TYPE.MX)) // line 36
        when {
            result.wasSuccessful() && result.answersOrEmptySet.isEmpty() -> AddressStatus.noMxRecord
            result.wasSuccessful() && result.answersOrEmptySet.isNotEmpty() -> AddressStatus.valid
            result.responseCode == RESPONSE_CODE.NX_DOMAIN -> AddressStatus.notRegistered
            else -> AddressStatus.unknown
        }
    } catch (err: DnssecValidationFailedException) {
        println("Error: ${err.message}")
        err.printStackTrace() // the new line
        AddressStatus.unknown
    } catch (t: IOException) {
        AddressStatus.unknown
    }
}

Here is now the relevant part for you:

Error: Validation of 1 NSEC3 record failed: Signature is invalid.
org.minidns.dnssec.DnssecValidationFailedException: Validation of 1 NSEC3 record failed: Signature is invalid.
    at org.minidns.dnssec.Verifier.verify(Verifier.java:76)
    at org.minidns.dnssec.DnssecClient.verifySignedRecords(DnssecClient.java:396)
    at org.minidns.dnssec.DnssecClient.verifySignatures(DnssecClient.java:321)
    at org.minidns.dnssec.DnssecClient.verifyNsec(DnssecClient.java:260)
    at org.minidns.dnssec.DnssecClient.verify(DnssecClient.java:151)
    at org.minidns.dnssec.DnssecClient.performVerification(DnssecClient.java:115)
    at org.minidns.dnssec.DnssecClient.queryDnssec(DnssecClient.java:105)
    at org.minidns.hla.DnssecResolverApi.resolve(DnssecResolverApi.java:65)
    at eu.rekisoft.android.editmail.DnsTest.checkDomain(DnsTest.kt:36)
    at eu.rekisoft.android.editmail.DnsTest.evaluating(DnsTest.kt:29)
Flowdalic commented 2 years ago

It appears there are invalid NSEC RRs for .xxx, together with valid ones. MiniDNS does throw a DnssecValidationFailedException as soon as it can not verify the signature for a NSEC record. I assume dig simply checks if there is at least a valid one. If my current, preliminary, assessment is correct, then this behavior could be changed (or at least made configurable) to match dig's behavior.