Skip to content

Added subdomain dns check for domain verification.#50

Merged
philippem merged 5 commits into
Blockstream:masterfrom
Randy808:subdomain-dns-verification
Sep 27, 2024
Merged

Added subdomain dns check for domain verification.#50
philippem merged 5 commits into
Blockstream:masterfrom
Randy808:subdomain-dns-verification

Conversation

@Randy808

@Randy808 Randy808 commented Feb 12, 2024

Copy link
Copy Markdown
Collaborator

Currently domains are validated using a well-known uri located at https://{domain}/.well-known/liquid-asset-issuer-proof-{asset_id} but this change aims to add an alternative verification method leveraging DNS TXT records. This would empower issuers to add proofs using their DNS provider and avoid the requirement of hosting at a particular domain.

@Randy808 Randy808 force-pushed the subdomain-dns-verification branch from aabebcf to 3755c69 Compare February 13, 2024 14:33
@Randy808 Randy808 marked this pull request as ready for review February 13, 2024 14:34
@Randy808 Randy808 requested a review from shesek February 13, 2024 14:34
Comment thread src/entity.rs
Comment thread src/entity.rs Outdated
@shesek

shesek commented Feb 19, 2024

Copy link
Copy Markdown
Collaborator

Why was it decided to verify the TXT record against a subdomain with the asset's id prefix?

Requiring issuers to prove their ability to add TXT records directly to the user-visible domain name would give us a stronger guarantee of ownership. The distinction shouldn't matter in the typical case, but there are some exceptional cases where it may - for example domain owners that give DNS control over subdomains to their users (like afaird.org or dyno), or if an attacker manages to gain DNS access to a seemingly-innocent subdomain through social engineering (another way to mitigate the last concern is to make the subdomain name intention clearer, for example liquid-asset-verification-d8a5e71fab92.foocorp.com).

Note that verifying against the main domain name requires checking all TXT records and not just the first one as currently implemented, both because they may be other non-liquid-related TXT records and because there may be multiple assets linked to the domain.

This also has the advantage of discoverability, as users can query for TXT records from the main domain name to find linked assets, without having to know the asset ids to begin with. This can also be used to resolve strings like usdt@tether.to to the linked asset id (tickers are guaranteed to be unique within a domain name). Doing both things is already theoretically possible through the asset registry (if it supported lookups by domain name, which it currently doesn't), but it can reduce reliance on it.

@shesek

shesek commented Feb 20, 2024

Copy link
Copy Markdown
Collaborator

It would be awesome if this used @TheBlueMatt's dnssec-prover (website) to keep and serve self-validating proofs of domain ownership/linkage.

With the current HTTP-based verification method, we basically tell users that we looked and found the verification file at some point in the past, but can't prove it to them. The user has to take our word for it, or issue an HTTP request to check themselves - which requires costly IO (esp if you're looking to verify the registry in bulk), has privacy implications (your DNS provider and network observers can tell which assets you're interested in, which likely means you have them in your wallet) and may fail due to some temporary connection error or because the file was since removed. If it does fail, the user can't tell if we lied or if something changed since. And even if they can somehow tell we lied, they wouldn't be able to prove it.

With dnssec-prover we can provide proofs that don't require trusting the asset registry (only the DNS root trust anchors and hierarchy) and that can be validated on their own without requiring any network activity (offline even).

Edit: also, dnssec-prover supports WASM so we could even run verification in the liquid block explorer client side ^_^

@shesek

shesek commented Feb 21, 2024

Copy link
Copy Markdown
Collaborator

Some other alternatives for the DNS address to use for the TXT records:

  • A fixed subdomain for all the assets, e.g. _liquid-assets.foocorp.com. This still has the advantage of discoverability, but doesn't pollute the main domain name's records.

  • A subdomain per ticket, e.g. usdt._liquid-assets.foocorp.com. This allows resolving asset identifiers like usdt@foocorp.com, but not to discover the list of all assets issued by the issuer.

@Randy808

Randy808 commented Feb 22, 2024

Copy link
Copy Markdown
Collaborator Author

I liked the fixed subdomain approach. I considered putting the TXT records on the main domain but I didn't want to interfere with any existing TXT records infrastructure an issuer may have already set up. I also think discoverability of assets for a particular domain is desirable over downloading and searching the index.json file from asset_registry_db.

I'll also look more into the dnssec-prover you mentioned because it sounds like there would be a lot of upside for issuers if implemented.

Edit: I added a description to this pull request to give the motivation behind this change

@shesek

shesek commented Feb 26, 2024

Copy link
Copy Markdown
Collaborator

Thanks for better explaining the motivation. So basically that issuers don't have to manage a webserver in addition to the DNS they must already manage either way. Makes sense.


I suggest we use a a more compact verification string for the TXT record rather than the human-readable sentence we use for HTTP-based verification. And also include the ticker name in there. Perhaps something like liquid-asset-verification=<asset-id>,<ticker>


It should be noted that the TXT record only verifies the linkage in one direction - that the domain owner verifiably agreed to associate their domain name to the specified asset id (and ticker if we add that). But it doesn't verify that the on-chain issuance associated itself with the domain name via the contract hash commitment.

I think there's value in providing just that one-way verification (typically the trust anchor for the user is the domain name and not the asset id, so while it makes sense for an asset to try and impersonate itself as related to a domain, it doesn't really seem to make sense for a domain owner to impersonate an asset id), but we could theoretically do better if we wanted to -

A two-way verification requires 3 more pieces of information: the contract_json (and its SHA256 contract_hash), the issuance_txin (asset issuance txid + the index of the issuance input in it) and the issuance_prevout (the prev output spent by the issuance input)

The asset_id is a function of contract_hash, issuance_txin and issuance_prevout, so given these 3 you can verify that the asset_id issuance committed to the given contract json/hash.

We can get the asset_id, contract_hash, issuance_txin and issuance_prevout to fit inside a TXT record (its ASCII-only with a maximum of 255 characters, and we need roughly 183 characters if we use bae64 encoding. also the asset_id isn't strictly necessary because it can be computed from the other fields, but its probably better to have it more readily available. and you don't really need the contract_hash if you have the full contract_json, however...)

However getting the contract_json in is more tricky. It doesn't quite fit even as a separate TXT record. I checked and (after disregarding some extreme outliers in large volumes that were nearly identical and significantly altered the results) it appears that 68% of the issuance JSONs are under the 255 limit while the average issuance JSON length is 281.

A simple solution is splitting the contract_json over multiple TXT records. Another one is restructuring the contract json hashing such that we only hash only the officially supported fields at the top level (version, ticker, name, precision, domain name and issuer pubkey), plus a single hash for all the custom fields added by the issuer if any (could also be a merkle tree but that may be an overkill). That way the proof data necessary for verifying the official fields (and specifically the domain linkage) is more compact and should fit inside a single record. You'll only need the full contract_json if you need to verify custom fields.

@ErikDeSmedt

Copy link
Copy Markdown
Collaborator

Multiple users have been asking for DNS-verification.

I think the two-way verification is interesting but I don't want it to delay this PR.
Could we move this discussion to a separate issue?

@shesek

shesek commented Mar 1, 2024

Copy link
Copy Markdown
Collaborator

I think the two-way verification is interesting but I don't want it to delay this PR.

Agreed. Just wanted to share my thoughts and put this out there as an option, I wouldn't prioritize this either. (It's perhaps worth noting that other similar domain-based solutions like Lightning Address and TheBlueMatt's Human Readable Payment Instructions don't do two-way either. also, two-way is possible here, just not through the TXT records)

So we just need to finalize the DNS address to use and the TXT record contents. Also dnssec-prover would be a big win imo, but it can be done later in a separate PR.

@Randy808

Randy808 commented Mar 19, 2024

Copy link
Copy Markdown
Collaborator Author

Updated the PR but haven't been able to get the tests to run

Edit: Working after updating packages with cargo update rocket rocket_contrib pear pear_codegen

@Randy808 Randy808 force-pushed the subdomain-dns-verification branch from 0e90c99 to 034c87c Compare March 19, 2024 14:22
@Randy808 Randy808 force-pushed the subdomain-dns-verification branch 3 times, most recently from 4868488 to cfc3be5 Compare April 23, 2024 15:04
@Randy808

Copy link
Copy Markdown
Collaborator Author

@shesek Anything else needed to get this PR in?

Comment thread Cargo.toml Outdated
@Randy808 Randy808 requested a review from shesek May 6, 2024 20:59
@Randy808 Randy808 force-pushed the subdomain-dns-verification branch 2 times, most recently from c457916 to 490996b Compare May 7, 2024 14:02
@Randy808 Randy808 requested review from ErikDeSmedt and philippem May 20, 2024 16:20
@Randy808

Copy link
Copy Markdown
Collaborator Author

@shesek ?

shesek
shesek previously requested changes May 26, 2024

@shesek shesek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so sure about @TheBlueMatt's suggestion (I think it might've been tongue in cheek? like if you're not doing it the right way, you might as well..)

For me, using the standard DNS protocol is preferable over using Google's dns.google even if we're using Google's 8.8.8.8, as there's no vendor lock-in and we could freely and easily switch to another DNS server at any time.

If using Google's DNS servers isn't desirable, we could also use Resolver::from_system_conf(), which will use the DNS servers from /etc/resolve.conf (probably the datacenter's DNS?), or define any other DNS server we want.

Also, if we do end up deciding to use DNS, perhaps we could look into switching from trust_dns_resolver to hickory_resolver? The former mentions on its README that the latter is its continuation.

Comment thread src/entity.rs Outdated
Comment thread src/entity.rs Outdated
Comment thread src/entity.rs Outdated
@shesek

shesek commented May 26, 2024

Copy link
Copy Markdown
Collaborator

If using Google's DNS servers isn't desirable, we could also use Resolver::from_system_conf(), which will use the DNS servers from /etc/resolve.conf

Oh huh, I just realized that this would actually also be Google, since this is hosted on GCP...

@Randy808

Randy808 commented May 27, 2024

Copy link
Copy Markdown
Collaborator Author

I'm not so sure about @TheBlueMatt's suggestion (I think it might've been tongue in cheek? like if you're not doing it the right way, you might as well..)

For me, using the standard DNS protocol is preferable over using Google's dns.google even if we're using Google's 8.8.8.8, as there's no vendor lock-in and we could freely and easily switch to another DNS server at any time.

If using Google's DNS servers isn't desirable, we could also use Resolver::from_system_conf(), which will use the DNS servers from /etc/resolve.conf (probably the datacenter's DNS?), or define any other DNS server we want.

Also, if we do end up deciding to use DNS, perhaps we could look into switching from trust_dns_resolver to hickory_resolver? The former mentions on its README that the latter is its continuation.

No I don't think he was, considering the default that's used for hickory-dns (and trust-dns) is google and that the dependency list is already pretty bulky (with rust-bitcoin and rust-elements). The newer versions of hickory-dns had some kind of dependency conflict so trust-dns was used to avoid having to making any changes to dependency versions (which are already extremely out of date given that there hasn't been a commit in 2 years).

@Randy808 Randy808 force-pushed the subdomain-dns-verification branch from 490996b to 861cf53 Compare August 20, 2024 20:48
@Randy808 Randy808 requested a review from shesek August 20, 2024 20:51
@Randy808

Copy link
Copy Markdown
Collaborator Author

Updated PR
cc: @jtrivinop

@jtrivinop

Copy link
Copy Markdown

@shesek is there anything pending here before this can be merged?

@shesek

shesek commented Sep 26, 2024

Copy link
Copy Markdown
Collaborator

I'm still not convinced that avoiding an extra dependency is worth relying on a non-standard Google-only API.

If it is decided to go with this approach, the implementation itself looks good to me. But there appears to be some unintentionally committed changes to Cargo.toml and the Dockerfile.

@Randy808

Copy link
Copy Markdown
Collaborator Author

Not mistaken, but reverted since #52 seems to do a more comprehensive update.

@philippem

Copy link
Copy Markdown
Collaborator

I'm still not convinced that avoiding an extra dependency is worth relying on a non-standard Google-only API.

If it is decided to go with this approach, the implementation itself looks good to me. But there appears to be some unintentionally committed changes to Cargo.toml and the Dockerfile.

We'd like to proceed with this implementation.

@philippem philippem dismissed shesek’s stale review September 27, 2024 18:50

We'd like to proceed with the implementation.

@philippem philippem merged commit 858e867 into Blockstream:master Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants