From ec9159fd6986c32a9946041dab63a11d81657b2f Mon Sep 17 00:00:00 2001 From: Andrew Ruthven Date: Sat, 30 Mar 2024 20:11:15 +1300 Subject: [PATCH] Fix some warnings about using undefined keys, simplify dns strings Fixes: - PHP Warning: Trying to access array offset on value of type null in inc/iSchedule.php on line 83 - PHP Warning: Undefined array key "t" in inc/iSchedule.php on line 165 - PHP Warning: Undefined array key "t" in inc/iSchedule.php on line 167 - PHP Warning: Undefined array key "p" in inc/iSchedule.php on line 184 Concatenate the DNS entry we're going to look up once rather than everytime we need it. --- ChangeLog | 3 +++ inc/iSchedule.php | 43 ++++++++++++++++++++++++++++++------------- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2b22d0e8..fc493620 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +2024-03-30 Andrew Ruthven + * Many more changes for PHP 8.2 support. + 2024-03-10 Andrew Ruthven * On a fatal error, include the Request ID which is now in the log file to aid investigations. diff --git a/inc/iSchedule.php b/inc/iSchedule.php index 961f7e0b..a24cfff1 100644 --- a/inc/iSchedule.php +++ b/inc/iSchedule.php @@ -80,13 +80,14 @@ class iSchedule { global $icfg; // TODO handle parents of subdomains and procuration records - if ( $icfg [ $this->remote_selector . '._domainkey.' . $this->remote_server ] ) - { - $this->dk = $icfg [ $this->remote_selector . '._domainkey.' . $this->remote_server ]; + $dns_str = $this->remote_selector . '._domainkey.' . $this->remote_server; + + if ( isset($icfg) && isset($icfg[$dns_str]) ) { + $this->dk = $icfg[$dns_str]; return true; } - $dkim = dns_get_record ( $this->remote_selector . '._domainkey.' . $this->remote_server , DNS_TXT ); + $dkim = dns_get_record($dns_str, DNS_TXT); if ( count ( $dkim ) > 0 ) { $this->dk = $dkim [ 0 ] [ 'txt' ]; @@ -156,17 +157,18 @@ class iSchedule dbg_error_log( 'ischedule', 'validateKey ERROR: bad key algorythm, algo was:' . $this->parsed [ 'k' ] ); return false; // we only speak rsa for now } - if ( isset ( $this->parsed [ 't' ] ) && ! preg_match ( '/^[y:s]+$/', $this->parsed [ 't' ] ) ) { - dbg_error_log( 'ischedule', 'validateKey ERROR: type mismatch' ); - return false; - } - else - { + + if (isset($this->parsed['t'])) { + if ( ! preg_match ( '/^[y:s]+$/', $this->parsed [ 't' ] ) ) { + dbg_error_log( 'ischedule', 'validateKey ERROR: type mismatch' ); + return false; + } if ( preg_match ( '/y/', $this->parsed [ 't' ] ) ) $this->failOnError = false; if ( preg_match ( '/s/', $this->parsed [ 't' ] ) ) $this->subdomainsOK = false; } + if ( isset ( $this->parsed [ 'g' ] ) ) $this->remote_user_rule = $this->parsed [ 'g' ]; else @@ -181,7 +183,7 @@ class iSchedule $this->remote_public_key = $data; } else { - dbg_error_log( 'ischedule', 'validateKey ERROR: no key in dns record' . $this->parsed [ 'p' ] ); + dbg_error_log( 'ischedule', 'validateKey ERROR: no key in dns record' ); return false; } $this->failed = false; @@ -511,31 +513,39 @@ class iSchedule $this->failed = true; $tags = preg_split ( '/;[\s\t]/', $sig ); + foreach ( $tags as $v ) { list($key,$value) = preg_split ( '/=/', $v, 2 ); $dkim[$key] = $value; } + // the canonicalization method is currently undefined as of draft-01 of the iSchedule spec // but it does define the value, it should be simple-http. RFC4871 also defines two methods // simple and relaxed, simple is probably the same as simple http // relaxed allows for header case folding and whitespace folding, see section 3.4.4 of RFC4871 if ( ! preg_match ( '{(simple|simple-http|relaxed)(/(simple|simple-http|relaxed))?}', $dkim['c'], $matches ) ) // canonicalization method return 'bad canonicalization:' . $dkim['c'] ; + if ( count ( $matches ) > 2 ) $this->body_cannon = $matches[2]; else $this->body_cannon = $matches[1]; + $this->header_cannon = $matches[1]; - // signing algorythm REQUIRED + + // signing algorithm REQUIRED if ( $dkim['a'] != 'rsa-sha1' && $dkim['a'] != 'rsa-sha256' ) // we only support the minimum required - return 'bad signing algorythm:' . $dkim['a'] ; + return 'bad signing algorithm:' . $dkim['a'] ; + // query method to retrieve public key, could/should we add https to the spec? REQUIRED if ( $dkim['q'] != 'dns/txt' ) return 'bad query method'; + // domain of the signing entity REQUIRED if ( ! isset ( $dkim['d'] ) ) return 'missing signing domain'; + $this->remote_server = $dkim['d']; // identity of signing AGENT, OPTIONAL if ( isset ( $dkim['i'] ) ) @@ -547,10 +557,12 @@ class iSchedule if ( strstr ( $dkim [ 'i' ], '@' ) ) $this->remote_user = substr ( $dkim [ 'i' ], 0, strpos ( $dkim [ 'i' ], '@' ) - 1 ); } + // selector used to retrieve public key REQUIRED if ( ! isset ( $dkim['s'] ) ) return 'missing selector'; $this->remote_selector = $dkim['s']; + // signed header fields, colon seperated REQUIRED if ( ! isset ( $dkim['h'] ) ) return 'missing list of signed headers'; @@ -563,18 +575,23 @@ class iSchedule if ( in_array ( strtolower ( $h ), $this->disallowed_headers ) ) return "$h is NOT allowed in signed header fields per RFC4871 or iSchedule"; } + foreach ( $this->required_headers as $h ) if ( ! in_array ( strtolower ( $h ), $sh ) ) return "$h is REQUIRED but missing in signed header fields per iSchedule"; + // body hash REQUIRED if ( ! isset ( $dkim['bh'] ) ) return 'missing body signature'; + // signed header hash REQUIRED if ( ! isset ( $dkim['b'] ) ) return 'missing signature in b field'; + // length of body used for signing if ( isset ( $dkim['l'] ) ) $this->signed_length = $dkim['l']; + $this->failed = false; $this->DKSig = $dkim; return true;