Skip to content

/pdns

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

Add support for encrypting IP addresses #gdpr #6242

Closed
wants to merge 19 commits into from

Conversation

Reviewers

@rgacogne rgacogne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@ahupowerdns @rgacogne @pieterlexis
@ahupowerdns
Copy link
Member

commented Feb 2, 2018 •
edited

Short description

With this change, PowerDNS core gains ability to encrypt & decrypt IP addresses as described in https://medium.com/@bert.hubert/on-ip-address-encryption-security-analysis-with-respect-for-privacy-dabe1201b476
For IPv4 this uses ipcrypt, for IPv6 it uses a 128-bit AES ECB operation.
This PR also hooks up ipencrypt() and ipdecrypt() methods for dnsdist use, specifically to pseudonomyse logging.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

ahupowerdns added some commits Feb 2, 2018

@ahupowerdns
Add support for encrypting IP addresses #gdpr 
With this change, PowerDNS core gains ability to encrypt & decrypt IP addresses as described in https://medium.com/@bert.hubert/on-ip-address-encryption-security-analysis-with-respect-for-privacy-dabe1201b476
For IPv4 this uses ipcrypt, for IPv6 it uses a 128-bit AES ECB operation.
This CR also hooks up ipencrypt() and ipdecrypt() methods for dnsdist use, specifically to pseudonomyse logging.
e84e713
@ahupowerdns
add in symlinks for dnsdist
41bfd92
@pieterlexis

This comment has been minimized.

Sign in to view
Copy link

commented on pdns/dnsdistdist/ext/ipcrypt/LICENSE in e84e713 Feb 2, 2018

I'm pretty sure this has to be a relative symlink :)

This comment has been minimized.

Sign in to view
Copy link
Owner Author

replied Feb 2, 2018

fixed, thanks.
@pieterlexis

This comment has been minimized.

Sign in to view
Copy link

commented on pdns/ipcrypt.cc in e84e713 Feb 2, 2018

small comment: we have a ComboAddress::isIPv4() and a ComboAddress::isIPv6() function

This comment has been minimized.

Sign in to view
Copy link
Owner Author

replied Feb 2, 2018

This is close enough to idiom, it will never, ever change.
@pieterlexis

This comment has been minimized.

Sign in to view
Copy link

commented on pdns/Makefile.am in e84e713 Feb 2, 2018

we can also move this to the testrunner_SOURCES only in dnsdistdist/Makefile.am, where it makes more sense to test this

This comment has been minimized.

Sign in to view
Copy link
Owner Author

replied Feb 2, 2018

no it doesn't, this is a PowerDNS feature, not a dnsdist feature. It'll crop up in lots of places. Let us think big!

ahupowerdns added some commits Feb 2, 2018

@ahupowerdns
fix ipcrypt.h in testrunner Makefile
Loading status checks…
1365845
@ahupowerdns
add documentation to dnsdist
Loading status checks…
c0c2503
@ahupowerdns
add key derivatin from password, add ipencrypt/ipdecrypt to pdnsutil … 
…& document it
Loading status checks…
2257ee7
@ahupowerdns
fix up salt for pbkdf2
Loading status checks…
05c79eb
@ahupowerdns
salt was confusing, should be ipcipheripcipher
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
e54e0bd
@rgacogne

rgacogne requested changes Feb 12, 2018
View changes

Copy link
Member

left a comment

LGTM, just one nit regarding dnsdist's linkage.
pdns/dnsdistdist/Makefile.am
@@ -152,14 +152,13 @@ if HAVE_RE2
dnsdist_LDADD += $(RE2_LIBS)
endif

if HAVE_DNS_OVER_TLS

This comment has been minimized.

Sign in to view
Copy link
@rgacogne

rgacogne Feb 12, 2018

Member

This would cause dnsdist to be linked against libgnutls and $(LIBSSL_LIBS) when they are available, regardless of whether DNS over TLS support is enabled. If I'm not mistaken you only need to link against $(LIBCRYPTO_LIBS) for this to work?

This comment has been minimized.

Sign in to view
Copy link
@ahupowerdns

ahupowerdns Feb 16, 2018

Author Member

yes, that is correct, but I did not find a way to make that happen correctly.
pdns/dnsdistdist/configure.ac
@@ -52,10 +52,11 @@ PDNS_CHECK_LUA_HPP
DNSDIST_ENABLE_DNS_OVER_TLS
DNSDIST_CHECK_GNUTLS
DNSDIST_CHECK_LIBSSL
PDNS_CHECK_LIBCRYPTO
AS_IF([test "x$enable_dns_over_tls" != "xno"], [
AS_IF([test "$HAVE_LIBSSL" = "1"], [
# we need libcrypto if libssl is enabled

This comment has been minimized.

Sign in to view
Copy link
@rgacogne

rgacogne Feb 12, 2018

Member

This comment is now useless

@rgacogne rgacogne referenced this pull request Feb 13, 2019

Merged

Add support for encrypting IP addresses #gdpr #7481

6 of 7 tasks complete
@rgacogne

This comment has been minimized.

Sign in to view
Copy link
Member

commented Apr 4, 2019

Superseded by #7481.

@rgacogne rgacogne closed this Apr 4, 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.