v1.1.12, notifycmd improvements

ChangeLog

@@ -1,3 +1,15 @@
+v1.1.12
+----------------------------------------------------------------------------------------------------
+  * less confusing logging when MDNotifyCmd returns a failure exit code
+  * MDNotifyCmd can be configured with arguments to which the managed domain
+    names are appended on invocation
+  * added more test cases for MDNotifyCmd use
+
+v1.1.11
+----------------------------------------------------------------------------------------------------
+  * fixes a Null Dereference when specially crafted requests are sent to the server. Reported
+    by Daniel Caminada <[email protected]>.
+
 v1.1.10
 ----------------------------------------------------------------------------------------------------
   * fixes error in renew window calculation that may lead to mod_md running

configure.ac

@@ -14,7 +14,7 @@
 #
 
 AC_PREREQ([2.69])
-AC_INIT([mod_md], [1.1.10], [[email protected]])
+AC_INIT([mod_md], [1.1.12], [[email protected]])
 
 LT_PREREQ([2.2.6])
 LT_INIT()

mod_md.xcodeproj/project.pbxproj

@@ -171,6 +171,7 @@
 		B2FC60761F59894A005B7D9E /* TODO.md */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = net.daringfireball.markdown; path = TODO.md; sourceTree = SOURCE_ROOT; };
 		B2FC607F1F682781005B7D9E /* mod_ssl_md-2.4.x-v5.diff */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = "mod_ssl_md-2.4.x-v5.diff"; sourceTree = "<group>"; };
 		B2FEEDDF1FFB910D0029E19C /* test_0800_stapling.py */ = {isa = PBXFileReference; lastKnownFileType = text.script.python; path = test_0800_stapling.py; sourceTree = "<group>"; };
+		B2FFC9E920C5776A004A8F58 /* test_0900_notify.py */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.script.python; path = test_0900_notify.py; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXGroup section */
@@ -350,6 +351,7 @@
 				75EDD0811F13BE96003CBD39 /* test_0600_roundtrip.py */,
 				B2B81ED71F17BF3200E35CA3 /* test_0700_auto.py */,
 				B2FEEDDF1FFB910D0029E19C /* test_0800_stapling.py */,
+				B2FFC9E920C5776A004A8F58 /* test_0900_notify.py */,
 				B255730C1EE6D11200E0B132 /* test_base.py */,
 				B2D309F71EC1AD26007BECC8 /* test.ini.in */,
 				B2123B851F2DD7A200267CAF /* unit */,

src/md_version.h

@@ -26,15 +26,15 @@
  * @macro
  * Version number of the md module as c string
  */
