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

dnsdist: Implement SNIRule for DoT and DoH #7825

Merged
merged 4 commits into from May 20, 2019

Conversation

Projects
None yet
4 participants
@rgacogne
Copy link
Member

commented May 15, 2019

Short description

Support for DoT should work with any version of OpenSSL or GnuTLS but for DoH it requires the addition of h2o_socket_get_ssl_server_name() in libh2o (PR in preparation). It's already possible to do HTTP Host header matching for DoH, though.

Closes #7210.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@rgacogne rgacogne added this to the dnsdist-1.4.0 milestone May 15, 2019

@rgacogne rgacogne requested a review from Habbie May 17, 2019

@Habbie

Habbie approved these changes May 17, 2019

Copy link
Member

left a comment

Looks good!

I've tested with OpenSSL and GnuTLS. I did not test h2o SNI.

@rgacogne rgacogne force-pushed the rgacogne:dnsdist-snirule branch from 963b7f1 to 5fc1b44 May 17, 2019

@rgacogne

This comment has been minimized.

Copy link
Member Author

commented May 17, 2019

The corresponding PR to h2o is h2o/h2o#2054. I don't think we need to wait until it is either merged or refused, though, because even if it does not make it we properly detect that the support is not present.

@pieterlexis

This comment has been minimized.

Copy link
Member

commented May 17, 2019

It might make sense to document that as of h2o version 2.3.0-beta, it is not possible to extract the SNI value from DOH connections and that HTTP Host should be used.

@Habbie

This comment has been minimized.

Copy link
Member

commented May 18, 2019

It might make sense to document that as of h2o version 2.3.0-beta, it is not possible to extract the SNI value from DOH connections and that HTTP Host should be used.

Linking to HTTPHeaderRule in that bit of text would also be nice.

@M4t7e

This comment has been minimized.

Copy link

commented May 18, 2019

I have tested the SNIRule for DoT and it works as expected so far. Thank you very much! It would be great to see this feature in the upcoming release 😃

@rgacogne

This comment has been minimized.

Copy link
Member Author

commented May 19, 2019

The h2o PR (h2o/h2o#2054) has been merged so h2o_socket_get_ssl_server_name() should be in the upcoming 2.3.0 release.

@rgacogne

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

Thanks! I added a few words stating that SNIRule will unfortunately not work with h2o <= 2.3.0-beta, with a link to HTTPHeaderRule.

rgacogne added some commits May 15, 2019

@rgacogne rgacogne force-pushed the rgacogne:dnsdist-snirule branch from 36f56a2 to ba45a22 May 20, 2019

@rgacogne

This comment has been minimized.

Copy link
Member Author

commented May 20, 2019

Rebased to fix a conflict following the merge of #7830.

@rgacogne rgacogne merged commit ff5730b into PowerDNS:master May 20, 2019

18 checks passed

ci/circleci: build-auth Your tests passed on CircleCI!
Details
ci/circleci: build-recursor Your tests passed on CircleCI!
Details
ci/circleci: test-auth-algorithms Your tests passed on CircleCI!
Details
ci/circleci: test-auth-api Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-bind Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-gmysql Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-gpgsql Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-gsqlite3 Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-ldap Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-lmdb Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-mydns Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-odbc-mssql Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-odbc-sqlite3 Your tests passed on CircleCI!
Details
ci/circleci: test-auth-regress-tinydns Your tests passed on CircleCI!
Details
ci/circleci: test-recursor-api Your tests passed on CircleCI!
Details
ci/circleci: test-recursor-bulk Your tests passed on CircleCI!
Details
ci/circleci: test-recursor-regression Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rgacogne rgacogne deleted the rgacogne:dnsdist-snirule branch May 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.