From 18ea5e0ad21f75f3098c51091225c68945fe58e4 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Tue, 30 Jul 2019 17:49:17 -0400 Subject: [PATCH 1/2] Add domain_audit repository Signed-off-by: Mcat12 --- src/services/domain_audit/mod.rs | 13 ++ src/services/domain_audit/repository.rs | 181 ++++++++++++++++++++++++ src/services/mod.rs | 1 + test/gravity.sql | 3 + 4 files changed, 198 insertions(+) create mode 100644 src/services/domain_audit/mod.rs create mode 100644 src/services/domain_audit/repository.rs diff --git a/src/services/domain_audit/mod.rs b/src/services/domain_audit/mod.rs new file mode 100644 index 00000000..6b0bcbe0 --- /dev/null +++ b/src/services/domain_audit/mod.rs @@ -0,0 +1,13 @@ +// Pi-hole: A black hole for Internet advertisements +// (c) 2019 Pi-hole, LLC (https://pi-hole.net) +// Network-wide ad blocking via your own hardware. +// +// API +// Domain Audit Log Services +// +// This file is copyright under the latest version of the EUPL. +// Please see LICENSE file for your rights under this license. + +mod repository; + +pub use self::repository::*; diff --git a/src/services/domain_audit/repository.rs b/src/services/domain_audit/repository.rs new file mode 100644 index 00000000..59082cda --- /dev/null +++ b/src/services/domain_audit/repository.rs @@ -0,0 +1,181 @@ +// Pi-hole: A black hole for Internet advertisements +// (c) 2019 Pi-hole, LLC (https://pi-hole.net) +// Network-wide ad blocking via your own hardware. +// +// API +// Domain Audit Log Database Repository +// +// This file is copyright under the latest version of the EUPL. +// Please see LICENSE file for your rights under this license. + +use crate::{ + databases::gravity::GravityDatabase, + util::{Error, ErrorKind} +}; +use diesel::{expression::exists::exists, insert_into, prelude::*, select}; +use failure::ResultExt; +use rocket::{ + request::{self, FromRequest}, + Outcome, Request +}; +use std::marker::PhantomData; + +#[cfg(test)] +use mock_it::Mock; + +/// Describes interactions with the domain audit data store +pub trait DomainAuditRepository { + /// Check if the domain is contained in the audit table + fn contains(&self, domain: &str) -> Result; + + /// Get all audited domains + fn get_all(&self) -> Result, Error>; + + /// Add a domain to the audit table + fn add(&self, domain: &str) -> Result<(), Error>; +} + +service!( + DomainAuditRepositoryGuard, + DomainAuditRepository, + DomainAuditRepositoryImpl, + DomainAuditRepositoryMock +); + +/// The implementation of `DomainAuditRepository` +pub struct DomainAuditRepositoryImpl<'r> { + db: GravityDatabase, + phantom: PhantomData<&'r ()> +} + +impl<'r> DomainAuditRepositoryImpl<'r> { + fn new(db: GravityDatabase) -> Self { + DomainAuditRepositoryImpl { + db, + phantom: PhantomData + } + } +} + +impl<'a, 'r> FromRequest<'a, 'r> for DomainAuditRepositoryImpl<'r> { + type Error = (); + + fn from_request(request: &'a Request<'r>) -> request::Outcome { + let db = request.guard::()?; + Outcome::Success(DomainAuditRepositoryImpl::new(db)) + } +} + +impl<'r> DomainAuditRepository for DomainAuditRepositoryImpl<'r> { + fn contains(&self, input_domain: &str) -> Result { + use crate::databases::gravity::domain_audit::dsl::*; + let db = &self.db as &SqliteConnection; + + select(exists(domain_audit.filter(domain.eq(input_domain)))) + .get_result(db) + .context(ErrorKind::GravityDatabase) + .map_err(Error::from) + } + + fn get_all(&self) -> Result, Error> { + use crate::databases::gravity::domain_audit::dsl::*; + let db = &self.db as &SqliteConnection; + + domain_audit + .select(domain) + .load(db) + .context(ErrorKind::GravityDatabase) + .map_err(Error::from) + } + + fn add(&self, input_domain: &str) -> Result<(), Error> { + use crate::databases::gravity::domain_audit::dsl::*; + let db = &self.db as &SqliteConnection; + + insert_into(domain_audit) + .values(domain.eq(input_domain)) + .execute(db) + .context(ErrorKind::GravityDatabase)?; + + Ok(()) + } +} + +#[cfg(test)] +pub struct DomainAuditRepositoryMock { + pub contains: Mock>, + pub get_all: Mock<(), Result, Error>>, + pub add: Mock> +} + +#[cfg(test)] +impl Default for DomainAuditRepositoryMock { + fn default() -> Self { + DomainAuditRepositoryMock { + contains: Mock::new(Ok(false)), + get_all: Mock::new(Ok(Vec::new())), + add: Mock::new(Ok(())) + } + } +} + +#[cfg(test)] +impl DomainAuditRepository for DomainAuditRepositoryMock { + fn contains(&self, domain: &str) -> Result { + self.contains.called(domain.to_owned()) + } + + fn get_all(&self) -> Result, Error> { + self.get_all.called(()) + } + + fn add(&self, domain: &str) -> Result<(), Error> { + self.add.called(domain.to_owned()) + } +} + +#[cfg(test)] +mod tests { + use crate::{ + databases::gravity::connect_to_gravity_test_db, + services::domain_audit::{DomainAuditRepository, DomainAuditRepositoryImpl} + }; + + /// If the audit table contains the domain, true will be returned + #[test] + fn contains_true() { + let db = connect_to_gravity_test_db(); + let repo = DomainAuditRepositoryImpl::new(db); + + assert_eq!(repo.contains("audited.domain").unwrap(), true); + } + + /// If the audit table does not contain the domain, false will be returned + #[test] + fn contains_false() { + let db = connect_to_gravity_test_db(); + let repo = DomainAuditRepositoryImpl::new(db); + + assert_eq!(repo.contains("not.audited.domain").unwrap(), false); + } + + /// All audited domains are retrieved + #[test] + fn get_all() { + let db = connect_to_gravity_test_db(); + let repo = DomainAuditRepositoryImpl::new(db); + + assert_eq!(repo.get_all().unwrap(), vec!["audited.domain".to_owned()]); + } + + /// After adding, the database will contain the domain + #[test] + fn add_success() { + let db = connect_to_gravity_test_db(); + let repo = DomainAuditRepositoryImpl::new(db); + + repo.add("new.audited.domain").unwrap(); + + assert_eq!(repo.contains("new.audited.domain").unwrap(), true); + } +} diff --git a/src/services/mod.rs b/src/services/mod.rs index 551f47ad..5d547203 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -11,6 +11,7 @@ #[macro_use] mod service; +pub mod domain_audit; pub mod lists; pub use self::service::*; diff --git a/test/gravity.sql b/test/gravity.sql index 3a637b82..96606c2c 100644 --- a/test/gravity.sql +++ b/test/gravity.sql @@ -205,3 +205,6 @@ VALUES ('test.com'), ('vra.outbrain.com'), ('vra4.com'), ('vriaj.com'); + +INSERT INTO domain_audit (domain) +VALUES ('audited.domain'); From 774201911939c14d03c8ea1f05280bb1c7d5f1f9 Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Tue, 30 Jul 2019 18:02:24 -0400 Subject: [PATCH 2/2] Use the domain_audit table for top domain audit filter The audit log file is no longer used and has been removed. Signed-off-by: Mcat12 --- src/env/config/file_locations.rs | 6 -- src/env/file.rs | 2 - src/ftl/memory_model/domain.rs | 8 +- src/routes/stats/database/top_domains_db.rs | 97 ++++++++++++++++----- src/routes/stats/top_domains.rs | 40 ++++++--- 5 files changed, 110 insertions(+), 43 deletions(-) diff --git a/src/env/config/file_locations.rs b/src/env/config/file_locations.rs index 926ecfa9..62d29601 100644 --- a/src/env/config/file_locations.rs +++ b/src/env/config/file_locations.rs @@ -31,8 +31,6 @@ pub struct Files { local_versions: String, #[serde(default = "default_local_branches")] local_branches: String, - #[serde(default = "default_audit_log")] - audit_log: String, #[serde(default = "default_gravity")] gravity: String, #[serde(default = "default_gravity_backup")] @@ -54,7 +52,6 @@ impl Default for Files { ftl_config: default_ftl_config(), local_versions: default_local_versions(), local_branches: default_local_branches(), - audit_log: default_audit_log(), gravity: default_gravity(), gravity_backup: default_gravity_backup(), black_list: default_black_list(), @@ -74,7 +71,6 @@ impl Files { &self.ftl_config, &self.local_versions, &self.local_branches, - &self.audit_log, &self.gravity, &self.gravity_backup, &self.black_list, @@ -95,7 +91,6 @@ impl Files { PiholeFile::FtlConfig => &self.ftl_config, PiholeFile::LocalVersions => &self.local_versions, PiholeFile::LocalBranches => &self.local_branches, - PiholeFile::AuditLog => &self.audit_log, PiholeFile::Gravity => &self.gravity, PiholeFile::GravityBackup => &self.gravity_backup, PiholeFile::BlackList => &self.black_list, @@ -121,7 +116,6 @@ default!(default_setup_vars, SetupVars); default!(default_ftl_config, FtlConfig); default!(default_local_versions, LocalVersions); default!(default_local_branches, LocalBranches); -default!(default_audit_log, AuditLog); default!(default_gravity, Gravity); default!(default_gravity_backup, GravityBackup); default!(default_black_list, BlackList); diff --git a/src/env/file.rs b/src/env/file.rs index 47ce2be9..15b70853 100644 --- a/src/env/file.rs +++ b/src/env/file.rs @@ -19,7 +19,6 @@ pub enum PiholeFile { FtlConfig, LocalVersions, LocalBranches, - AuditLog, Gravity, GravityBackup, BlackList, @@ -38,7 +37,6 @@ impl PiholeFile { PiholeFile::FtlConfig => "/etc/pihole/pihole-FTL.conf", PiholeFile::LocalVersions => "/etc/pihole/localversions", PiholeFile::LocalBranches => "/etc/pihole/localbranches", - PiholeFile::AuditLog => "/etc/pihole/auditlog.list", PiholeFile::Gravity => "/etc/pihole/gravity.list", PiholeFile::GravityBackup => "/etc/pihole/gravity.list.bck", PiholeFile::BlackList => "/etc/pihole/black.list", diff --git a/src/ftl/memory_model/domain.rs b/src/ftl/memory_model/domain.rs index b23cf4ab..c852c154 100644 --- a/src/ftl/memory_model/domain.rs +++ b/src/ftl/memory_model/domain.rs @@ -16,8 +16,8 @@ use crate::ftl::memory_model::MAGIC_BYTE; /// The domain struct stored in shared memory #[repr(C)] -#[cfg_attr(test, derive(PartialEq, Debug))] -#[derive(Copy, Clone)] +#[cfg_attr(test, derive(Debug))] +#[derive(Copy, Clone, Eq, PartialEq, Hash)] pub struct FtlDomain { magic: libc::c_uchar, pub regex_match: FtlRegexMatch, @@ -67,8 +67,8 @@ impl Default for FtlDomain { /// The regex state a domain can hold. Unknown is the default state, before it /// is checked when a query of the domain comes in. #[repr(u8)] -#[cfg_attr(test, derive(PartialEq, Debug))] -#[derive(Copy, Clone)] +#[cfg_attr(test, derive(Debug))] +#[derive(Copy, Clone, Eq, PartialEq, Hash)] #[allow(dead_code)] pub enum FtlRegexMatch { Unknown, diff --git a/src/routes/stats/database/top_domains_db.rs b/src/routes/stats/database/top_domains_db.rs index c96475ae..95b2b2c0 100644 --- a/src/routes/stats/database/top_domains_db.rs +++ b/src/routes/stats/database/top_domains_db.rs @@ -10,7 +10,7 @@ use crate::{ databases::ftl::FtlDatabase, - env::{Env, PiholeFile}, + env::Env, ftl::BLOCKED_STATUSES, routes::{ auth::User, @@ -23,6 +23,7 @@ use crate::{ top_domains::{TopDomainItemReply, TopDomainParams, TopDomainsReply} } }, + services::domain_audit::{DomainAuditRepository, DomainAuditRepositoryGuard}, util::{reply_result, Error, ErrorKind, Reply} }; use diesel::{dsl::sql, prelude::*, sql_types::BigInt, sqlite::SqliteConnection}; @@ -37,14 +38,16 @@ pub fn top_domains_db( db: FtlDatabase, from: u64, until: u64, - params: Form + params: Form, + domain_audit: DomainAuditRepositoryGuard ) -> Reply { reply_result(top_domains_db_impl( &env, &db as &SqliteConnection, from, until, - params.into_inner() + params.into_inner(), + &*domain_audit )) } @@ -54,7 +57,8 @@ fn top_domains_db_impl( db: &SqliteConnection, from: u64, until: u64, - params: TopDomainParams + params: TopDomainParams, + domain_audit: &dyn DomainAuditRepository ) -> Result { // Resolve the parameters let limit = params.limit.unwrap_or(10); @@ -84,7 +88,7 @@ fn top_domains_db_impl( } // Find domains which should not be considered - let ignored_domains = get_ignored_domains(env, audit)?; + let ignored_domains = get_ignored_domains(env, audit, domain_audit)?; // Fetch the top domains and map into the reply structure let top_domains: Vec = @@ -114,7 +118,11 @@ fn top_domains_db_impl( /// Get the list of domains to ignore. If the audit flag is true, audited /// domains are ignored (only show unaudited domains). -fn get_ignored_domains(env: &Env, audit: bool) -> Result, Error> { +fn get_ignored_domains( + env: &Env, + audit: bool, + domain_audit: &dyn DomainAuditRepository +) -> Result, Error> { // Ignore domains excluded via SetupVars let mut ignored_domains = get_excluded_domains(env)?; @@ -123,7 +131,7 @@ fn get_ignored_domains(env: &Env, audit: bool) -> Result, Error> { // If audit flag is true, only include unaudited domains if audit { - ignored_domains.extend(env.read_file_lines(PiholeFile::AuditLog)?); + ignored_domains.extend(domain_audit.get_all()?); } Ok(ignored_domains) @@ -185,6 +193,7 @@ mod test { databases::ftl::connect_to_ftl_test_db, env::PiholeFile, routes::stats::top_domains::{TopDomainItemReply, TopDomainParams, TopDomainsReply}, + services::domain_audit::DomainAuditRepositoryMock, testing::TestEnvBuilder }; @@ -248,8 +257,15 @@ mod test { .file(PiholeFile::FtlConfig, "") .build(); let params = TopDomainParams::default(); - let actual = - top_domains_db_impl(&env, &db, FROM_TIMESTAMP, UNTIL_TIMESTAMP, params).unwrap(); + let actual = top_domains_db_impl( + &env, + &*db, + FROM_TIMESTAMP, + UNTIL_TIMESTAMP, + params, + &DomainAuditRepositoryMock::default() + ) + .unwrap(); assert_eq!(actual, expected); } @@ -281,8 +297,15 @@ mod test { limit: Some(2), ..TopDomainParams::default() }; - let actual = - top_domains_db_impl(&env, &db, FROM_TIMESTAMP, UNTIL_TIMESTAMP, params).unwrap(); + let actual = top_domains_db_impl( + &env, + &*db, + FROM_TIMESTAMP, + UNTIL_TIMESTAMP, + params, + &DomainAuditRepositoryMock::default() + ) + .unwrap(); assert_eq!(actual, expected); } @@ -307,8 +330,15 @@ mod test { blocked: Some(true), ..TopDomainParams::default() }; - let actual = - top_domains_db_impl(&env, &db, FROM_TIMESTAMP, UNTIL_TIMESTAMP, params).unwrap(); + let actual = top_domains_db_impl( + &env, + &*db, + FROM_TIMESTAMP, + UNTIL_TIMESTAMP, + params, + &DomainAuditRepositoryMock::default() + ) + .unwrap(); assert_eq!(actual, expected); } @@ -342,8 +372,15 @@ mod test { limit: Some(2), ..TopDomainParams::default() }; - let actual = - top_domains_db_impl(&env, &db, FROM_TIMESTAMP, UNTIL_TIMESTAMP, params).unwrap(); + let actual = top_domains_db_impl( + &env, + &*db, + FROM_TIMESTAMP, + UNTIL_TIMESTAMP, + params, + &DomainAuditRepositoryMock::default() + ) + .unwrap(); assert_eq!(actual, expected); } @@ -371,15 +408,27 @@ mod test { let env = TestEnvBuilder::new() .file(PiholeFile::SetupVars, "") .file(PiholeFile::FtlConfig, "") - .file(PiholeFile::AuditLog, "1.ubuntu.pool.ntp.org") .build(); let params = TopDomainParams { audit: Some(true), limit: Some(2), ..TopDomainParams::default() }; - let actual = - top_domains_db_impl(&env, &db, FROM_TIMESTAMP, UNTIL_TIMESTAMP, params).unwrap(); + let domain_audit = DomainAuditRepositoryMock::default(); + domain_audit + .get_all + .given(()) + .will_return(Ok(vec!["1.ubuntu.pool.ntp.org".to_owned()])); + + let actual = top_domains_db_impl( + &env, + &*db, + FROM_TIMESTAMP, + UNTIL_TIMESTAMP, + params, + &domain_audit + ) + .unwrap(); assert_eq!(actual, expected); } @@ -409,15 +458,21 @@ mod test { "API_EXCLUDE_DOMAINS=1.ubuntu.pool.ntp.org" ) .file(PiholeFile::FtlConfig, "") - .file(PiholeFile::AuditLog, "") .build(); let params = TopDomainParams { audit: Some(true), limit: Some(2), ..TopDomainParams::default() }; - let actual = - top_domains_db_impl(&env, &db, FROM_TIMESTAMP, UNTIL_TIMESTAMP, params).unwrap(); + let actual = top_domains_db_impl( + &env, + &*db, + FROM_TIMESTAMP, + UNTIL_TIMESTAMP, + params, + &DomainAuditRepositoryMock::default() + ) + .unwrap(); assert_eq!(actual, expected); } diff --git a/src/routes/stats/top_domains.rs b/src/routes/stats/top_domains.rs index 4a0dce67..9e798d0c 100644 --- a/src/routes/stats/top_domains.rs +++ b/src/routes/stats/top_domains.rs @@ -9,16 +9,18 @@ // Please see LICENSE file for your rights under this license. use crate::{ - env::{Env, PiholeFile}, + env::Env, ftl::{FtlDomain, FtlMemory}, routes::{ auth::User, stats::common::{remove_excluded_domains, remove_hidden_domains} }, + services::domain_audit::{DomainAuditRepository, DomainAuditRepositoryGuard}, settings::{ConfigEntry, FtlConfEntry, FtlPrivacyLevel, SetupVarsEntry}, util::{reply_result, Error, Reply} }; use rocket::{request::Form, State}; +use std::{collections::HashSet, iter::FromIterator}; /// Return the top domains #[get("/stats/top_domains?")] @@ -26,9 +28,15 @@ pub fn top_domains( _auth: User, ftl_memory: State, env: State, - params: Form + params: Form, + domain_audit: DomainAuditRepositoryGuard ) -> Reply { - reply_result(get_top_domains(&ftl_memory, &env, params.into_inner())) + reply_result(get_top_domains( + &ftl_memory, + &env, + params.into_inner(), + &*domain_audit + )) } /// Represents the possible GET parameters for top (blocked) domains requests @@ -63,7 +71,8 @@ pub struct TopDomainItemReply { fn get_top_domains( ftl_memory: &FtlMemory, env: &Env, - params: TopDomainParams + params: TopDomainParams, + domain_audit: &dyn DomainAuditRepository ) -> Result { // Resolve the parameters let limit = params.limit.unwrap_or(10); @@ -116,12 +125,10 @@ fn get_top_domains( // If audit flag is true, only include unaudited domains if audit { - let audited_domains = env.read_file_lines(PiholeFile::AuditLog)?; + let audited_domains: HashSet = domain_audit.get_all().map(HashSet::from_iter)?; - // Get a vector of references to strings, to better compare with the domains - let audited_domains: Vec<&str> = audited_domains.iter().map(String::as_str).collect(); - - domains.retain(|domain| !audited_domains.contains(&domain.get_domain(&strings))); + // Only keep unaudited domains + domains.retain(|domain| !audited_domains.contains(domain.get_domain(&strings))); } // Sort the domains (descending by default) @@ -239,6 +246,7 @@ mod test { use crate::{ env::PiholeFile, ftl::{FtlCounters, FtlDomain, FtlMemory, FtlRegexMatch, FtlSettings}, + services::domain_audit::DomainAuditRepositoryMock, testing::TestBuilder }; use std::collections::HashMap; @@ -282,6 +290,7 @@ mod test { TestBuilder::new() .endpoint("/admin/api/stats/top_domains") .ftl_memory(test_data()) + .mock_service(DomainAuditRepositoryMock::default()) .file(PiholeFile::SetupVars, "") .file(PiholeFile::FtlConfig, "") .expect_json(json!({ @@ -300,6 +309,7 @@ mod test { TestBuilder::new() .endpoint("/admin/api/stats/top_domains?limit=1") .ftl_memory(test_data()) + .mock_service(DomainAuditRepositoryMock::default()) .file(PiholeFile::SetupVars, "") .file(PiholeFile::FtlConfig, "") .expect_json(json!({ @@ -318,6 +328,7 @@ mod test { TestBuilder::new() .endpoint("/admin/api/stats/top_domains?blocked=true") .ftl_memory(test_data()) + .mock_service(DomainAuditRepositoryMock::default()) .file(PiholeFile::SetupVars, "") .file(PiholeFile::FtlConfig, "") .expect_json(json!({ @@ -337,6 +348,7 @@ mod test { TestBuilder::new() .endpoint("/admin/api/stats/top_domains?ascending=true") .ftl_memory(test_data()) + .mock_service(DomainAuditRepositoryMock::default()) .file(PiholeFile::SetupVars, "") .file(PiholeFile::FtlConfig, "") .expect_json(json!({ @@ -353,12 +365,19 @@ mod test { /// audited domains #[test] fn audit() { + let domain_audit = DomainAuditRepositoryMock::default(); + + domain_audit + .get_all + .given(()) + .will_return(Ok(vec!["example.net".to_owned()])); + TestBuilder::new() .endpoint("/admin/api/stats/top_domains?audit=true") .ftl_memory(test_data()) + .mock_service(domain_audit) .file(PiholeFile::SetupVars, "") .file(PiholeFile::FtlConfig, "") - .file(PiholeFile::AuditLog, "example.net") .expect_json(json!({ "top_domains": [ { "domain": "github.com", "count": 20 } @@ -374,6 +393,7 @@ mod test { TestBuilder::new() .endpoint("/admin/api/stats/top_domains") .ftl_memory(test_data()) + .mock_service(DomainAuditRepositoryMock::default()) .file(PiholeFile::SetupVars, "API_EXCLUDE_DOMAINS=example.net") .file(PiholeFile::FtlConfig, "") .expect_json(json!({