dnsdist: Allow randomly selecting a backend UDP socket and query ID

[rgacogne]

Short description

This PRs add two options that can be used to reduce the risk of UDP exchanges when the path between dnsdist and its backend is not trusted:

• setRandomizedIdsOverUDP() allows picking up a random ID
• setRandomizedOutgoingSockets() allows selecting a random outgoing socket if several are available, effectively improving the entropy of the source port.

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)

[github-actions]

@check-spelling-bot Report

Unrecognized words, please review:

• Pqueries

Previously acknowledged words that are now absent

dnsheader

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:rgacogne/pdns.git repository
on the ddist-random-ports branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/PowerDNS/pdns/issues/comments/1008949373" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

If the flagged items do not appear to be text

If items relate to a ...

• well-formed pattern.

  If you can write a pattern that would match it,
  try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

• binary file.

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

[rgacogne]

setRandomizedIdsOverUDP() allows picking up a random ID

I'm now thinking it would make to use a unbounded map instead of a pre-allocated vector when that option is in use. The map needs to be locked, of course, and will thus be less efficient than the current vector, but it's likely a better trade-off when memory availability is tight and the number of QPS is low, which is often the case in these setups.

[rgacogne]

setRandomizedIdsOverUDP() allows picking up a random ID

I'm now thinking it would make to use a unbounded map instead of a pre-allocated vector when that option is in use. The map needs to be locked, of course, and will thus be less efficient than the current vector, but it's likely a better trade-off when memory availability is tight and the number of QPS is low, which is often the case in these setups.

Done!

[omoerbeek]

Some general comments/questions. I also tested the code to see if there was any obvious bias in chosen sockets or random IDs and found none.

[omoerbeek]

If the state table is quite full and/or if you have bad luck this will start reusing IDs. What are the consequences of that? Should that be documented?

[rgacogne]

That already happens in master since we have a fixed number of possible states for in-flight queries, and is tracked in our stats as reused. Basically it means that your query will appear to have had a time out much faster than expected. I do not think it is a real issue for the use case this feature tries to address.

[leshow]

@rgacogne I am curious what value of sockets you use when you enable both these features? Obviously, a higher number would be better for entropy, but starting 2^16 sockets seems likely to cause issues?

[rgacogne]

You are right, I would not advise creating 2^16 sockets (although in theory it should work pretty well) :) I think the exact number depends on the environment you are running in: on a regular server I would advise at least 1000 sockets, which should already make any spoofing attempt significantly harder, and probably 10k if you are really worried about that threat. Although if the network is really hostile I would personally turn to outgoing DoT or DoH, but of course that might not be always possible. On a low-end device 100 sockets should be fine, depending on the memory constraints.

[ckdarby]

@rgacogne Maybe a dumb observation, but this seems like a partial implementation of Recommendations for Transport-Protocol Port Randomization RFC 6056?

Was there any particular objection to having implemented the RFC for UDP? I noticed in this PRs docs the stance seems to be dnsdist doesn't care and operators should just switch to TCP backend only instead.

[rgacogne]

The long story is that dnsdist has been designed as a reverse-proxy, so the path between dnsdist and the backend was supposed to be trusted. We are seeing more and more people using it as a regular proxy these days, and this PR was a step in making that safer, even though we clearly would advise using outgoing DoT or DoH is the network is really an untrusted one.
Still given that enough ports are used, I'm not sure this implementation is less secure than the one described in RFC6056's section 3.3.2, as for every query a port is randomly chosen against all the ones available.
Although it's not true if the initial port selection is biased, which might be true on some OS, so perhaps we should use the random port selection algorithm that we have in the recursor instead of relying on the OS.
Or am I missing something important from RFC 6056 that we do not implement?

[ckdarby]

[Removed to reclarify as I had a misunderstanding that the PR took an actual port range]