-#define MOD_MD_VERSION "1.1.10-git"
+#define MOD_MD_VERSION "1.1.12-git"
 
 /**
  * @macro
  * Numerical representation of the version number of the md module
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_MD_VERSION_NUM 0x01010a
+#define MOD_MD_VERSION_NUM 0x01010c
 
 #define MD_ACME_DEF_URL    "https://acme-v01.api.letsencrypt.org/directory"
 

src/mod_md.c

@@ -823,8 +823,12 @@ static apr_status_t run_watchdog(int state, void *baton, apr_pool_t *ptemp)
                                  wd->mc->notify_cmd, exit_code);
                 }
                 else {
+                    if (APR_EINCOMPLETE == rv && exit_code) {
+                        rv = 0;
+                    }
                     ap_log_error(APLOG_MARK, APLOG_ERR, rv, wd->s, APLOGNO(10109) 
-                                 "executing configured MDNotifyCmd %s", wd->mc->notify_cmd);
+                                 "executing MDNotifyCmd %s returned %d", 
+                                  wd->mc->notify_cmd, exit_code);
                     notified = 0;
                 } 
             }
@@ -1305,7 +1309,8 @@ static int md_http_challenge_pr(request_rec *r)
     int configured;
     apr_status_t rv;
 
-    if (!strncmp(ACME_CHALLENGE_PREFIX, r->parsed_uri.path, sizeof(ACME_CHALLENGE_PREFIX)-1)) {
+    if (r->parsed_uri.path 
+        && !strncmp(ACME_CHALLENGE_PREFIX, r->parsed_uri.path, sizeof(ACME_CHALLENGE_PREFIX)-1)) {
         sc = ap_get_module_config(r->server->module_config, &md_module);
         if (sc && sc->mc) {
             configured = (NULL != md_get_by_domain(sc->mc->mds, r->hostname));
@@ -1371,7 +1376,7 @@ static int md_require_https_maybe(request_rec *r)
     const char *s;
     int status;
 
-    if (opt_ssl_is_https 
+    if (opt_ssl_is_https && r->parsed_uri.path
         && strncmp(WELL_KNOWN_PREFIX, r->parsed_uri.path, sizeof(WELL_KNOWN_PREFIX)-1)) {
 
         sc = ap_get_module_config(r->server->module_config, &md_module);

src/mod_md_config.c

@@ -771,16 +771,16 @@ static const char *md_config_set_pkeys(cmd_parms *cmd, void *dc,
     return apr_pstrcat(cmd->pool, "unsupported private key type \"", ptype, "\"", NULL);
 }
 
-static const char *md_config_set_notify_cmd(cmd_parms *cmd, void *arg, const char *value)
+static const char *md_config_set_notify_cmd(cmd_parms *cmd, void *mconfig, const char *arg)
 {
     md_srv_conf_t *sc = md_config_get(cmd->server);
     const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
 
     if (err) {
         return err;
     }
-    sc->mc->notify_cmd = value;
-    (void)arg;
+    sc->mc->notify_cmd = arg;
+    (void)mconfig;
     return NULL;
 }
 
@@ -837,8 +837,8 @@ const command_rec md_cmds[] = {
                   "Time length for renewal before certificate expires (defaults to days)"),
     AP_INIT_TAKE1(     MD_CMD_REQUIREHTTPS, md_config_set_require_https, NULL, RSRC_CONF, 
                   "Redirect non-secure requests to the https: equivalent."),
-    AP_INIT_TAKE1(     MD_CMD_NOTIFYCMD, md_config_set_notify_cmd, NULL, RSRC_CONF, 
-                  "set the command to run when signup/renew of domain is complete."),
+    AP_INIT_RAW_ARGS(MD_CMD_NOTIFYCMD, md_config_set_notify_cmd, NULL, RSRC_CONF, 
+                  "set the command and optional arguments to run when signup/renew of domain is complete."),
     AP_INIT_TAKE1(     MD_CMD_BASE_SERVER, md_config_set_base_server, NULL, RSRC_CONF, 
                   "allow managing of base server outside virtual hosts."),
 

test/notify.py

@@ -4,8 +4,14 @@
 import sys
 
 def main(argv):
-    sys.stderr.write("%s %s" % (argv[0], argv))
-    sys.exit(7)
+    if len(argv) > 2:
+        f1 = open(argv[1], 'w+')
+        f1.write('%s' % (argv))
+        f1.close()
+        sys.exit(0)
+    else:
+        sys.stderr.write("%s without arguments" % (argv[0]))
+        sys.exit(7)
 
 if __name__ == "__main__":
     main(sys.argv)

test/test_0500_drive.py

@@ -396,6 +396,24 @@ def test_500_111(self):
         assert r['http_status'] == 303
         assert r['http_headers']['Location'] == expLocation
 
+    def test_500_120(self):
+        # test case: NP dereference reported by Daniel Caminada <[email protected]>
+        domain = "test500-120-" + TestDrive.dns_uniq
+        name = "www." + domain
+        conf = HttpdConf( TestDrive.TMP_CONF )
+        conf.add_admin( "admin@" + domain )
+        conf.add_drive_mode( "manual" )
+        conf.add_md( [name] )
+        conf.add_vhost(TestEnv.HTTPS_PORT, name, aliasList=[], withSSL=True)
+        conf.install()
+        assert TestEnv.apache_restart() == 0
+        r = TestEnv.run( [ "openssl", "s_client",  
+              "-connect", "%s:%s" % (TestEnv.HTTPD_HOST, TestEnv.HTTPS_PORT),
+              "-servername", "example.com", "-crlf"
+              ], "GET https:// HTTP/1.1\nHost: example.com\n\n" )
+        assert TestEnv.apache_restart() == 0
+        # assert that no crash is reported in the log
+        assert not TestEnv.apache_err_scan( re.compile("^.* child pid \S+ exit .*$") )
 
     # --------- critical state change -> drive again ---------
 

test/test_0700_auto.py

@@ -425,54 +425,6 @@ def test_7011(self):
         self._check_md_names(domain, dns_list)
         assert TestEnv.await_completion( [ domain ], 10 )
 
-    #-----------------------------------------------------------------------------------------------
-    # test case: signup with configured notify cmd that is invalid
-    #
-    def test_7020(self):
-        domain = ("%s-" % self.test_n) + TestAuto.dns_uniq
-
-        # generate config with two MDs
-        dnsList = [ domain, "www." + domain ]
-        conf = HttpdConf( TestAuto.TMP_CONF )
-        conf.add_admin( "[email protected]" )
-        conf._add_line( "MDNotifyCmd blablabla" )
-        conf.add_drive_mode( "auto" )
-        conf.add_md( dnsList )
-        conf.add_vhost( TestEnv.HTTPS_PORT, domain, aliasList=[ dnsList[1] ], withSSL=True )
-        conf.install()
-
-        # restart, check that md is in store
-        assert TestEnv.apache_restart() == 0
-        self._check_md_names( domain, dnsList )
-        # await drive completion
-        assert TestEnv.await_completion( [ domain ], 30 )
-        self._check_md_cert(dnsList)
-        # this command should have failed and logged an error
-        assert (1, 0) == TestEnv.apache_err_total()
-
-    def test_7021(self):
-        domain = ("%s-" % self.test_n) + TestAuto.dns_uniq
-
-        # generate config with two MDs
-        dnsList = [ domain, "www." + domain ]
-        conf = HttpdConf( TestAuto.TMP_CONF )
-        conf.add_admin( "[email protected]" )
-        conf._add_line( "MDNotifyCmd %s/notify.py" % TestEnv.TESTROOT )
-        conf.add_drive_mode( "auto" )
-        conf.add_md( dnsList )
-        conf.add_vhost( TestEnv.HTTPS_PORT, domain, aliasList=[ dnsList[1] ], withSSL=True )
-        conf.install()
-
-        # restart, check that md is in store
-        assert TestEnv.apache_restart() == 0
-        self._check_md_names( domain, dnsList )
-        # await drive completion
-        assert TestEnv.await_completion( [ domain ], 30 )
-        self._check_md_cert(dnsList)
-        # this command should have failed and logged an error
-        TestEnv.apachectl_stderr = None
-        assert (0, 0) == TestEnv.apache_err_total()
-
     #-----------------------------------------------------------------------------------------------
     # test case: one MD with several dns names. sign up. remove the *first* name
     # in the MD. restart. should find and keep the existing MD.

test/test_0900_notify.py

@@ -0,0 +1,128 @@
+# test mod_md notify support
+
+import json
+import os
+import pytest
+import re
+import socket
+import ssl
+import sys
+import time
+
+from datetime import datetime
+from httplib import HTTPSConnection
+from test_base import TestEnv
+from test_base import HttpdConf
+from test_base import CertUtil
+
+
+def setup_module(module):
+    print("setup_module    module:%s" % module.__name__)
+    TestEnv.init()
+    TestEnv.APACHE_CONF_SRC = "data/test_auto"
+    TestEnv.check_acme()
+    TestEnv.clear_store()
+    TestEnv.install_test_conf();
+    assert TestEnv.apache_start() == 0
+
+
+def teardown_module(module):
+    print("teardown_module module:%s" % module.__name__)
+    assert TestEnv.apache_stop() == 0
+
+
+class TestAuto:
+
+    @classmethod
+    def setup_class(cls):
+        time.sleep(1)
+        cls.dns_uniq = "%d.org" % time.time()
+        cls.TMP_CONF = os.path.join(TestEnv.GEN_DIR, "auto.conf")
+
+
+    def setup_method(self, method):
+        print("setup_method: %s" % method.__name__)
+        TestEnv.apache_err_reset();
+        TestEnv.clear_store()
+        TestEnv.install_test_conf();
+        self.test_n = re.match("test_(.+)", method.__name__).group(1)
+        self.test_domain =  ("%s-" % self.test_n) + TestAuto.dns_uniq
+
+    def teardown_method(self, method):
+        print("teardown_method: %s" % method.__name__)
+
+    #-----------------------------------------------------------------------------------------------
+    # MD host with notifcy command
+    # 
+    #-----------------------------------------------------------------------------------------------
+    # test case: signup with configured notify cmd that is invalid
+    #
+    def test_9001(self):
+        domain = ("%s-" % self.test_n) + TestAuto.dns_uniq
+
+        # generate config with two MDs
+        dnsList = [ domain, "www." + domain ]
+        conf = HttpdConf( TestAuto.TMP_CONF )
+        conf.add_admin( "[email protected]" )
+        conf.add_notify_cmd( "blablabla" )
+        conf.add_drive_mode( "auto" )
+        conf.add_md( dnsList )
+        conf.add_vhost( TestEnv.HTTPS_PORT, domain, aliasList=[ dnsList[1] ], withSSL=True )
+        conf.install()
+
+        # restart, and retrieve cert
+        assert TestEnv.apache_restart() == 0
+        assert TestEnv.await_completion( [ domain ], 30 )
+        # this command should have failed and logged an error
+        assert (1, 0) == TestEnv.apache_err_total()
+
+    def test_9010(self):
+        domain = ("%s-" % self.test_n) + TestAuto.dns_uniq
+        ncmd = ("%s/notify.py" % TestEnv.TESTROOT)
+        nlog = ("%s/notify.log" % TestEnv.GEN_DIR)
+
+        # generate config with two MDs
+        dnsList = [ domain, "www." + domain ]
+        conf = HttpdConf( TestAuto.TMP_CONF )
+        conf.add_admin( "[email protected]" )
+        conf.add_notify_cmd( "%s %s" % (ncmd, nlog) )
+        conf.add_drive_mode( "auto" )
+        conf.add_md( dnsList )
+        conf.add_vhost( TestEnv.HTTPS_PORT, domain, aliasList=[ dnsList[1] ], withSSL=True )
+        conf.install()
+
+        # restart, and retrieve cert
+        assert TestEnv.apache_restart() == 0
+        assert TestEnv.await_completion( [ domain ], 30 )
+        # this command should have failed and logged an error
+        assert (0, 0) == TestEnv.apache_err_total()
+        nlines = open(nlog).readlines()
+        assert 1 == len(nlines)
+        assert ("['%s', '%s', '%s']" % (ncmd, nlog, domain)) == nlines[0]
+
+    def test_9011(self):
+        domain = ("%s-" % self.test_n) + TestAuto.dns_uniq
+        ncmd = ("%s/notify.py" % TestEnv.TESTROOT)
+        nlog = ("%s/notify.log" % TestEnv.GEN_DIR)
+
+        # generate config with two MDs
+        dnsList = [ domain, "www." + domain ]
+        conf = HttpdConf( TestAuto.TMP_CONF )
+        conf.add_admin( "[email protected]" )
+        conf.add_notify_cmd( "%s %s test_9011" % (ncmd, nlog) )
+        conf.add_drive_mode( "auto" )
+        conf.add_md( dnsList )
+        conf.add_vhost( TestEnv.HTTPS_PORT, domain, aliasList=[ dnsList[1] ], withSSL=True )
+        conf.install()
+
+        # restart, and retrieve cert
+        assert TestEnv.apache_restart() == 0
+        assert TestEnv.await_completion( [ domain ], 30 )
+        # this command should have failed and logged an error
+        assert (0, 0) == TestEnv.apache_err_total()
+        nlines = open(nlog).readlines()
+        assert 1 == len(nlines)
+        assert ("['%s', '%s', 'test_9011', '%s']" % (ncmd, nlog, domain)) == nlines[0]
+
+
+

test/test_base.py

@@ -90,10 +90,10 @@ def set_store_dir( cls, dir ) :
     _a2md_args_raw = []
 
     @classmethod
-    def run( cls, args ) :
+    def run( cls, args, input=None ) :
         print "execute: ", " ".join(args)
-        p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        (output, errput) = p.communicate()
+        p = subprocess.Popen(args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        (output, errput) = p.communicate(input)
         rv = p.wait()
         print "stderr: ", errput
         try:
@@ -417,9 +417,7 @@ def apache_err_scan( cls, regex ):
             return False
         fin = open(cls.ERROR_LOG)
         for line in fin:
-            m = regex.match(line)
-            print ("match: %s" % line)
-            if m:
+            if regex.match(line):
                 return True
         return False
 
@@ -576,6 +574,9 @@ def add_http_proxy(self, url):
     def add_require_ssl(self, mode):
         self._add_line("  MDRequireHttps %s\n" % mode)
 
+    def add_notify_cmd(self, cmd):
+        self._add_line("  MDNotifyCmd %s\n" % cmd)
+
     def add_vhost(self, port, name, aliasList, docRoot="htdocs", 
                   withSSL=True, certPath=None, keyPath=None):
         self.start_vhost(port, name, aliasList, docRoot, withSSL, certPath, keyPath)