Use a common function to read fixed-size buffers from files

src/mod_sso/mod_sso.c

@@ -61,10 +61,8 @@ typedef struct {
   // Note: public_key is a binary buffer (non zero-terminated).
   const unsigned char *public_key;
 
-  // Same for the session_key (not fixed size though, so we store the
-  // size as well).
+  // Same for the session_key.
   const unsigned char *session_key;
-  size_t session_key_len;
 
   // All known groups (2.4: unused).
   apr_array_header_t *groups;
@@ -106,7 +104,6 @@ static void *create_modsso_config(apr_pool_t *p, char *s) {
   newcfg->domain = NULL;
   newcfg->public_key = NULL;
   newcfg->session_key = NULL;
-  newcfg->session_key_len = 0;
   newcfg->groups = NULL;
 
   // Return the created configuration struct.
@@ -122,16 +119,8 @@ static void *merge_modsso_config(apr_pool_t *p, void *base, void *add) {
       cadd->login_server ? cadd->login_server : cbase->login_server;
   newcfg->service = cadd->service ? cadd->service : cbase->service;
   newcfg->domain = cadd->domain ? cadd->domain : cbase->domain;
-  newcfg->public_key = cbase->public_key;
-  if (cadd->public_key) {
-    newcfg->public_key = cadd->public_key;
-  }
-  newcfg->session_key = cbase->session_key;
-  newcfg->session_key_len = cbase->session_key_len;
-  if (cadd->session_key) {
-    newcfg->session_key = cadd->session_key;
-    newcfg->session_key_len = cadd->session_key_len;
-  }
+  newcfg->public_key = cadd->public_key ? cadd->public_key : cbase->public_key;
+  newcfg->session_key = cadd->session_key ? cadd->session_key : cbase->session_key;
 
   // Groups are not merged, last takes precedence (if set).
   newcfg->groups = cadd->groups ? cadd->groups : cbase->groups;
@@ -163,43 +152,20 @@ static const char *set_modsso_domain(cmd_parms *parms, void *mconfig,
 static const char *set_modsso_public_key_file(cmd_parms *parms, void *mconfig,
                                               const char *arg) {
   modsso_config *s_cfg = (modsso_config *)mconfig;
-  char buf[128];
-  apr_size_t n = sizeof(buf);
-  apr_file_t *file;
-  int status;
 
-  if (apr_file_open(&file, arg, APR_FOPEN_READ, 0, parms->pool) !=
-      APR_SUCCESS) {
-    return "Could not open SSOPublicKeyFile";
+  if (modsso_read_fixed_size_file(parms->pool, arg, SSO_PUBLIC_KEY_SIZE, &s_cfg->public_key) < 0) {
+    return "Could not read SSOPublicKeyFile";
   }
-  status = apr_file_read(file, (void *)buf, &n);
-  apr_file_close(file);
-  if (status != APR_SUCCESS) {
-    return "Could not read contents of SSOPublicKeyFile";
-  }
-
-  unsigned char *key = (unsigned char *)apr_palloc(parms->pool, n);
-  memcpy(key, buf, n);
-  s_cfg->public_key = key;
-
   return NULL;
 }
 
 static const char *set_modsso_session_key_file(cmd_parms *parms, void *mconfig,
                                                const char *arg) {
   modsso_config *s_cfg = (modsso_config *)mconfig;
-  unsigned char *session_key = NULL;
-  size_t session_key_len = MODSSO_SESSION_KEY_SIZE;
 
-  session_key = (unsigned char *)apr_palloc(parms->pool, session_key_len);
-  if (modsso_session_read_key_from_file(parms->pool, arg, session_key,
-                                        &session_key_len) < 0) {
-    return "Could not open SSOSessionKeyFile";
+  if (modsso_read_fixed_size_file(parms->pool, arg, MODSSO_SESSION_KEY_SIZE, &s_cfg->session_key) < 0) {
+    return "Could not read SSOSessionKeyFile";
   }
-
-  s_cfg->session_key = session_key;
-  s_cfg->session_key_len = session_key_len;
-
   return NULL;
 }
 
@@ -463,7 +429,7 @@ static int mod_sso_method_handler(request_rec *r) {
     // Parse the SSO ticket and validate the nonce with the session.
     // Only do this if a session key is set (sessions are enabled).
     if (s_cfg->session_key != NULL) {
-      if (modsso_session_read(r, s_cfg->session_key, s_cfg->session_key_len,
+      if (modsso_session_read(r, s_cfg->session_key, MODSSO_SESSION_KEY_SIZE,
                               &unique_id, sso_login_path) < 0) {
         ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
                      "sso: could not read session cookie");
@@ -528,7 +494,7 @@ static int redirect_to_login_server(request_rec *r, modsso_config *s_cfg,
       // sending the session cookie on every unrelated request.
       // Ignore errors here, not much else we can do.
       sso_login_path = apr_pstrcat(r->pool, service_path, "sso_login", NULL);
-      modsso_session_save(r, s_cfg->session_key, s_cfg->session_key_len,
+      modsso_session_save(r, s_cfg->session_key, MODSSO_SESSION_KEY_SIZE,
                           unique_id, sso_login_path);
     }
   }

src/mod_sso/mod_sso.h

@@ -63,10 +63,10 @@ void modsso_set_cookie(request_rec *r, const char *cookie_name,
 
 void modsso_del_cookie(request_rec *r, const char *cookie_name,
                        const char *path);
+int modsso_read_fixed_size_file(apr_pool_t *p, const char *path,
+                                size_t size, const unsigned char **out);
 
 // session.c
-int modsso_session_read_key_from_file(apr_pool_t *pool, const char *path,
-                                      unsigned char *out, size_t *outsz);
 int modsso_session_generate_temp_key(apr_pool_t *pool, unsigned char *out,
                                      size_t *outsz);
 int modsso_session_deserialize(apr_pool_t *pool, const unsigned char *key,

src/mod_sso/session.c

@@ -37,35 +37,16 @@
 
 static const char *session_cookie_name = "_sso_local_session";
 
-/**
- * Read key from a file.
- */
-int modsso_session_read_key_from_file(apr_pool_t *pool, const char *path,
-                                      unsigned char *out, size_t *outsz) {
-  apr_size_t n = *outsz;
-  apr_file_t *file;
-  int status;
-  if (*outsz < MODSSO_SESSION_KEY_SIZE) {
-    return -1;
-  }
-  if (apr_file_open(&file, path, APR_FOPEN_READ, 0, pool) != APR_SUCCESS) {
-    return -1;
-  }
-  status = apr_file_read(file, (void *)out, &n);
-  apr_file_close(file);
-  if (status != APR_SUCCESS) {
-    return -1;
-  }
-  *outsz = n;
-  return 0;
-}
-
 /**
  * Generate a temporary key (bad!).
  */
 int modsso_session_generate_temp_key(apr_pool_t *pool, unsigned char *out,
                                      size_t *outsz) {
-  return modsso_session_read_key_from_file(pool, "/dev/urandom", out, outsz);
+  if (*outsz < MODSSO_SESSION_KEY_SIZE)
+    return -1;
+  *outsz = MODSSO_SESSION_KEY_SIZE;
+  apr_generate_random_bytes(out, *outsz);
+  return 0;
 }
 
 /**

src/mod_sso/sso_utils.c

@@ -231,3 +231,28 @@ void modsso_del_cookie(request_rec *r, const char *cookie_name, const char *path
   apr_table_addn(r->headers_out, "Set-Cookie", rfc2109);
   apr_table_addn(r->err_headers_out, "Set-Cookie", rfc2109);
 }
+
+int modsso_read_fixed_size_file(apr_pool_t *pool, const char *path, size_t size,
+                                const unsigned char **out) {
+  char *m = NULL;
+  int status;
+  apr_file_t *file;
+  apr_size_t n;
+
+  if (apr_file_open(&file, path, APR_FOPEN_READ, 0, pool) != APR_SUCCESS)
+    goto fail;
+
+  n = size;
+  m = apr_palloc(pool, n);
+  status = apr_file_read(file, m, &n);
+  apr_file_close(file);
+  if (status != APR_SUCCESS || n != size)
+    goto fail;
+
+  *out = (unsigned char *)m;
+  return 0;
+
+fail:
+  // apr_pfree(pool, m);
+  return -1;
+}