From 22020e64ee8b0dd9392481e2e8a45df8cb35dc88 Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Tue, 30 Jan 2024 13:09:42 +1000 Subject: [PATCH 1/4] Add noreply shape check #36 --- classes/check/dnsnoreply.php | 96 ++++++++++++++++++++++++++++++++++++ classes/dns_util.php | 38 ++++++++++++++ lang/en/tool_emailutils.php | 1 + lib.php | 1 + version.php | 4 +- 5 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 classes/check/dnsnoreply.php diff --git a/classes/check/dnsnoreply.php b/classes/check/dnsnoreply.php new file mode 100644 index 0000000..4177ab8 --- /dev/null +++ b/classes/check/dnsnoreply.php @@ -0,0 +1,96 @@ +. +/** + * DNS Email Noreply check. + * + * @package tool_emailutils + * @author Benjamin Walker + * @copyright Catalyst IT 2024 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + */ + +namespace tool_emailutils\check; +use core\check\check; +use core\check\result; +use tool_emailutils\dns_util; + +/** + * DNS Email Noreply check. + * + * @package tool_emailutils + * @author Benjamin Walker + * @copyright Catalyst IT 2024 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class dnsnoreply extends check { + + /** + * A link to a place to action this + * + * @return \action_link|null + */ + public function get_action_link(): ?\action_link { + return new \action_link( + new \moodle_url('/admin/settings.php?section=outgoingmailconfig'), + get_string('outgoingmailconfig', 'core_admin')); + } + + /** + * Get Result. + * + * @return result + */ + public function get_result() : result { + global $DB, $CFG; + + $url = new \moodle_url($CFG->wwwroot); + $domain = $url->get_host(); + + $details = ''; + $status = result::INFO; + $summary = ''; + + $dns = new dns_util(); + + $noreply = $dns->get_noreply(); + $details .= "

No reply email: $noreply

"; + + $noreplydomain = $dns->get_noreply_domain(); + $details .= "

No reply domain: $noreplydomain

"; + + $details .= "

LMS domain: $domain

"; + + $primarydomain = $dns->get_primary_domain($domain); + + if ($noreplydomain == $domain) { + $status = result::OK; + $summary = "LMS is same as noreply domain"; + } else if (str_contains($domain, '.' . $noreplydomain)) { + $status = result::OK; + $summary = "LMS is a subdomain of noreply domain"; + } else if (str_contains($noreplydomain, '.' . $domain)) { + $status = result::OK; + $summary = "Noreply domain is a subdomain of LMS"; + } else if ($noreply == $primarydomain || str_contains($noreplydomain, '.' . $primarydomain)) { + $summary = "LMS and noreply domain have a shared domain"; + } else { + $summary = "LMS and noreply domain have nothing in common"; + } + + return new result($status, $summary, $details); + } +} diff --git a/classes/dns_util.php b/classes/dns_util.php index 64bfdf6..2576989 100644 --- a/classes/dns_util.php +++ b/classes/dns_util.php @@ -58,6 +58,44 @@ public function get_noreply_domain() { return $noreplydomain; } + /** + * Attempts to extract the primary domain from a domain + * + * This may not always be clear, if there is any confusion return the full domain. + * @param string $domain domain to check + * @return string primary domain + */ + public function get_primary_domain($domain) { + $originaldomain = $domain; + + // Checks for the first domain that has a NS record. + while ($domain) { + $records = @dns_get_record($domain, DNS_NS); + if (!empty($records)) { + return $domain; + } + $parts = explode('.', $domain); + // A domain should always have more than 1 part. + if (count($parts) >= 2) { + break; + } + $domain = join('.', array_slice($parts, 1)); + } + return $originaldomain; + } + + /** + * Attempts to extract the subdomains from a domain + * + * This may not always be clear, if there is any confusion return known subdomains or a blank string. + * @param string $domain domain to check + * @return string subdomains + */ + public function get_subdomains($domain) { + $primarydomain = $this->get_primary_domain($domain); + return rtrim(strstr($domain, $primarydomain, true), '.'); + } + /** * Get spf txt record contents * @return string txt record diff --git a/lang/en/tool_emailutils.php b/lang/en/tool_emailutils.php index cbcc362..0d5fc19 100644 --- a/lang/en/tool_emailutils.php +++ b/lang/en/tool_emailutils.php @@ -47,6 +47,7 @@ '; $string['checkdnsmx'] = 'DNS Email MX check'; +$string['checkdnsnoreply'] = 'DNS Email noreply shape check'; $string['dnssettings'] = 'SPF / DKIM / DMARC DNS settings'; $string['dnsspfinclude'] = 'SPF include'; $string['dnsspfinclude_help'] = '

