On 07/04/2021 03:45, Brian Hartvigsen wrote: > 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" (https://docs.umbrella.com/umbrella-api/docs/create-a-device) for how > to get a Device ID and "Organization ID endpoint" > (https://docs.umbrella.com/umbrella-api/docs/organization-endpoint) to get > organizations ID. For OpenDNS Home Users, there is no organization, see > Registration API endpoint > (https://docs.umbrella.com/umbrella-api/docs/registration-api-endpoint2) for > how to get a Device ID. Asset ID should be ignored unless specifically > instructed to use by support.
I think that this is fine to go in in principle. I have some queries about the actual code, as follows: 1) the version field is set to zero, but https://docs.umbrella.com/umbrella-api/docs/identifying-dns-traffic says it should be one. 2) I don't like the umbrella_data[512] declaration. I know it can't overflow, but declaring the array to the exact maximum size (and defining the calculation for that in a comment) makes it less likely that future modifiers of the code will assume they can add stuff without checking. I'd go further and declare a struct with the fixed stuff (the magic number, flags and version and a char array of the size needed for the longest set of sub-options. 3) Why is umbrella_device being converted from a text string to a byte array during packet-manipulation? That would surely be better done in option.c during option parsing, with some error checking for non-hex characters as well. 4) Your code starts each field with a single byte id, for instance 0x04 for UMBRELLA_ASSET, but https://docs.umbrella.com/umbrella-api/docs/identifying-dns-traffic says it should be two bytes, 0x00, 0x04 5) You are modifying queries with per-client data (addresses) so you need to set cacheable to zero in add_edns0_config() so that data which is valid for only one client doesn't get returned to another client from the cache. 6) Consider using the PUTLONG and PUTSHORT macros instead of memcpy() calls, to match the rest of the code. Cheers, Simon. _______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss