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

inet_dns: support for SIG(0) #7815

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

inet_dns: support for SIG(0) #7815

wants to merge 4 commits into from

Conversation

jimdigriz
Copy link
Contributor

@jimdigriz jimdigriz commented Nov 2, 2023

This Work-in-Progress is to add SIG(0) support to OTP; similarly to the support for TSIG in #6985.

This work is necessary so that user code may implement the signing required by SRP; I have created a reference client implementation.

Outstanding tasks:

  • decoder
  • encoder
  • verify messages
    • supports query validation only so far
  • sign messages
  • support TCP multi-message
  • tests (...see below though, I need guidance on what would be consider appropriate)
  • (optionally) support more KEY formats

Outstanding Questions:

  • is this wanted? if not, could I lobby for just the #dns_rr_sig{} part so the rest could be handled in user land; similarly to TSIG we need to know the byte offset of the SIG(0) record and this is best worked out in inet_dns.erl (which also includes FORMERR handling) to avoid having a second DNS decoder to figure that out
  • public_key.hrl is included in inet_dns.erl
    • this is to support {encode,decode}_key_publickey/2 which maybe should be moved into inet_dns_sig0.erl
      • I remember for TSIG it was requested that {encode,decode}_algname/1 remained in inet_dns.erl
    • I like the idea of returning public_key types in the KEY RR as the binary would not be usable unable to do this anyway (avoiding needless duplication in user code)
  • which security algorithms need to be included
  • there is some overlap with DNSSEC here, so there may be a desire to generalise this
    • this may though be no more difficult that writing an implementation of Canonical DNS Name Ordering (RFC2535, section 8)
    • I do not know enough about DNSSEC to be able to determine how the interface should function so would prefer to keep this as SIG(0) only

Things that make this fun, I am unable to see how end-to-end testing will work as:

  • knotd does not support SIG(0); looks like it was removed in 2.0
  • BIND9 supports SIG(0) but does not:
    • support multiple messages seen via TCP
    • sign responses
      • looking through the instructions and the source code I think it is unable to
      • though in practice this is probably not useful, as the client would need to pre-trust the KEY RR somehow
  • dnspython does not support SIG(0)

Maybe a rework of the reference Python implementation would suffice as a sort of DNS bitbanger?

Copy link
Contributor

github-actions bot commented Nov 2, 2023

CT Test Results

       2 files       65 suites   1h 1m 19s ⏱️
1 511 tests 1 251 ✔️ 253 💤 7
1 703 runs  1 395 ✔️ 301 💤 7

For more details on these failures, see this check.

Results for commit 65e99dd.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@garazdawi garazdawi marked this pull request as draft November 3, 2023 07:52
@jhogberg jhogberg added the team:PS Assigned to OTP team PS label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants