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

Some IPFS DOH Fixes #14631

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Some IPFS DOH Fixes #14631

merged 2 commits into from
Aug 17, 2022

Conversation

cypt4
Copy link
Collaborator

@cypt4 cypt4 commented Aug 15, 2022

  1. Remove IPFS DOH DNS override when user switches DOH off.
  2. Remove {?dns} from the url in DNS.Resolvers, because lib-p2p only uses POST requests.

Fixes brave/brave-browser#24678
Fixes brave/brave-browser#24706

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

For brave/brave-browser#24678
Enable DOH, check brave_ipfs/config file in the browser's data folder. Make sure there is DNS.Resolvers property
Disable DOH, check the config again. Make sure there is no DNS.Resolvers property.

For brave/brave-browser#24706
Make sure that DOH override works for Google DNS which is using {?dns} template url.
Before check also it is needed to modify brave_ipfs/config to remove non-dnsname bootstrap urls.

Fixes brave/brave-browser#24706
Currently Kubo doesn't support such template and only uses POST requests
Copy link
Member

@yrliou yrliou left a comment

Choose a reason for hiding this comment

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

LGTM

@cypt4 cypt4 merged commit bb5136e into master Aug 17, 2022
@cypt4 cypt4 deleted the brave_24678 branch August 17, 2022 11:15
@github-actions github-actions bot added this to the 1.44.x - Nightly milestone Aug 17, 2022
@cypt4 cypt4 changed the title Remove DNS.Resolvers from IPNS config if DOH is disabled Some IPFS DOH Fixes Aug 18, 2022
brave-builds pushed a commit that referenced this pull request Aug 19, 2022
@stephendonner
Copy link
Contributor

  1. Issue Remove DNS.Resolvers IPNS config property if user disables DOH brave-browser#24678: filed DNS resolver doesn't get reset after enabling, then disabling, DoH for IPNS brave-browser#24900
  1. Verified PASSED using
Brave 1.44.62 Chromium: 104.0.5112.102 (Official Build) nightly (x86_64)
Revision 8e5396254975ef939f2ef7d0bd334e48a052b536-refs/branch-heads/5112@{#1478}
OS macOS Version 11.6.8 (Build 20G730)
  1. install 1.44.62
  2. launch Brave
  3. load brave://ipfs-internals/
  4. click on Install and start
  5. chose Google DNS as a DoH provider in brave://settings/security
  6. removed the following from {profile_folder}/brave_ipfs/config
"/ip4/104.131.131.82/tcp/4001/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ","/ip4/104.131.131.82/udp/4001/quic/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ",
  1. loaded ipns://ipfs.io
  2. clicked Use a Brave IPFS node

Confirmed I was able to load ipns://ipfs.io

example example example example
Screen Shot 2022-08-23 at 9 41 09 AM Screen Shot 2022-08-23 at 9 42 33 AM Screen Shot 2022-08-23 at 10 29 14 AM Screen Shot 2022-08-23 at 9 42 41 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants