Skip to content

Normalise DNS qnames to lowercase on the database write path #772

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.

Sign up for GitHub

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

Merged
kasnder merged 1 commit into from
Aug 22, 2026
Merged

Normalise DNS qnames to lowercase on the database write path #772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions app/src/main/java/eu/faircode/netguard/DatabaseHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.locks.ReentrantReadWriteLock;

Expand Down Expand Up @@ -84,7 +85,7 @@ public Cursor getHosts() {
private static final String TAG = "TrackerControl.Database";

private static final String DB_NAME = "Netguard";
private static final int DB_VERSION = 22;
private static final int DB_VERSION = 23;

private static boolean once = true;
private static List<LogChangedListener> logChangedListeners = new ArrayList<>();
Expand Down Expand Up @@ -447,6 +448,23 @@ public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
db.execSQL("ALTER TABLE access ADD COLUMN uncertain INTEGER");
oldVersion = 22;
}
if (oldVersion < 23) {
// Remove case-variant duplicates before lowercasing because idx_dns is UNIQUE.
// Keep the freshest row for each DNS identity, breaking time ties by ID.
db.execSQL("DELETE FROM dns WHERE ID NOT IN (" +
"SELECT MAX(d.ID) FROM dns d" +
" JOIN (" +
"SELECT lower(qname) AS qname, lower(aname) AS aname, resource, MAX(time) AS time" +
" FROM dns" +
" GROUP BY lower(qname), lower(aname), resource" +
") latest ON lower(d.qname) = latest.qname" +
" AND lower(d.aname) = latest.aname" +
" AND d.resource = latest.resource" +
" AND d.time = latest.time" +
" GROUP BY latest.qname, latest.aname, latest.resource)");
db.execSQL("UPDATE dns SET qname = lower(qname), aname = lower(aname)");
oldVersion = 23;
}

if (oldVersion == DB_VERSION) {
db.setVersion(oldVersion);
Expand Down Expand Up @@ -935,7 +953,7 @@ public Cursor getAccess(int uid) {
// There is a segmented index on uid
// There is no index on time for write performance
String query = "SELECT a.ID AS _id, a.*";
query += ", (SELECT COUNT(DISTINCT d.qname) FROM dns d WHERE d.resource IN (SELECT d1.resource FROM dns d1 WHERE d1.qname = a.daddr)) count";
query += ", (SELECT COUNT(DISTINCT d.qname) FROM dns d WHERE d.resource IN (SELECT d1.resource FROM dns d1 WHERE d1.qname = lower(a.daddr))) count";
query += " FROM access a";
query += " WHERE a.uid = ?";
query += " ORDER BY a.time DESC";
Expand Down Expand Up @@ -1053,6 +1071,10 @@ public Cursor getRecentTrackerActivity() {

// DNS

private static String lower(String name) {
return (name == null ? null : name.toLowerCase(Locale.ROOT));
}

public boolean insertDns(ResourceRecord rr) {
lock.writeLock().lock();
try {
Expand All @@ -1069,12 +1091,21 @@ public boolean insertDns(ResourceRecord rr) {
cv.put("time", rr.Time);
cv.put("ttl", ttl * 1000L);

// DNS names are case-insensitive, but the tracker and hosts lookups
// are keyed in lowercase (see TrackerList.findTracker). Storing the
// wire case verbatim made a server-chosen CNAME case — or a resolver
// using 0x20 randomisation — a distinct row under the BINARY-collated
// idx_dns, splitting one domain across several rows and making
// getQAName report it as several qnames sharing an IP.
String qname = lower(rr.QName);
String aname = lower(rr.AName);

int rows = db.update("dns", cv, "qname = ? AND aname = ? AND resource = ?",
new String[] { rr.QName, rr.AName, rr.Resource });
new String[] { qname, aname, rr.Resource });

if (rows == 0) {
cv.put("qname", rr.QName);
cv.put("aname", rr.AName);
cv.put("qname", qname);
cv.put("aname", aname);
cv.put("resource", rr.Resource);

if (db.insert("dns", null, cv) == -1)
Expand Down Expand Up @@ -1199,6 +1230,7 @@ public Cursor getAlternateQNames(String qname) {
lock.readLock().lock();
try {
SQLiteDatabase db = this.getReadableDatabase();
qname = lower(qname);
String query = "SELECT DISTINCT d2.qname";
query += " FROM dns d1";
query += " JOIN dns d2";
Expand All @@ -1216,6 +1248,7 @@ public Cursor getAName(String qname, boolean alive) {
lock.readLock().lock();
try {
SQLiteDatabase db = this.getReadableDatabase();
qname = lower(qname);
String query = "SELECT d.qname, d.aname, d.time, d.ttl";
query += " FROM dns d";
query += " WHERE d.qname = ?";
Expand Down Expand Up @@ -1248,13 +1281,16 @@ public Cursor getAccessDns(String dname) {
lock.readLock().lock();
try {
SQLiteDatabase db = this.getReadableDatabase();
// dns.qname is stored lowercase; access.daddr is written from it, so
// the parameter is matched against the indexed column as-is.
dname = lower(dname);

// There is a segmented index on dns.qname
// There is an index on access.daddr and access.block
String query = "SELECT a.uid, a.version, a.protocol, a.daddr, d.resource, a.dport, a.block, d.time, d.ttl";
query += " FROM access AS a";
query += " LEFT JOIN dns AS d";
query += " ON d.qname = a.daddr";
query += " ON d.qname = lower(a.daddr)";
query += " WHERE a.block >= 0";
query += " AND (d.time IS NULL OR d.time + d.ttl >= " + now + ")";
if (dname != null)
Expand Down
18 changes: 18 additions & 0 deletions app/src/test/java/eu/faircode/netguard/DatabaseHelperDnsAttributionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,24 @@ public void repeatedObservationsOfSameQnameCollapseToFreshestRow() {
}
}

@Test
public void caseVariantQnamesShareOneRowAndDoNotLookUncertain() {
DatabaseHelper dh = DatabaseHelper.getInstance(RuntimeEnvironment.getApplication());
dh.clearDns();

String ip = "203.0.113.25";
dh.insertDns(rr(1_000L, "Graph.Facebook.Com", "Alias.Example.Com", ip, 3600));
dh.insertDns(rr(5_000L, "graph.facebook.com", "alias.example.com", ip, 3600));

try (Cursor c = dh.getQAName(-1, ip, false)) {
assertEquals("case variants must share one stored qname", 1, c.getCount());
assertTrue(c.moveToFirst());
assertEquals("graph.facebook.com", c.getString(c.getColumnIndexOrThrow("qname")));
assertEquals("alias.example.com", c.getString(c.getColumnIndexOrThrow("aname")));
assertEquals(5_000L, c.getLong(c.getColumnIndexOrThrow("time")));
}
}

@Test
public void aliveFilterAppliesBeforeDedup() {
DatabaseHelper dh = DatabaseHelper.getInstance(RuntimeEnvironment.getApplication());
Expand Down
37 changes: 33 additions & 4 deletions app/src/test/java/eu/faircode/netguard/DatabaseHelperMigrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,18 @@ public void upgradeFrom21AddsUncertainWithoutLosingAccessRows() {
database.execSQL("ALTER TABLE access ADD COLUMN sent INTEGER");
database.execSQL("ALTER TABLE access ADD COLUMN received INTEGER");
database.execSQL("ALTER TABLE access ADD COLUMN connections INTEGER");
database.execSQL("CREATE TABLE dns ("
+ "ID INTEGER PRIMARY KEY AUTOINCREMENT, time INTEGER NOT NULL, "
+ "qname TEXT NOT NULL, aname TEXT NOT NULL, resource TEXT NOT NULL, ttl INTEGER)");
database.execSQL("CREATE UNIQUE INDEX idx_dns ON dns(qname, aname, resource)");
database.execSQL("INSERT INTO access "
+ "(uid, version, protocol, daddr, dport, time, allowed, block, sent, received, connections) "
+ "VALUES (1001, 4, 6, 'tracker.example', 443, 123456, 0, 1, 10, 20, 3)");
database.setVersion(21);

helper.onUpgrade(database, 21, 22);
helper.onUpgrade(database, 21, 23);

assertEquals(22, database.getVersion());
assertEquals(23, database.getVersion());
assertTrue(columnExists("access", "uncertain"));
try (Cursor cursor = database.rawQuery("SELECT * FROM access WHERE uid = 1001", null)) {
assertTrue(cursor.moveToFirst());
Expand Down Expand Up @@ -70,9 +74,9 @@ public void upgradeFrom16PreservesDataAndBuildsCurrentSchema() {
+ "VALUES (654321, 'example.org', 'alias.example.org', '1.2.3.4', 60)");
database.setVersion(16);

helper.onUpgrade(database, 16, 22);
helper.onUpgrade(database, 16, 23);

assertEquals(22, database.getVersion());
assertEquals(23, database.getVersion());
assertTrue(columnExists("access", "sent"));
assertTrue(columnExists("access", "received"));
assertTrue(columnExists("access", "connections"));
Expand All @@ -96,6 +100,31 @@ public void upgradeFrom16PreservesDataAndBuildsCurrentSchema() {
}
}

@Test
public void upgradeFrom22NormalizesAndDeduplicatesDns() {
database.execSQL("CREATE TABLE dns ("
+ "ID INTEGER PRIMARY KEY AUTOINCREMENT, time INTEGER NOT NULL, "
+ "qname TEXT NOT NULL, aname TEXT NOT NULL, resource TEXT NOT NULL, ttl INTEGER)");
database.execSQL("CREATE UNIQUE INDEX idx_dns ON dns(qname, aname, resource)");
database.execSQL("INSERT INTO dns (time, qname, aname, resource, ttl) "
+ "VALUES (1000, 'Graph.Facebook.Com', 'Alias.Example.Com', '203.0.113.25', 60)");
database.execSQL("INSERT INTO dns (time, qname, aname, resource, ttl) "
+ "VALUES (5000, 'graph.facebook.com', 'alias.example.com', '203.0.113.25', 60)");
database.setVersion(22);

helper.onUpgrade(database, 22, 23);

assertEquals(23, database.getVersion());
try (Cursor cursor = database.rawQuery("SELECT qname, aname, resource, time FROM dns", null)) {
assertEquals(1, cursor.getCount());
assertTrue(cursor.moveToFirst());
assertEquals("graph.facebook.com", cursor.getString(0));
assertEquals("alias.example.com", cursor.getString(1));
assertEquals("203.0.113.25", cursor.getString(2));
assertEquals(5000L, cursor.getLong(3));
}
}

private static void createVersion16AccessTable(SQLiteDatabase db) {
db.execSQL("CREATE TABLE access ("
+ "ID INTEGER PRIMARY KEY AUTOINCREMENT, uid INTEGER NOT NULL, "
Expand Down