Merge pull request #19 from mgkuhn/cookiekeyfile

Cookiekeyfile

README.Config

@@ -146,26 +146,29 @@ a particular directory to anybody with a Ucam-WebAuth login is, e.g.
   </Directory>
 
 An alternative simple configuration will allow access to the resources
-to users with a Ucam-WebAuth login or to client computers with
-hostnames ending .cam.ac.uk, but not otherwise:
+to users with a Ucam-WebAuth login, to client computers with hostnames
+ending .cam.ac.uk, or to those using a CUDN IPv6 address, but not
+otherwise:
 
   AACookieKey "some random string"
   <Directory "path to protected directory">
-    Order allow,deny
-    Allow from .cam.ac.uk
+    Require host .cam.ac.uk
+    Require ip 2001:630:210::/44
     AuthType Ucam-WebAuth
     Require valid-user
-    Satisfy any
   </Directory>
 
 In these examples the resources to be protected are selected by a
 <Directory> directive; resources specified by <Location> or <Files>
 directives can also be protected in the same way. 'AuthType' must be
-set to 'Ucam-WebAuth'. A 'Require' directive must appear before
-authentication will take place.
+set to 'Ucam-WebAuth'. A directive 'Require user', 'Require group' or
+'Require valid-user' must appear before authentication will take
+place.
 
-See standard Apache documentation for more details of the 'Order',
-'Allow', 'Require', and 'Satisfy' directives.
+See standard Apache 2.4 documentation for more details of the
+'Require' directive. (Apache 2.2 and earlier use a more complictated
+authorization language involving 'Order', 'Allow', 'Require', and
+'Satisfy' directives.)
 
 The AACookieKey directive is required, though it can appear either
 outside the <Directory> block, in which case it provides a default for
@@ -377,7 +380,7 @@ AACacheControl
 
 AACookieKey
 
-  Syntax:   AACookieKey "text"
+  Syntax:   AACookieKey "text" | "file:filename"
   Context:  all
   Override: AuthConfig
   Module:   mod_ucam_webauth
@@ -389,16 +392,49 @@ AACookieKey
   cookies (see AACookieName, AACookiePath, AACookieDomain below). This
   parameter has no default and must be set.
 
+  If the parameter starts with "file:", the key will instead be read
+  from the file named in the rest of the string. Relative paths will
+  be interpreted from the server root. The key will be formed from the
+  first 64 bytes of the named file, or the entire file, whichever is
+  shorter. If the file does not provide at least 16 bytes, the server
+  will abort with an error. Using the "file:" prefix is not permitted
+  in .htaccess files.
+
+  On Linux, create a separate cookie.key file with e.g.
+
+    $ sudo bash
+    # ( umask 0077 ; head -c 64 /dev/random >/etc/apache2/cookie.key )
+
+  and then use that with
+
+    AACookieKey file:cookie.key
+
+  Alternatively,
+
+    AACookieKey file:/dev/urandom
+
+  will request from the kernel's random-byte generator a new cookie
+  key each time the server rereads its configuration, and avoids a
+  secret on the file system. However, this will trigger a round trip
+  to Raven for all currently-authenticated users every time Apache is
+  restarted/reloaded. While not a major issue, this will, for example,
+  result in the loss of POST data. (On the other hand, using
+  "AAHeaderKey file:/dev/urandom" would be pointless: the whole point
+  of AAHeaderKey is to establish a shared secret with other systems.)
+
   The key must not disclosed, since with knowledge of the key an
-  attacker can forge authentication. For preference, AACookieKey
-  should be placed in a file that is only readable by the user used to
-  start Apache, commonly root. One way to achieve this, while leaving
-  the main configuration file mainly readable, would be to put the
-  AACookieKey directive in a file only readable by root (or whoever)
-  that is incorporated into the main configuration using the 'Include'
-  directive. It may also be necessary to disable the standard mod_info
-  module since this can be use to display the Apache configuration and
-  this will include the value assigned to AACookieKey.
+  attacker can forge authentication. Putting the key directly into
+  a main configuration file can cause two problems:
+
+    - Read access to this Apache configuration file must be
+      restricted.
+
+    - Access to the server information page generated by the standard
+      mod_info module must also be restricted, since that will also
+      reveal the parameter of the AACookieKey directive.
+
+  Storing the key in a separate cookie.key file, only readable to
+  root, avoids both these risks.
 
 AACookieName
 
@@ -683,7 +719,7 @@ AAHeaders
 
 AAHeaderKey
 
-  Syntax:   AAHeaderKey "text" | none
+  Syntax:   AAHeaderKey "text" | "file:filename" | none
   Context:  all
   Override: AuthConfig
   Module:   mod_ucam_webauth
@@ -697,17 +733,10 @@ AAHeaderKey
   string 'none' in which case no hash generation will take place.
 
   Knowledge of this key could allow an attacker to forge a request
