nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
105.38k stars 28.58k forks source link

[DNS] TLSA records [HTTPS] DANE request #39569

Open Falci opened 2 years ago

Falci commented 2 years ago

Is your feature request related to a problem? Please describe. I'd like to make an HTTPS request to a server that uses a self-signed certificate that follows the DANE protocol (Wikipedia)

Describe the solution you'd like I believe the best option would be an extra option on HTTPS request:

https.get('https://example.com', {dane: true})

Describe alternatives you've considered I tried to create a new https.Agent that forces rejectUnauthorized: false; Then, I got the tlsSocket instance in the keylog event and added a listener for the secureConnect event; This moment I realised that the DNS api don't have a resolveTLSA. Not sure how to continue from here.

gireeshpunathil commented 2 years ago

@nodejs/dns

github-actions[bot] commented 2 years ago

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

rithvikvibhu commented 2 years ago

This request is a bit old, but I'd like to +1, it would be really helpful.

Even if dane isn't added to https, at least resolveTLSA can be.

Looks like it's just adding a few lines next to https://github.com/nodejs/node/blob/1000eb168a06cc4403454c48dd1a2f8bdfd60275/lib/dns.js#L304 and https://github.com/nodejs/node/blob/3914354cd7ddc65774f13bbe435978217149793c/src/cares_wrap.h#L457

For reference, I'm looking at how resolveTxt was added: https://github.com/nodejs/node/commit/d9c67aebfff6ec2187890eb24f48edf087e385db

bnoordhuis commented 2 years ago

The first step would be to add TLSA support to upstream c-ares, then add a binding to node.

Iso5786 commented 2 years ago

In some time I also would be in need of this feature.

github-actions[bot] commented 1 year ago

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] commented 1 year ago

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

titanism commented 1 year ago

👋 Hey - we made 🍊 Tangerine with support for TLSA records among others (such as CERT) that the DNS module does not provide. We used inspiration from dnspython for the format of objects returned.

🍊 Tangerine is a 1:1 drop-in replacement for the Node.js DNS module and it also supports resolver.resolve(host, 'TLSA') and resolver.resolveTlsa) – which would make it possible for TLSA/DANE lookup. See https://github.com/forwardemail/tangerine. It also uses DNS over HTTPS and is as fast as the Node.js DNS module (see our benchmarks in the README of this repo).

const Tangerine = require('tangerine');

const tangerine = new Tangerine();

console.log(await tangerine.resolveTlsa('_25._tcp.internet.nl'));
[
  {
    cert: Buffer @Uint8Array [
      e1ae9c3d e848ece1 ba72e0d9 91ae4d0d 9ec547c6 bad1ddda b9d6beb0 a7e0e0d8
    ],
    mtype: 1,
    name: 'proloprod.mail._dane.internet.nl',
    selector: 1,
    ttl: 622,
    usage: 2,
  },
  {
    cert: Buffer @Uint8Array [
      d6fea64d 4e68caea b7cbb2e0 f905d7f3 ca3308b1 2fd88c5b 469f08ad 7e05c7c7
    ],
    mtype: 1,
    name: 'proloprod.mail._dane.internet.nl',
    selector: 1,
    ttl: 622,
    usage: 3,
  },
]

See https://github.com/forwardemail/tangerine#tangerineresolvetlsahostname--options-abortcontroller for more insight.

bradh352 commented 8 months ago

c-ares will support this in v1.22 by year's end. https://github.com/c-ares/c-ares/pull/600

bnoordhuis commented 8 months ago

Thanks for the update. I'll reopen the issue.

bradh352 commented 8 months ago

c-ares 1.22.0 released with this capability

titanism commented 8 months ago

Awesome, can't wait to get DANE support added to our project @forwardemail

Falci commented 8 months ago

The first step would be to add TLSA support to upstream c-ares, then add a binding to node.

How is the process here? Now that we have the first step done, do we need a task specifically for adding a binding?

Am I too naive assuming this could be done by simply copying and renaming the binding of another DNS record? If not, and if there's nobody else with demand capacity, I could give it a try.

bradh352 commented 8 months ago

Unfortunately the implementation for TLSA uses the new DNS Record helpers in c-ares, and not the legacy parser style, so if you're looking to just copy paste and make slight modifications, that's not going to work, though I don't think you'll find it too difficult.

The general logic is you'll pass ARES_REC_TYPE_TLSA as the query in ares_create_query() or ares_mkquery() whichever you normally use.

Then to extract the results, you'd call:

ares_dns_record_t *dnsrec = NULL;
/* abuf and alen are returned to the callback registered with ares_send(), which is what I assume you're using */
ares_dns_parse(abuf, alen, 0, &dnsrec);
for (size_t i=0; i<ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER); i++) {
  const ares_dns_rr_t *rr = ares_dns_record_rr_get(dnsrec, ARES_SECTION_ANSWER, i);
  if (ares_dns_rr_get_type(rr) != ARES_REC_TYPE_TLSA)
    continue;
  /* Extract data */
  unsigned char cert_usage = ares_dns_rr_get_u8(rr, ARES_RR_TLSA_CERT_USAGE);
  unsigned char selector = ares_dns_rr_get_u8(rr, ARES_RR_TLSA_SELECTOR);
  unsigned char match = ares_dns_rr_get_u8(rr, ARES_RR_TLSA_MATCH);
  size_t data_len;
  const unsigned char *data = ares_dns_rr_get_bin(rr, ARES_RR_TLSA_DATA, &data_len);

  /* Do something with extracted data ... */
}
ares_dns_record_destroy(dnsrec);

There's also ares_tlsa_usage_t, ares_tlsa_selector_t, and ares_tlsa_match_t with some enumerations containing known values for some of the fields.

Docs here on the new functionality: https://c-ares.org/ares_dns_record.html https://c-ares.org/ares_dns_rr.html https://c-ares.org/ares_dns_mapping.html

github-actions[bot] commented 2 months ago

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

rithvikvibhu commented 1 month ago

Commenting so that stalebot does not close the issue. There is a PR for this: #52983