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

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

Open
Falci opened this issue Jul 28, 2021 · 16 comments
Open

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

Falci opened this issue Jul 28, 2021 · 16 comments
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. dns Issues and PRs related to the dns subsystem. feature request Issues that request new features to be added to Node.js. https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.

Comments

@Falci
Copy link

Falci commented Jul 28, 2021

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.

@Ayase-252 Ayase-252 added the feature request Issues that request new features to be added to Node.js. label Jul 29, 2021
@gireeshpunathil
Copy link
Member

@nodejs/dns

@tniessen tniessen added dns Issues and PRs related to the dns subsystem. https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem. labels Aug 8, 2021
@github-actions
Copy link
Contributor

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 github-actions bot added the stale label Mar 29, 2022
@rithvikvibhu
Copy link

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

node/lib/dns.js

Line 304 in 1000eb1

Resolver.prototype.resolveTxt = resolveMap.TXT = resolver('queryTxt');
and
static constexpr const char* name = "resolveTxt";

For reference, I'm looking at how resolveTxt was added: d9c67ae

@bnoordhuis
Copy link
Member

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

@Iso5786
Copy link

Iso5786 commented Jun 2, 2022

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

@github-actions
Copy link
Contributor

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 github-actions bot added the stale label Nov 30, 2022
@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2022
@titanism
Copy link

titanism commented Mar 7, 2023

👋 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
Copy link
Contributor

bradh352 commented Nov 7, 2023

c-ares will support this in v1.22 by year's end. c-ares/c-ares#600

@bnoordhuis
Copy link
Member

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

@bnoordhuis bnoordhuis reopened this Nov 7, 2023
@bnoordhuis bnoordhuis added cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. and removed stale labels Nov 7, 2023
@bradh352
Copy link
Contributor

c-ares 1.22.0 released with this capability

@titanism
Copy link

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

@Falci
Copy link
Author

Falci commented Nov 15, 2023

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

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

Copy link
Contributor

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered 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
Copy link

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

@github-actions github-actions bot removed the stale label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. dns Issues and PRs related to the dns subsystem. feature request Issues that request new features to be added to Node.js. https Issues or PRs related to the https subsystem. tls Issues and PRs related to the tls subsystem.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

9 participants