-  containing what appear to be header values set by this module.  As
-  with AACookieKey, for preference this directive should be placed in
-  a file that is only readable by the user used to start Apache,
-  commonly root. One way to achieve this, while leaving the main
-  configuration file mainly readable, would be to put the AACookieKey
-  directive in a file only readable by root (or whoever) that is
-  incorporated into the main configuration using the 'Include'
-  directive. It may also be necessary to disable the standard mod_info
-  module since this can be use to display the Apache configuration and
-  this will include the value assigned to AAHeaderKey.
-
+  containing what appear to be header values set by this module. See
+  the description of AACookieKey for more information on how to read
+  the key from a file only readable by the user used to start Apache,
+  commonly root.
 
 AAIgnoreResponseLife
 

mod_ucam_webauth.c

@@ -1879,7 +1879,7 @@ dump_config(request_rec *r, apr_pool_t *p,
 /* --- */
 
 /* Note that most string and flag parameters are processed by the generic
-   ap_set_string_slot and ap_set flag_slot routines */
+   ap_set_string_slot and ap_set_flag_slot routines */
 
 static const char *
 set_response_timeout(cmd_parms *cmd,
@@ -1996,6 +1996,74 @@ set_cache_control(cmd_parms *cmd,
 
 /* --- */
 
+/* process argument of AACookieKey and AAHeaderKey */
+static const char *
+set_key(cmd_parms *cmd,
+	void *mconfig,
+	const char *arg)
+
+{
+  int offset = (int)(long)cmd->info;
+  char **key = (char **)((char *)mconfig + offset);
+  const char *directive =
+    cmd->directive->directive;  /* "AACookieKey" or "AAHeaderKey" */
+  const char *path;
+  apr_file_t *file;
+  apr_size_t len = 64;  /* maximum number of bytes to read from file */
+  apr_status_t status;
+  char buf[256];
+  int i;
+
+  if (strncmp(arg, "file:", 5) == 0) {
+    /* load HMAC key bytes from file */
+    if (ap_check_cmd_context(cmd, NOT_IN_HTACCESS)) {
+      /* Reasons for not allowing 'file:' access in .htaccess:
+       *  - file would be read for each request (performance)
+       *  - could be abused by someone with control over
+       *    a .htaccess file to extract confidential information
+       *    from a file only readable to the Apache process.
+       *  - cmd->temp_pool no longer available
+       */
+      return "AACookieKey/AAHeaderKey: 'file:' key not permitted in .htaccess";
+    }
+    if (!arg[5] || !(path = ap_server_root_relative(cmd->temp_pool, arg + 5))) {
+      return apr_pstrcat(cmd->pool, directive, " 'file:", arg+5,
+			 "': invalid file path", NULL);
+    }
+    status = apr_file_open(&file, path, APR_FOPEN_READ | APR_FOPEN_BINARY,
+			   0, cmd->temp_pool);
+    if (status != APR_SUCCESS)
+      return apr_pstrcat(cmd->pool, directive, " 'file:",
+			 path, "': ", apr_strerror(status, buf, sizeof(buf)),
+			 NULL);
+    *key = apr_pcalloc(cmd->pool, len+1);
+    if (!*key) return "apr_pcalloc() == NULL";
+    apr_file_read(file, *key, &len);
+    apr_file_close(file);
+    if (len < 16)
+      return  apr_pstrcat(cmd->pool, directive, " 'file:", arg+5,
+			  "': key file too short (16 bytes required)", NULL);
+    /* Substitute \0 in binary input, so key can be handled as string. */
+    for (i = 0; i < len; i++) {
+      if ((*key)[i] == 0)
+	(*key)[i] = '@';
+    }
+    (*key)[len] = '\0';
+  } else {
+    /* use string argument directly as HMAC key */
+    *key = (char *) arg;
+  }
+
+  ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_STARTUP, 0, cmd->server,
+	       "setting %s '%-.4s...' (%lu bytes)",
+	       directive, *key, strlen(*key));
+
+  return NULL;
+
+}
+
+/* --- */
+
 static const char *
 set_log_level(cmd_parms *cmd,
 	      void *mconfig,
@@ -3182,7 +3250,7 @@ static const command_rec webauth_commands[] = {
                 "re-use of cached content"),
 
   AP_INIT_TAKE1("AACookieKey",
-		ap_set_string_slot,
+		set_key,
 		(void *)APR_OFFSETOF
 		(mod_ucam_webauth_cfg,cookie_key),
 		RSRC_CONF | OR_AUTHCFG,
@@ -3296,7 +3364,7 @@ static const command_rec webauth_commands[] = {
 		   "a list of additional headers to include in the request"),
 
   AP_INIT_TAKE1("AAHeaderKey",
-		ap_set_string_slot,
+		set_key,
 		(void *)APR_OFFSETOF
 		(mod_ucam_webauth_cfg,header_key),
 		RSRC_CONF | OR_AUTHCFG,