This is an SPF include domain which is expected to be present in the record. For example if this was set to spf.acme.org then the SPF security check would pass if the SPF record was v=spf1 include:spf.acme.org -all.

diff --git a/lib.php b/lib.php index 6e67a1f..dc589dc 100644 --- a/lib.php +++ b/lib.php @@ -46,6 +46,7 @@ function tool_emailutils_security_checks() { new \tool_emailutils\check\dnsdkim(), new \tool_emailutils\check\dnsdmarc(), new \tool_emailutils\check\dnsmx(), + new \tool_emailutils\check\dnsnoreply(), new \tool_emailutils\check\dnspostmastertools(), ]; } diff --git a/version.php b/version.php index 953990e..ba9e0e6 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024011501; -$plugin->release = 2024011501; +$plugin->version = 2024011502; +$plugin->release = 2024011502; $plugin->requires = 2020061500; $plugin->component = 'tool_emailutils'; $plugin->dependencies = [ From 7ce7cc68d36119f1920a2a6ff2e5afaf497b1a58 Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Tue, 30 Jan 2024 13:12:58 +1000 Subject: [PATCH 2/4] Improve default dkim selector #34 --- classes/dns_util.php | 43 ++++++++++++++++++++++++++++++++++++ classes/form/create_dkim.php | 20 ++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/classes/dns_util.php b/classes/dns_util.php index 2576989..61ab94c 100644 --- a/classes/dns_util.php +++ b/classes/dns_util.php @@ -249,5 +249,48 @@ public function get_matching_dns_record($domain, $match) { } return ''; } + + /** + * Gets the selector suffix + * @param string $domain check specific domain + * @return string suffix + */ + public function get_selector_suffix($domain = '') { + GLOBAL $CFG; + + if (empty($domain)) { + $url = new \moodle_url($CFG->wwwroot); + $domain = $url->get_host(); + } + + // Determine the suffix based on the LMS domain and noreply domain. + $primarydomain = $this->get_primary_domain($domain); + $noreplydomain = $this->get_noreply_domain(); + if ($primarydomain == $noreplydomain) { + // Noreply domain is same as primary domain, add all LMS subdomains. + $suffix = $this->get_subdomains($domain); + } else if (str_contains($domain, '.' . $noreplydomain)) { + // Noreply domain includes part of the LMS subdomain, only add different subdomains. + $suffix = str_replace('.' . $noreplydomain, '', $domain); + } else if (str_contains($noreplydomain, '.' . $domain)) { + // Noreply domain is a subdomain of LMS, domain already has all info. + $suffix = ''; + } else if (str_contains($noreplydomain, '.' . $primarydomain)) { + // Noreply domain is a different subdomain of primary domain, add all LMS subdomains. + $suffix = $this->get_subdomains($domain); + } else { + // Noreply domain shares nothing in common with LMS, add entire LMS domain. + $suffix = $domain; + } + + // Clean the suffix to remove www and foreign language chars, and convert '.' to '-'. + // Email filter is enough because domains don't contain the other allowed chars. + $suffix = ltrim($suffix, 'www.'); + $suffix = trim(filter_var($suffix, FILTER_SANITIZE_EMAIL), '.'); + $suffix = str_replace('.', '-', $suffix); + + return $suffix; + } + } diff --git a/classes/form/create_dkim.php b/classes/form/create_dkim.php index 929e61e..1a710c2 100644 --- a/classes/form/create_dkim.php +++ b/classes/form/create_dkim.php @@ -53,7 +53,7 @@ public function definition() { $group[] =& $mform->createElement('text', 'selector', array("size" => 20)); - $selector = \userdate(time(), get_string('selectordefault', 'tool_emailutils')); + $selector = $this->get_default_selector(); $mform->setDefault("selector", $selector); $mform->setType('selector', PARAM_HOST); @@ -74,4 +74,22 @@ public function validation($data, $files) { $errors = parent::validation($data, $files); return $errors; } + + /** + * Gets a selector value to use as a default + * + * @return string default selector + */ + private function get_default_selector() { + // Add date to default. + $selector = \userdate(time(), get_string('selectordefault', 'tool_emailutils')); + + // Add suffix. + $dns = new \tool_emailutils\dns_util(); + if ($suffix = $dns->get_selector_suffix()) { + $selector .= '-' . $suffix; + } + return $selector; + } + } From 71736325f3d4dbf1bc36db67be40adf01c5681cc Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Tue, 30 Jan 2024 13:28:45 +1000 Subject: [PATCH 3/4] CI Checks --- classes/check/dnsmx.php | 4 +++- classes/dns_util.php | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/classes/check/dnsmx.php b/classes/check/dnsmx.php index 5f2d7a3..5058804 100644 --- a/classes/check/dnsmx.php +++ b/classes/check/dnsmx.php @@ -79,7 +79,9 @@ public function get_result() : result { $status = result::ERROR; $summary = "MX DNS record missing"; } else { - $allmxdomains = join('
', array_map(fn($x) => $x['target'] . ' (' . $x['pri'] . ')', $mxdomains)); + $allmxdomains = join('
', array_map(function($x) { + return $x['target'] . ' (' . $x['pri'] . ')'; + }, $mxdomains)); $details .= "

MX record found on domain $noreplydomain pointing to
$allmxdomains

"; $status = result::OK; $summary = "MX record points to " . $mxdomains[0]['target']; diff --git a/classes/dns_util.php b/classes/dns_util.php index 61ab94c..4c311a0 100644 --- a/classes/dns_util.php +++ b/classes/dns_util.php @@ -98,6 +98,7 @@ public function get_subdomains($domain) { /** * Get spf txt record contents + * @param string $domain specify a different domain * @return string txt record */ public function get_spf_record($domain = '') { @@ -134,7 +135,7 @@ public function get_mxtoolbox_spf_url() { * Returns the include if matched * * The include can have a wildcard and this will return the actual matched value. - * @param string include domain + * @param string $include include domain * @return string matched include */ public function include_present(string $include) { @@ -163,6 +164,8 @@ public function get_dkim_selector() { /** * Get DKIM txt record contents + * @param string $selector DKIM selector + * @param string $domain DKIM domain * @return string txt record */ public function get_dkim_dns_domain($selector, $domain) { @@ -171,6 +174,7 @@ public function get_dkim_dns_domain($selector, $domain) { /** * Get DKIM txt record contents + * @param string $selector DKIM selector * @return string txt record */ public function get_dkim_record($selector) { @@ -187,7 +191,7 @@ public function get_dkim_record($selector) { /** * Get DKIM txt record contents - * @return string txt record + * @return array txt record */ public function get_dmarc_dns_record() { $domain = $this->get_noreply_domain(); @@ -215,13 +219,14 @@ public function get_dmarc_dns_record() { /** * Get MX record contents - * @return string txt record + * @param string $domain domain to check + * @return array txt record */ public function get_mx_record($domain) { $records = @dns_get_record($domain, DNS_MX); if (empty($records)) { - return; + return []; } usort($records, function($a, $b) { if ($a['pri'] == $b['pri']) { @@ -234,6 +239,8 @@ public function get_mx_record($domain) { /** * Get matching record contents + * @param string $domain domain to check + * @param string $match search for specific match * @return string txt record */ public function get_matching_dns_record($domain, $match) { From 9443de96a27e0ba1fc035a415bdb2182d4e6e1b5 Mon Sep 17 00:00:00 2001 From: Benjamin Walker Date: Thu, 1 Feb 2024 00:17:07 +1000 Subject: [PATCH 4/4] Add dkim selector suffix tests #34 --- tests/suffix_test.php | 150 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 tests/suffix_test.php diff --git a/tests/suffix_test.php b/tests/suffix_test.php new file mode 100644 index 0000000..63bd31e --- /dev/null +++ b/tests/suffix_test.php @@ -0,0 +1,150 @@ +. + +/** + * Tests for DKIM default suffix. + * + * @package tool_emailutils + * @author Benjamin Walker + * @copyright Catalyst IT 2024 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + */ + +/** + * Tests for DKIM default suffix. + * + * @copyright Catalyst IT 2024 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class tool_emailutils_suffix_test extends advanced_testcase { + + /** + * Test suffix. + * + * @param string $lmsdomain lms domain + * @param string $noreplydomain noreply domain + * @param string $primarydomain primary domain + * @param string $selectorsuffix selector suffix + * @dataProvider dns_comparisons + */ + public function test_suffix(string $lmsdomain, string $noreplydomain, string $primarydomain, string $selectorsuffix) { + $this->resetAfterTest(); + $mock = $this->getMockBuilder('\tool_emailutils\dns_util') + ->setMethods(['get_primary_domain', 'get_noreply_domain']) + ->getMock(); + + $mock->expects($this->any()) + ->method('get_primary_domain') + ->willReturn($primarydomain); + + $mock->expects($this->any()) + ->method('get_noreply_domain') + ->willReturn($noreplydomain); + + $selector = $mock->get_selector_suffix($lmsdomain); + $this->assertEquals($selectorsuffix, $selector); + } + + /** + * Data provider used to test comparisons between different domains. + * + * @return array + */ + public function dns_comparisons() { + return [ + 'no subdomain' => [ + 'client.com', + 'client.com', + 'client.com', + '', + ], + 'subdomain' => [ + 'lms.client.com', + 'client.com', + 'client.com', + 'lms', + ], + 'another subdomain' => [ + 'moodle.client.com', + 'client.com', + 'client.com', + 'moodle', + ], + 'multiple subdomain' => [ + 'lms.moodle.client.com', + 'client.com', + 'client.com', + 'lms-moodle', + ], + 'longer tld' => [ + 'lms.moodle.client.nsw.gov.au', + 'client.nsw.gov.au', + 'client.nsw.gov.au', + 'lms-moodle', + ], + 'www only subdomain' => [ + 'www.client.com', + 'client.com', + 'client.com', + '', + ], + 'www multiple subdomain' => [ + 'www.moodle.client.com', + 'client.com', + 'client.com', + 'moodle', + ], + 'different subdomain' => [ + 'lms.client.com', + 'mail.client.com', + 'client.com', + 'lms', + ], + 'noreply contains part of subdomain' => [ + 'lms.moodle.client.com', + 'moodle.client.com', + 'client.com', + 'lms', + ], + 'noreply subdomain of lms' => [ + 'lms.moodle.client.com', + 'email.lms.moodle.client.com', + 'client.com', + '', + ], + 'unable to identify primary domain' => [ + 'lms.moodle.client.com', + 'client.com', + 'lms.moodle.client.com', + 'lms-moodle', + ], + 'different noreply' => [ + 'lms.moodle.client.com', + 'vendor.com', + 'client.com', + 'lms-moodle-client-com', + ], + 'different noreply with no subdomain' => [ + 'client.com', + 'vendor.com', + 'client.com', + 'client-com', + ], + ]; + } + +}