hildjj / dohdec

Lookup and decode DNS records using DNS-over-HTTPS (DoH)
MIT License
19 stars 5 forks source link

Support option to disable DNSSEC validation #38

Closed gnarea closed 1 year ago

gnarea commented 1 year ago

This enables the cd flag when DNSSEC records are requested, but we want to disable DNSSEC validation on the resolver.

I created this test file to test the changes against real resolvers (but didn't commit it):

// pkg/dohdec/test/real.ava.js

import {DNSoverHTTPS} from '../lib/doh.js'
import {DNSoverTLS} from '../lib/index.js'
// eslint-disable-next-line node/no-missing-import
import test from 'ava'

test('DoH - no dnssec', async t => {
  const doh = new DNSoverHTTPS()
  const r = await doh.lookup('ietf.org', {
    json: false,
    dnssec: false,
  })
  t.truthy(r)

  t.false(r.answers.some(a => a.type === 'RRSIG'))
  t.false(r.flag_cd)
})

test('DoH - dnssec', async t => {
  const doh = new DNSoverHTTPS()
  const r = await doh.lookup('ietf.org', {
    json: false,
    dnssec: true,
  })
  t.truthy(r)

  t.true(r.answers.some(a => a.type === 'RRSIG'))
  t.false(r.flag_cd)
})

test('DoH - dnssec failed', async t => {
  const doh = new DNSoverHTTPS()
  const r = await doh.lookup('dnssec-failed.org', {
    json: false,
    dnssec: true,
  })
  t.truthy(r)

  t.is(r.rcode, 'SERVFAIL')
  t.is(r.answers.length, 0)
  t.false(r.flag_cd)
})

test('DoH - dnssec failed with cd=1', async t => {
  const doh = new DNSoverHTTPS()
  const r = await doh.lookup('dnssec-failed.org', {
    json: false,
    dnssec: true,
    dnssecCd: true,
  })
  t.truthy(r)

  t.is(r.rcode, 'NOERROR')
  t.true(r.answers.some(a => a.type === 'RRSIG'))
  t.true(r.flag_cd)
})

test('DoT - no dnssec', async t => {
  const dot = new DNSoverTLS()
  const r = await dot.lookup('ietf.org', {
    json: false,
    dnssec: false,
  })
  t.truthy(r)

  t.false(r.answers.some(a => a.type === 'RRSIG'))
  t.false(r.flag_cd)
})

test('DoT - dnssec', async t => {
  const dot = new DNSoverTLS()
  const r = await dot.lookup('ietf.org', {
    json: false,
    dnssec: true,
  })
  t.truthy(r)

  t.true(r.answers.some(a => a.type === 'RRSIG'))
  t.false(r.flag_cd)
})

test('DoT - dnssec failed', async t => {
  const dot = new DNSoverTLS()
  const r = await dot.lookup('dnssec-failed.org', {
    json: false,
    dnssec: true,
  })
  t.truthy(r)

  t.is(r.rcode, 'SERVFAIL')
  t.is(r.answers.length, 0)
  t.false(r.flag_cd)
})

Fixes #37

gnarea commented 1 year ago

I spotted a couple of bugs whilst testing the PR but now everything looks good and I won't be making further changes until you provide some feedback.

hildjj commented 1 year ago

OK, this looks ready to go to me. There's a solid 10% chance this is going to break GHA when it lands, because I hadn't updated the action definition to run on pull requests to main instead of master, but I'll deal with that in my update PR that will come after this. Anything else before I merge?

gnarea commented 1 year ago

Thank you! I think we're good to go! šŸš€

gnarea commented 2 days ago

Hi @hildjj šŸ‘‹šŸ¾ Can you please release this change to NPM? I've been relying on the Git tag but that's now breaking CI for me. TIA! šŸ™‡šŸ¾

hildjj commented 2 days ago

I'll look at this first thing Tuesday. I think this is blocked on https://github.com/hildjj/mock-tls-server/issues/2, so I either have to fix that or rewrite the tests.

hildjj commented 5 hours ago

OK, I think I'm ready to cut a major release, but I'm too tired today to re-learn lerna. Release probably will happen tomorrow morning MDT.