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

Cisco Umbrella Remote IP/Device ID support #1096

Closed
wants to merge 4 commits into from

Conversation

tresni
Copy link
Contributor

@tresni tresni commented Apr 2, 2021

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?: 5


This pull request add support for Cisco Umbrella/OpenDNS Device ID and remote IP reporting. This is based on the information at https://docs.umbrella.com/umbrella-api/docs/identifying-dns-traffic and https://docs.umbrella.com/umbrella-api/docs/identifying-dns-traffic2 . Using --umbrella by itself will enable Remote IP reporting. This can not be used for any policy filtering in Cisco Umbrella/OpenDNS. Additional information can be supplied using specific option specifications, multiple can be separated by a comma:

--umbrella=orgid:1234,deviceid=0123456789abcdef

Specifies that you want to report organization 1234 using device 0123456789abcdef. For Cisco Umbrella Enterprise, see Register (Create) a device how to get a Device ID and Organization ID endpoint to get organizations ID. For OpenDNS Home Users, there is no organization, see Registration API endpoint for how to get a Device ID. Asset ID should be ignored unless specifically instructed to use by support.

Mo Money, Mo Problems -- The Notorious B.I.G.

Signed-off-by: Brian Hartvigsen <brian.andrew@brianandjenny.com>
So the FCC won't let me be or let me be me -- Eminem

Signed-off-by: Brian Hartvigsen <brian.andrew@brianandjenny.com>
Signed-off-by: Brian Hartvigsen <brian.andrew@brianandjenny.com>
@DL6ER
Copy link
Member

DL6ER commented Apr 3, 2021

Thank you for your suggestion. Unfortunately, we cannot accept it right now as we have a policy to not accept changes to the dnsmasq code, unless it cannot be avoided (like adding Pi-hole-specific changes).
In your case, however, this is a pure dnsmasq change and, hence, iit should be submitted to the mother project. This will not only ensure everyone can benefit from it, but also it will avoid/reduce future merge conflicts when updating dnsmasq in our own project. We'll quickly incorporate it from there.

To show how much we appreciate your submission, we can help you converting your suggested change into a patch that can be directly submitted as attachment to dnsmasq-discuss@lists.thekelleys.org.uk (Info page, there is no option to open PRs to dnsmasq otherwise). Your authorship would be preserved by this patch.

I'm sorry for this but I hope you will understand our motives and why this is good for everyone and had nothing to do at all with your proposed change.

Also, please submit code changes to pi-hole/FTL#development instead of master in the future. This makes sure there are no merge conflicts because development is where, well, development takes place and where things are battle-tested. We never merge anything right away into master.

@DL6ER DL6ER closed this Apr 3, 2021
@tresni
Copy link
Contributor Author

tresni commented Apr 3, 2021

I'm happy to support either way. This was a change that I mentioned in the forums so I wanted to make sure I got a PR open. If submitting to dnsmasq is the way to go, I'm happy to do that. I have the dnsmasq source locally as well, I'll make an attempt at porting the patch to that code base. If I run into issues, can I post here?

@DL6ER
Copy link
Member

DL6ER commented Apr 3, 2021

Absolutely!

@tresni
Copy link
Contributor Author

tresni commented Apr 7, 2021

Patch has been submitted

@DL6ER
Copy link
Member

DL6ER commented Apr 14, 2021

@tresni If you want to use your feature already now in Pi-hole, you can switch ti the bleeding-edge FTL branch which just picked up your patch today (a27969c).

Use

pihole checkout ftl update/dnsmasq-v2.86

to get to this branch but remember to use

pihole checkout ftl master

before/after the next (not this week) regular release of FTL because these feature branches are deleted after they are merged.

@tresni
Copy link
Contributor Author

tresni commented Apr 15, 2021

Thanks @DL6ER , I'm currently using it and passed on the instructions to a few others I know.

@DL6ER
Copy link
Member

DL6ER commented Apr 15, 2021

@tresni Just be aware that this is in fact a bleeding-edge branch where we're constantly adding new stuff from the HEAD of dnsmasq development. It may happen that things break and we do not recognize (and fix) this immediately. We prefer to wait for official patches accepted upstream and this may even take weeks (or months) in extreme cases. Hence, I created the branch special/umbrella which freezes the code at this very time so new regressions should not affect you (and others) looking only for this feature but not any other bleeding-edge new features.

I'll re-open this issue ticket so I remember to send you a message to go back to master when we release the next version of FTL that has this feature natively.

@DL6ER DL6ER reopened this Apr 15, 2021
@DL6ER DL6ER closed this Apr 15, 2021
@DL6ER DL6ER mentioned this pull request Jun 22, 2021
5 tasks
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/join-us-beta-testing-pi-hole-ftl-v5-9-web-v5-6-and-core-v5-4/48021/1

1 similar comment
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/join-us-beta-testing-pi-hole-ftl-v5-9-web-v5-6-and-core-v5-4/48021/1

@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/example-post/49062/1

@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-9-web-v5-6-and-core-v5-4-released/49544/1

@jakeslabnet
Copy link

Hi there @tresni !! Possibly the wrong place to post this comment, but I came across this feature in the release notes for FTL 5.9. Maybe I'm just incredibly dense, but any chance you've got the instructions on how to enable this feature? I have an Umbrella enterprise account, have all the info needed from there to register a device / tie it to my org....but am a bit lost on where to add this config on the pi-hole side.

@tresni
Copy link
Contributor Author

tresni commented Sep 12, 2021

@jaykder You can’t do this through any Pi-hole UI, so need to be okay with the command line to do this. Create a file called /etc/dnsmasq.d/99-umbrella.conf and add the options there:

umbrella=org-id:12345,device-id=abcdef012345678

see http://www.thekelleys.org.uk/dnsmasq/docs/dnsmasq-man.html for details on config

@jakeslabnet
Copy link

@jaykder You can’t do this through any Pi-hole UI, so need to be okay with the command line to do this. Create a file called /etc/dnsmasq.d/99-umbrella.conf and add the options there:

umbrella 
=org-id:12345,device-id=abcdef012345678

see http://www.thekelleys.org.uk/dnsmasq/docs/dnsmasq-man.html for details on config

Perfect, all good with some CLI edits, thanks so much! Very excited to see this feature, thank you for the work!

@tresni
Copy link
Contributor Author

tresni commented Sep 12, 2021

@jaykder heads up that there is not suppose to be a line break in there. Just noticed, I originally typed that comment up on my phone so I just thought it was wrapping because of the phone.

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.

None yet

4 participants