Skip to content

Add DNS oracle protocol - #917

Open
Jim8y wants to merge 30 commits into
masterfrom
feature/dns-doh
Open

Add DNS oracle protocol#917
Jim8y wants to merge 30 commits into
masterfrom
feature/dns-doh

Conversation

@Jim8y

@Jim8y Jim8y commented Nov 17, 2025

Copy link
Copy Markdown
Contributor

Summary

  • add DNS-over-HTTPS oracle protocol and config wiring
  • document dns:// usage and add tests for certificate parsing

Testing

  • not run (not requested)

@shargon shargon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that it's better to define the dns server, and make a dns query, instead of http query

@erikzhang

Copy link
Copy Markdown
Member

I think that it's better to define the dns server, and make a dns query, instead of http query

We need DNS over HTTPS.

@shargon

shargon commented Nov 17, 2025

Copy link
Copy Markdown
Member

I think that it's better to define the dns server, and make a dns query, instead of http query

We need DNS over HTTPS.

Then it's DoH no Dns, we should rename the oracle protocol

Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs
Comment thread docs/oracle-dns-protocol.md Outdated
Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs Outdated
Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs Outdated
Comment thread docs/oracle-dns-protocol.md Outdated
Comment thread docs/oracle-dns-protocol.md Outdated
Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs Outdated
Comment thread docs/oracle-dns-protocol.md
Comment thread plugins/OracleService/OracleSettings.cs
@cschuchardt88

cschuchardt88 commented Nov 22, 2025

Copy link
Copy Markdown
Member

I think that it's better to define the dns server, and make a dns query, instead of http query

DoH is very commom see https://www.rfc-editor.org/rfc/rfc8484.html

Can we follow rfc8484?

@github-actions github-actions Bot added the N4 label Nov 26, 2025
Comment thread plugins/OracleService/OracleSettings.cs
ajara87 and others added 6 commits November 30, 2025 17:15
Co-authored-by: Will <201105916+Wi1l-B0t@users.noreply.github.com>
Co-authored-by: Will <201105916+Wi1l-B0t@users.noreply.github.com>
- Replace application/dns-json with standard application/dns-message
- Implement DNS wire format (RFC 1035) for query/response encoding
- Use HTTP POST method per RFC 8484 specification
- Add DNS name compression pointer support
- Support user-specified authority in URI (dns://resolver/domain)
- Fix CryptographicException handling in BuildPublicKey
- Move Accept header to constructor
- Add comprehensive unit tests for wire format handling
- Add integration tests for Cloudflare, Google, and Quad9 DoH endpoints
- Update documentation with RFC 8484 compliance details
@Jim8y

Jim8y commented Dec 3, 2025

Copy link
Copy Markdown
Contributor Author
  • ✅ application/dns-json → application/dns-message (RFC 8484)
  • ✅ HTTP GET → HTTP POST
  • ✅ JSON parsing → DNS wire format (RFC 1035)
  • ✅ Support user-specified authority (dns://resolver/domain)
  • ✅ Fix CryptographicException handling in BuildPublicKey
  • ✅ Move Accept header to constructor
  • ✅ Integration tests covering Cloudflare, Google, Quad9

Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs Outdated
Comment thread plugins/OracleService/OracleService.cs Outdated
@Wi1l-B0t

Copy link
Copy Markdown
Contributor

UT Failed.

@shargon

shargon commented Dec 29, 2025

Copy link
Copy Markdown
Member

@Jim8y any progress with this?

Co-authored-by: Christopher Schuchardt <8141309+cschuchardt88@users.noreply.github.com>
@codecov

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.56140% with 145 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@3981b92). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ugins/OracleService/Protocols/OracleDnsProtocol.cs 78.90% 65 Missing and 43 partials ⚠️
plugins/OracleService/OracleService.cs 22.91% 30 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master     #917   +/-   ##
=========================================
  Coverage          ?   47.79%           
=========================================
  Files             ?      275           
  Lines             ?    16431           
  Branches          ?     2142           
=========================================
  Hits              ?     7854           
  Misses            ?     8016           
  Partials          ?      561           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread tests/Neo.Plugins.OracleService.Tests/UT_OracleDnsProtocol.cs Outdated
Comment thread plugins/OracleService/OracleService.cs Outdated
@shargon
shargon requested review from Copilot and removed request for doubiliu May 17, 2026 09:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a DNS-over-HTTPS (RFC 8484) oracle protocol for dns: URIs (RFC 4501), wiring it into the OracleService plugin alongside the existing https:// and neofs:// protocols. Resolved records are returned as NeoVM-serialized Struct envelopes for in-contract consumption.

Changes:

  • Add OracleDnsProtocol implementing DNS wire-format query/parse (A/AAAA/TXT/CERT and others) over DoH POST.
  • Add DnsSettings, default config entry, and register the dns scheme in OracleService.Start; route DNS results as raw stack-item bytes (filter not supported).
  • Add user docs and an extensive unit/integration test suite (URI parsing, RCODE handling, name compression, multiple answers, etc.).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
plugins/OracleService/Protocols/OracleDnsProtocol.cs New DoH oracle protocol with RFC 1035/8484 wire-format parsing and result serialization.
plugins/OracleService/OracleService.cs Registers the dns protocol and adds DNS-specific result handling, logging, and base64 decoding.
plugins/OracleService/OracleSettings.cs Adds DnsSettings (EndPoint, Timeout) and exposes it on OracleSettings.
plugins/OracleService/OracleService.json Adds default Dns configuration block.
tests/Neo.Plugins.OracleService.Tests/UT_OracleDnsProtocol.cs Unit + opt-in integration tests covering URI parsing, record types, error mapping, and live DoH endpoints.
docs/oracle-dns-protocol.md User-facing documentation describing config, URI syntax, response schema, and contract usage.
Comments suppressed due to low confidence (6)

plugins/OracleService/Protocols/OracleDnsProtocol.cs:636

  • The queryParameters parameter is assigned via ??= but never used afterwards. Either remove the parameter (it's only useful inside the function) or use it (e.g., to validate query keys or extract them). As written, the public-looking API takes a parameter that's effectively ignored, which is misleading for callers (and at least one caller in ProcessAsync does pre-parse and pass query).
    internal static string BuildQueryName(Uri uri, NameValueCollection? queryParameters = null)
    {
        queryParameters ??= ParseQueryString(uri.Query);
        string dnsName = NormalizeDnsName(uri.GetComponents(UriComponents.Path, UriFormat.Unescaped));
        if (string.IsNullOrEmpty(dnsName))
            throw new FormatException("dns: URI must include a dnsname.");

        return dnsName;
    }

plugins/OracleService/Protocols/OracleDnsProtocol.cs:529

  • DecodeDnsName accepts forward-pointing or self-referential compression pointers as long as jumpCount stays under 128. A malicious response can use up to 128 jumps to produce a name many KB long (each label up to 63 bytes) and exhaust memory before the cap triggers, or cause O(n²) parsing. RFC 1035 implementations typically also require pointers to point strictly backwards (to a previously-decoded offset) to prevent forward loops. Consider enforcing pointer < currentOffset and a tighter byte limit on the assembled name length (≤255 octets per RFC 1035).
    private static (string Name, int NewOffset) DecodeDnsName(byte[] data, int offset)
    {
        StringBuilder name = new();
        int originalOffset = offset;
        bool jumped = false;
        int jumpCount = 0;
        const int maxJumps = 128; // Prevent infinite loops

        while (offset < data.Length)
        {
            byte length = data[offset];

            if (length == 0)
            {
                offset++;
                break;
            }

            // Check for compression pointer (top 2 bits set)
            if ((length & 0xC0) == 0xC0)
            {
                if (offset + 1 >= data.Length)
                    throw new FormatException("DNS name compression pointer truncated.");

                if (++jumpCount > maxJumps)
                    throw new FormatException("DNS name compression loop detected.");

                int pointer = ((length & 0x3F) << 8) | data[offset + 1];
                if (!jumped)
                {
                    originalOffset = offset + 2;
                    jumped = true;
                }
                offset = pointer;
                continue;
            }

            offset++;
            if (offset + length > data.Length)
                throw new FormatException("DNS label extends beyond message.");

            if (name.Length > 0)
                name.Append('.');

            name.Append(Encoding.ASCII.GetString(data, offset, length));
            offset += length;
        }

        return (name.ToString(), jumped ? originalOffset : offset);
    }

plugins/OracleService/Protocols/OracleDnsProtocol.cs:595

  • FormatTxtRecord silently breaks when a length-prefix would overrun the buffer, swallowing the corruption and returning a partial (potentially empty) string while still reporting Success. Oracle responses are consensus-critical: returning partial data on malformed input means different nodes parsing in the same way still get an "OK" stack item but downstream consumers can't tell the record was truncated. Consider throwing a FormatException here so the protocol returns OracleResponseCode.Error instead of silently fabricating an answer.
    private static string FormatTxtRecord(byte[] rdata)
    {
        StringBuilder result = new();
        int offset = 0;

        while (offset < rdata.Length)
        {
            int length = rdata[offset++];
            if (offset + length > rdata.Length)
                break;

            if (result.Length > 0)
                result.Append(' ');

            result.Append('"');
            result.Append(Encoding.UTF8.GetString(rdata, offset, length));
            result.Append('"');
            offset += length;
        }

        return result.ToString();
    }

plugins/OracleService/Protocols/OracleDnsProtocol.cs:475

  • Each ProcessAsync call sends a fresh DNS request with a random 16-bit ID, but the protocol does not verify that the response ID matches the query ID, nor that the response's question section matches the query name/type. RFC 8484 makes spoofing harder than UDP, but you're still trusting the upstream resolver — any HTTPS server in the trust chain could return arbitrary records under a different name. Since oracle nodes must reach consensus on the answer, mismatched responses (e.g., from caches or misconfigured proxies) silently get accepted. Consider validating message.Id == requestId and that the question section names the requested owner/type.
    private static DnsMessage ParseDnsResponse(byte[] data)
    {
        if (data is null || data.Length < DnsHeaderSize)
            throw new FormatException("DNS response too short.");

        DnsMessage message = new()
        {
            Id = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(0, 2)),
            Flags = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(2, 2))
        };

        ushort qdCount = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(4, 2));
        ushort anCount = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(6, 2));

        int offset = DnsHeaderSize;

        // Skip question section
        for (int i = 0; i < qdCount; i++)
        {
            offset = SkipDnsName(data, offset);
            offset += 4; // QTYPE (2) + QCLASS (2)
        }

        // Parse answer section
        for (int i = 0; i < anCount; i++)
        {
            (string name, int newOffset) = DecodeDnsName(data, offset);
            offset = newOffset;

            if (offset + 10 > data.Length)
                throw new FormatException("DNS response truncated in answer section.");

            ushort type = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(offset, 2));
            ushort cls = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(offset + 2, 2));
            uint ttl = BinaryPrimitives.ReadUInt32BigEndian(data.AsSpan(offset + 4, 4));
            ushort rdLength = BinaryPrimitives.ReadUInt16BigEndian(data.AsSpan(offset + 8, 2));
            offset += 10;

            if (offset + rdLength > data.Length)
                throw new FormatException("DNS response truncated in RDATA.");

            byte[] rdata = new byte[rdLength];
            Array.Copy(data, offset, rdata, 0, rdLength);
            offset += rdLength;

            message.Answers.Add(new DnsResourceRecord
            {
                Name = name,
                Type = type,
                Class = cls,
                Ttl = ttl,
                RData = rdata
            });
        }

        return message;
    }

plugins/OracleService/Protocols/OracleDnsProtocol.cs:200

  • For DoH consensus to work across oracle nodes, all nodes must produce byte-identical results from the same dns: URL. Several pieces of this design make that fragile:
  1. When the URI omits an authority, each node uses its own locally configured Dns.EndPoint, so different operators will hit different resolvers and may get different Answers ordering, TTLs, or even contents (especially for geo-routed records). The answer set is sorted in the order the resolver returned it.
  2. The Ttl is included in the serialized struct verbatim. Two nodes querying seconds apart will see different TTL countdowns, breaking consensus on the result bytes.
  3. Multi-answer responses (e.g., A records) are not sorted, so different resolvers' answer orderings will produce different serialized bytes.

This makes dns: requests effectively non-consensus-safe in practice. Consider either: (a) requiring the URL to specify the authority and normalizing/canonicalizing TTLs (e.g., zero them out), and sorting answers; or (b) documenting prominently that operators must agree on Dns:EndPoint and that consensus is best-effort.

        ResultAnswer[] answers = dnsResponse.Answers
            .Select(a => new ResultAnswer
            {
                Name = a.Name.TrimEnd('.'),
                Type = GetRecordTypeLabel(a.Type),
                Ttl = a.Ttl,
                Data = FormatRData(a.Type, a.RData)
            })
            .ToArray();

        byte[] stackBytes = SerializeStackItemEnvelope(queryName, recordTypeLabel, answers);
        if (stackBytes.Length > OracleResponse.MaxResultSize)
            return (OracleResponseCode.ResponseTooLarge, null);

        return (OracleResponseCode.Success, Convert.ToBase64String(stackBytes));
    }

plugins/OracleService/Protocols/OracleDnsProtocol.cs:626

  • OracleDnsProtocol.Configure() calls EnsureConfigured(force: true) which re-reads OracleSettings.Default.Dns and re-sets client.Timeout. However, HttpClient.Timeout cannot be changed after the first request has been sent — it throws InvalidOperationException. Since Configure is called by the plugin lifecycle after the client has potentially been used (e.g., on hot-reload), this can crash the service. The sibling OracleHttpsProtocol.Configure also sets client.Timeout, but in that file the headers are cleared first as well — same potential issue. Consider either creating a new HttpClient per Configure call or using a per-request CancellationTokenSource with the configured timeout instead of HttpClient.Timeout.
    private void EnsureConfigured(bool force = false)
    {
        if (configured && !force)
            return;
        lock (syncRoot)
        {
            if (configured && !force)
                return;
            var dnsSettings = OracleSettings.Default?.Dns ?? throw new InvalidOperationException("DNS settings are not loaded.");
            endpoint = dnsSettings.EndPoint;
            client.Timeout = dnsSettings.Timeout;
            configured = true;
        }
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs Outdated
Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs
Comment thread plugins/OracleService/Protocols/OracleDnsProtocol.cs
Comment thread plugins/OracleService/OracleService.cs Outdated
@Jim8y

Jim8y commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Pushed a follow-up for deterministic DNS oracle results and stricter response parsing.

Changes:

  • validate DNS response ID and question section
  • sort answers by name/type/data
  • normalize TTL to 0
  • reject truncated TXT records
  • reject forward/self compression pointers
  • use per-request timeout instead of resetting HttpClient.Timeout

Validation:

  • dotnet test tests/Neo.Plugins.OracleService.Tests/Neo.Plugins.OracleService.Tests.csproj --no-restore --filter UT_OracleDnsProtocol --verbosity minimal
  • dotnet format tests/Neo.Plugins.OracleService.Tests/Neo.Plugins.OracleService.Tests.csproj --no-restore --verify-no-changes --verbosity minimal

@cschuchardt88

Copy link
Copy Markdown
Member

can we add in this PR a way to resolve Neo Named Service aka NNS?

Related neo-project/proposals#239

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants