From 3d3db5d8c1384ec289368d6e20c97effd11d7183 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Jun 20 2014 13:06:41 +0000 Subject: Backport of useful patches from upstream - Better handling of IDP reported errors - Better handling of session data storage size --- diff --git a/mod_auth_mellon-0.7.0-dynamic-session-storage.patch b/mod_auth_mellon-0.7.0-dynamic-session-storage.patch new file mode 100644 index 0000000..12cd647 --- /dev/null +++ b/mod_auth_mellon-0.7.0-dynamic-session-storage.patch @@ -0,0 +1,652 @@ +diff --git a/mod_mellon2/Makefile.in b/mod_mellon2/Makefile.in +index aa83ec2..a2f9d18 100644 +--- a/mod_mellon2/Makefile.in ++++ b/mod_mellon2/Makefile.in +@@ -23,7 +23,7 @@ DISTFILES=$(SRC) \ + all: mod_auth_mellon.la + + mod_auth_mellon.la: $(SRC) auth_mellon.h auth_mellon_compat.h +- @APXS2@ -Wc,"@OPENSSL_CFLAGS@ @LASSO_CFLAGS@ @CURL_CFLAGS@ @GLIB_CFLAGS@" -Wl,"@OPENSSL_LIBS@ @LASSO_LIBS@ @CURL_LIBS@ @GLIB_LIBS@" -Wc,-Wall -Wc,-g -c $(SRC) ++ @APXS2@ -Wc,"-std=c99 @OPENSSL_CFLAGS@ @LASSO_CFLAGS@ @CURL_CFLAGS@ @GLIB_CFLAGS@" -Wl,"@OPENSSL_LIBS@ @LASSO_LIBS@ @CURL_LIBS@ @GLIB_LIBS@" -Wc,-Wall -Wc,-g -c $(SRC) + + + # Building configure (for distribution) +diff --git a/mod_mellon2/README b/mod_mellon2/README +index eb48deb..2381713 100644 +--- a/mod_mellon2/README ++++ b/mod_mellon2/README +@@ -97,6 +97,13 @@ for mod_auth_mellon. The following is an example configuration: + # Default: MellonCacheSize 100 + MellonCacheSize 100 + ++# MellonCacheEntrySize sets the maximum size for a single session entry in ++# bytes. When mod_auth_mellon reaches this limit, it cannot store any more ++# data in the session and will return an error. The minimum entry size is ++# 65536 bytes, values lower than that will be ignored and the minimum will ++# be used. ++# Default: MellonCacheEntrySize 196608 ++ + # MellonLockFile is the full path to a file used for synchronizing access + # to the session data. The path should only be used by one instance of + # apache at a time. The server must be restarted before any changes to this +diff --git a/mod_mellon2/auth_mellon.h b/mod_mellon2/auth_mellon.h +index 8347013..c6a10b3 100644 +--- a/mod_mellon2/auth_mellon.h ++++ b/mod_mellon2/auth_mellon.h +@@ -22,6 +22,8 @@ + #ifndef MOD_AUTH_MELLON_H + #define MOD_AUTH_MELLON_H + ++#include ++ + #include + #include + #include +@@ -71,13 +73,10 @@ + /* Size definitions for the session cache. + */ + #define AM_CACHE_KEYSIZE 120 +-#define AM_CACHE_VARSIZE 128 +-#define AM_CACHE_VALSIZE 512-AM_CACHE_VARSIZE + #define AM_CACHE_ENVSIZE 128 + #define AM_CACHE_USERSIZE 512 +-#define AM_CACHE_MAX_LASSO_IDENTITY_SIZE 1024 +-#define AM_CACHE_MAX_LASSO_SESSION_SIZE 32768 +-#define AM_CACHE_MAX_LASSO_SAML_RESPONSE_SIZE 65536 ++#define AM_CACHE_DEFAULT_ENTRY_SIZE 196608 ++#define AM_CACHE_MIN_ENTRY_SIZE 65536 + + + /* This is the length of the id we use (for session IDs and +@@ -101,12 +100,15 @@ typedef struct am_mod_cfg_rec { + int post_count; + apr_size_t post_size; + ++ int entry_size; ++ + /* These variables can't be allowed to change after the session store + * has been initialized. Therefore we copy them before initializing + * the session store. + */ + int init_cache_size; + const char *init_lock_file; ++ apr_size_t init_entry_size; + + apr_shm_t *cache; + apr_global_mutex_t *lock; +@@ -240,10 +242,13 @@ typedef struct am_dir_cfg_rec { + LassoServer *server; + } am_dir_cfg_rec; + ++typedef struct am_cache_storage_t { ++ apr_uintptr_t ptr; ++} am_cache_storage_t; + + typedef struct am_cache_env_t { +- char varname[AM_CACHE_VARSIZE]; +- char value[AM_CACHE_VALSIZE]; ++ am_cache_storage_t varname; ++ am_cache_storage_t value; + } am_cache_env_t; + + typedef struct am_cache_entry_t { +@@ -252,16 +257,20 @@ typedef struct am_cache_entry_t { + apr_time_t expires; + int logged_in; + unsigned short size; +- char user[AM_CACHE_USERSIZE]; ++ am_cache_storage_t user; + + /* Variables used to store lasso state between login requests + *and logout requests. + */ +- char lasso_identity[AM_CACHE_MAX_LASSO_IDENTITY_SIZE]; +- char lasso_session[AM_CACHE_MAX_LASSO_SESSION_SIZE]; +- char lasso_saml_response[AM_CACHE_MAX_LASSO_SAML_RESPONSE_SIZE]; ++ am_cache_storage_t lasso_identity; ++ am_cache_storage_t lasso_session; ++ am_cache_storage_t lasso_saml_response; + + am_cache_env_t env[AM_CACHE_ENVSIZE]; ++ ++ apr_size_t pool_size; ++ apr_size_t pool_used; ++ char pool[]; + } am_cache_entry_t; + + typedef enum { +@@ -322,6 +331,8 @@ void am_cookie_delete(request_rec *r); + + am_cache_entry_t *am_cache_lock(server_rec *s, + am_cache_key_t type, const char *key); ++const char *am_cache_entry_get_string(am_cache_entry_t *e, ++ am_cache_storage_t *slot); + am_cache_entry_t *am_cache_new(server_rec *s, const char *key); + void am_cache_unlock(server_rec *s, am_cache_entry_t *entry); + +diff --git a/mod_mellon2/auth_mellon_cache.c b/mod_mellon2/auth_mellon_cache.c +index 3923569..70c4879 100644 +--- a/mod_mellon2/auth_mellon_cache.c ++++ b/mod_mellon2/auth_mellon_cache.c +@@ -55,8 +55,6 @@ am_cache_entry_t *am_cache_lock(server_rec *s, + return NULL; + break; + case AM_CACHE_NAMEID: +- if (strlen(key) > AM_CACHE_MAX_LASSO_IDENTITY_SIZE) +- return NULL; + break; + default: + return NULL; +@@ -113,6 +111,109 @@ am_cache_entry_t *am_cache_lock(server_rec *s, + return NULL; + } + ++static inline bool am_cache_entry_slot_is_empty(am_cache_storage_t *slot) ++{ ++ return (slot->ptr == 0); ++} ++ ++static inline void am_cache_storage_null(am_cache_storage_t *slot) ++{ ++ slot->ptr = 0; ++} ++ ++static inline void am_cache_entry_env_null(am_cache_entry_t *e) ++{ ++ for (int i = 0; i < AM_CACHE_ENVSIZE; i++) { ++ am_cache_storage_null(&e->env[i].varname); ++ am_cache_storage_null(&e->env[i].value); ++ } ++} ++ ++static inline apr_size_t am_cache_entry_pool_left(am_cache_entry_t *e) ++{ ++ return e->pool_size - e->pool_used; ++} ++ ++static inline apr_size_t am_cache_entry_pool_size(am_mod_cfg_rec *cfg) ++{ ++ return cfg->init_entry_size - sizeof(am_cache_entry_t); ++} ++ ++/* This function sets a string into the specified storage on the entry. ++ * ++ * NOTE: The string pointer may be NULL, in that case storage is freed ++ * and set to NULL. ++ * ++ * Parametrs: ++ * am_cache_entry_t *entry Pointer to an entry ++ * am_cache_storage_t *slot Pointer to storage ++ * const char *string Pointer to a replacement string ++ * ++ * Returns: ++ * 0 on success, HTTP_INTERNAL_SERVER_ERROR on error. ++ */ ++static int am_cache_entry_store_string(am_cache_entry_t *entry, ++ am_cache_storage_t *slot, ++ const char *string) ++{ ++ char *datastr = NULL; ++ apr_size_t datalen = 0; ++ apr_size_t str_len = 0; ++ ++ if (string == NULL) return 0; ++ ++ if (slot->ptr != 0) { ++ datastr = &entry->pool[slot->ptr]; ++ datalen = strlen(datastr) + 1; ++ } ++ str_len = strlen(string) + 1; ++ if (str_len - datalen <= 0) { ++ memcpy(datastr, string, str_len); ++ return 0; ++ } ++ ++ /* recover space if slot happens to point to the last allocated space */ ++ if (slot->ptr + datalen == entry->pool_used) { ++ entry->pool_used -= datalen; ++ slot->ptr = 0; ++ } ++ ++ if (am_cache_entry_pool_left(entry) < str_len) { ++ ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, ++ "apr_cache_entry_store_string() asked %zd available: %zd. " ++ "It may be a good idea to increase MellonCacheEntrySize.", ++ str_len, am_cache_entry_pool_left(entry)); ++ return HTTP_INTERNAL_SERVER_ERROR; ++ } ++ ++ slot->ptr = entry->pool_used; ++ datastr = &entry->pool[slot->ptr]; ++ memcpy(datastr, string, str_len); ++ entry->pool_used += str_len; ++ return 0; ++} ++ ++/* Returns a pointer to the string in the storage slot specified ++ * ++ * ++ * Parametrs: ++ * am_cache_entry_t *entry Pointer to an entry ++ * am_cache_storage_t *slot Pointer to storage slot ++ * ++ * Returns: ++ * A string or NULL if the slot is empty. ++ */ ++const char *am_cache_entry_get_string(am_cache_entry_t *e, ++ am_cache_storage_t *slot) ++{ ++ char *ret = NULL; ++ ++ if (slot->ptr != 0) { ++ ret = &e->pool[slot->ptr]; ++ } ++ ++ return ret; ++} + + /* This function locks the session table and creates a new session entry. + * It will first attempt to locate a free session. If it doesn't find a +@@ -222,10 +323,16 @@ am_cache_entry_t *am_cache_new(server_rec *s, const char *key) + + t->logged_in = 0; + t->size = 0; +- t->user[0] = '\0'; + +- t->lasso_identity[0] = '\0'; +- t->lasso_session[0] = '\0'; ++ am_cache_storage_null(&t->user); ++ am_cache_storage_null(&t->lasso_identity); ++ am_cache_storage_null(&t->lasso_session); ++ am_cache_storage_null(&t->lasso_saml_response); ++ am_cache_entry_env_null(t); ++ ++ t->pool_size = am_cache_entry_pool_size(mod_cfg); ++ t->pool[0] = '\0'; ++ t->pool_used = 1; + + return t; + } +@@ -286,27 +393,36 @@ void am_cache_update_expires(am_cache_entry_t *t, apr_time_t expires) + int am_cache_env_append(am_cache_entry_t *t, + const char *var, const char *val) + { ++ int status; ++ + /* Make sure that the name and value will fit inside the + * fixed size buffer. + */ +- if(strlen(val) >= AM_CACHE_VALSIZE || +- strlen(var) >= AM_CACHE_VARSIZE) { ++ if(t->size >= AM_CACHE_ENVSIZE) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, +- "Unable to store session data because it is to big. " +- "Name = \"%s\"; Value = \"%s\".", var, val); ++ "Unable to store attribute value because we have" ++ " reached the maximum number of name-value pairs for" ++ " this session. The maximum number is %d.", ++ AM_CACHE_ENVSIZE); + return HTTP_INTERNAL_SERVER_ERROR; + } + +- if(t->size >= AM_CACHE_ENVSIZE) { ++ status = am_cache_entry_store_string(t, &t->env[t->size].varname, var); ++ if (status != 0) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, +- "Unable to store attribute value because we have" +- " reached the maximum number of name-value pairs for" +- " this session."); ++ "Unable to store session data because there is no more " ++ "space in the session. Attribute Name = \"%s\".", var); ++ return HTTP_INTERNAL_SERVER_ERROR; ++ } ++ ++ status = am_cache_entry_store_string(t, &t->env[t->size].value, val); ++ if (status != 0) { ++ ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, ++ "Unable to store session data because there is no more " ++ "space in the session. Attribute Value = \"%s\".", val); + return HTTP_INTERNAL_SERVER_ERROR; + } + +- strcpy(t->env[t->size].varname, var); +- strcpy(t->env[t->size].value, val); + t->size++; + + return OK; +@@ -325,11 +441,15 @@ int am_cache_env_append(am_cache_entry_t *t, + const char *am_cache_env_fetch_first(am_cache_entry_t *t, + const char *var) + { ++ const char *str; + int i; + + for (i = 0; t->size; i++) { +- if (strcmp(t->env[i].varname, var) == 0) +- return t->env[i].value; ++ str = am_cache_entry_get_string(t, &t->env[i].varname); ++ if (str == NULL) ++ break; ++ if (strcmp(str, var) == 0) ++ return str; + } + + return NULL; +@@ -356,15 +476,24 @@ void am_cache_env_populate(request_rec *r, am_cache_entry_t *t) + const char *varname_prefix; + const char *value; + int *count; ++ int status; + + d = am_get_dir_cfg(r); + + /* Check if the user attribute has been set, and set it if it + * hasn't been set. */ +- if(t->user[0] == '\0') { ++ if (am_cache_entry_slot_is_empty(&t->user)) { + for(i = 0; i < t->size; ++i) { +- if(strcmp(t->env[i].varname, d->userattr) == 0) { +- strcpy(t->user, t->env[i].value); ++ varname = am_cache_entry_get_string(t, &t->env[i].varname); ++ if (strcmp(varname, d->userattr) == 0) { ++ value = am_cache_entry_get_string(t, &t->env[i].value); ++ status = am_cache_entry_store_string(t, &t->user, value); ++ if (status != 0) { ++ ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, ++ "Unable to store the user name because there" ++ " is no more space in the session. " ++ "Username = \"%s\".", value); ++ } + } + } + } +@@ -376,7 +505,7 @@ void am_cache_env_populate(request_rec *r, am_cache_entry_t *t) + * received from the IdP. + */ + for(i = 0; i < t->size; ++i) { +- varname = t->env[i].varname; ++ varname = am_cache_entry_get_string(t, &t->env[i].varname); + varname_prefix = "MELLON_"; + + /* Check if we should map this name into another name. */ +@@ -390,13 +519,21 @@ void am_cache_env_populate(request_rec *r, am_cache_entry_t *t) + } + } + +- value = t->env[i].value; ++ value = am_cache_entry_get_string(t, &t->env[i].value); + + /* + * If we find a variable remapping to MellonUser, use it. + */ +- if ((t->user[0] == '\0') && (strcmp(varname, d->userattr) == 0)) +- strcpy(t->user, value); ++ if (am_cache_entry_slot_is_empty(&t->user) && ++ (strcmp(varname, d->userattr) == 0)) { ++ status = am_cache_entry_store_string(t, &t->user, value); ++ if (status != 0) { ++ ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, ++ "Unable to store the user name because there" ++ " is no more space in the session. " ++ "Username = \"%s\".", value); ++ } ++ } + + /* Find the number of times this variable has been set. */ + count = apr_hash_get(counters, varname, APR_HASH_KEY_STRING); +@@ -424,9 +561,9 @@ void am_cache_env_populate(request_rec *r, am_cache_entry_t *t) + ++(*count); + } + +- if(t->user[0] != '\0') { ++ if (!am_cache_entry_slot_is_empty(&t->user)) { + /* We have a user-"name". Set r->user and r->ap_auth_type. */ +- r->user = apr_pstrdup(r->pool, t->user); ++ r->user = apr_pstrdup(r->pool, am_cache_entry_get_string(t, &t->user)); + r->ap_auth_type = apr_pstrdup(r->pool, "Mellon"); + } else { + /* We don't have a user-"name". Log error. */ +@@ -441,20 +578,24 @@ void am_cache_env_populate(request_rec *r, am_cache_entry_t *t) + /* Populate with the session? */ + if (d->dump_session) { + char *session; ++ const char *srcstr; + int srclen, dstlen; + +- srclen = strlen(t->lasso_session); ++ srcstr = am_cache_entry_get_string(t, &t->lasso_session); ++ srclen = strlen(srcstr); + dstlen = apr_base64_encode_len(srclen); + + session = apr_palloc(r->pool, dstlen); +- (void)apr_base64_encode(session, t->lasso_session, srclen); ++ (void)apr_base64_encode(session, srcstr, srclen); + apr_table_set(r->subprocess_env, "MELLON_SESSION", session); + } + +- if (d->dump_saml_response) +- apr_table_set(r->subprocess_env, +- "MELLON_SAML_RESPONSE", +- t->lasso_saml_response); ++ if (d->dump_saml_response) { ++ const char *sr = am_cache_entry_get_string(t, &t->lasso_saml_response); ++ if (sr) { ++ apr_table_set(r->subprocess_env, "MELLON_SAML_RESPONSE", sr); ++ } ++ } + } + + +@@ -496,56 +637,39 @@ int am_cache_set_lasso_state(am_cache_entry_t *session, + const char *lasso_session, + const char *lasso_saml_response) + { +- if(lasso_identity != NULL) { +- if(strlen(lasso_identity) < AM_CACHE_MAX_LASSO_IDENTITY_SIZE) { +- strcpy(session->lasso_identity, lasso_identity); +- } else { +- ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, +- "Lasso identity is to big for storage. Size of lasso" +- " identity is %" APR_SIZE_T_FMT ", max size is %" +- APR_SIZE_T_FMT ".", +- (apr_size_t)strlen(lasso_identity), +- (apr_size_t)AM_CACHE_MAX_LASSO_IDENTITY_SIZE - 1); +- return HTTP_INTERNAL_SERVER_ERROR; +- } +- } else { +- /* No identity dump to save. */ +- strcpy(session->lasso_identity, ""); +- } ++ int status; + ++ status = am_cache_entry_store_string(session, ++ &session->lasso_identity, ++ lasso_identity); ++ if (status != 0) { ++ ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, ++ "Lasso identity is to big for storage. Size of lasso" ++ " identity is %" APR_SIZE_T_FMT ".", ++ (apr_size_t)strlen(lasso_identity)); ++ return HTTP_INTERNAL_SERVER_ERROR; ++ } + +- if(lasso_session != NULL) { +- if(strlen(lasso_session) < AM_CACHE_MAX_LASSO_SESSION_SIZE) { +- strcpy(session->lasso_session, lasso_session); +- } else { +- ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, +- "Lasso session is to big for storage. Size of lasso" +- " session is %" APR_SIZE_T_FMT ", max size is %" +- APR_SIZE_T_FMT ".", +- (apr_size_t)strlen(lasso_session), +- (apr_size_t)AM_CACHE_MAX_LASSO_SESSION_SIZE - 1); +- return HTTP_INTERNAL_SERVER_ERROR; +- } +- } else { +- /* No session dump to save. */ +- strcpy(session->lasso_session, ""); ++ status = am_cache_entry_store_string(session, ++ &session->lasso_session, ++ lasso_session); ++ if (status != 0) { ++ ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, ++ "Lasso session is to big for storage. Size of lasso" ++ " session is %" APR_SIZE_T_FMT ".", ++ (apr_size_t)strlen(lasso_session)); ++ return HTTP_INTERNAL_SERVER_ERROR; + } + +- if(lasso_saml_response != NULL) { +- if(strlen(lasso_saml_response) < AM_CACHE_MAX_LASSO_SAML_RESPONSE_SIZE) { +- strcpy(session->lasso_saml_response, lasso_saml_response); +- } else { +- ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, +- "Lasso SAML response is to big for storage. " +- "Size of lasso session is %" APR_SIZE_T_FMT +- ", max size is %" APR_SIZE_T_FMT ".", +- (apr_size_t)strlen(lasso_saml_response), +- (apr_size_t)AM_CACHE_MAX_LASSO_SAML_RESPONSE_SIZE - 1); +- return HTTP_INTERNAL_SERVER_ERROR; +- } +- } else { +- /* No session dump to save. */ +- strcpy(session->lasso_saml_response, ""); ++ status = am_cache_entry_store_string(session, ++ &session->lasso_saml_response, ++ lasso_saml_response); ++ if (status != 0) { ++ ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL, ++ "Lasso SAML response is to big for storage. Size of " ++ "lasso SAML Reponse is %" APR_SIZE_T_FMT ".", ++ (apr_size_t)strlen(lasso_saml_response)); ++ return HTTP_INTERNAL_SERVER_ERROR; + } + + return OK; +@@ -562,11 +686,7 @@ int am_cache_set_lasso_state(am_cache_entry_t *session, + */ + const char *am_cache_get_lasso_identity(am_cache_entry_t *session) + { +- if(strlen(session->lasso_identity) == 0) { +- return NULL; +- } +- +- return session->lasso_identity; ++ return am_cache_entry_get_string(session, &session->lasso_identity); + } + + +@@ -580,9 +700,5 @@ const char *am_cache_get_lasso_identity(am_cache_entry_t *session) + */ + const char *am_cache_get_lasso_session(am_cache_entry_t *session) + { +- if(strlen(session->lasso_session) == 0) { +- return NULL; +- } +- +- return session->lasso_session; ++ return am_cache_entry_get_string(session, &session->lasso_session); + } +diff --git a/mod_mellon2/auth_mellon_config.c b/mod_mellon2/auth_mellon_config.c +index 36f6b96..dbcbfaa 100644 +--- a/mod_mellon2/auth_mellon_config.c ++++ b/mod_mellon2/auth_mellon_config.c +@@ -19,8 +19,6 @@ + * + */ + +-#include +- + #include "auth_mellon.h" + + /* This is the default endpoint path. Remember to update the description of +@@ -863,6 +861,15 @@ const command_rec auth_mellon_commands[] = { + " take effect. The default value is 100." + ), + AP_INIT_TAKE1( ++ "MellonCacheEntrySize", ++ am_set_module_config_int_slot, ++ (void *)APR_OFFSETOF(am_mod_cfg_rec, entry_size), ++ RSRC_CONF, ++ "The maximum size for a single session entry. You must" ++ " restart the server before any changes to this directive will" ++ " take effect. The default value is 192KiB." ++ ), ++ AP_INIT_TAKE1( + "MellonLockFile", + am_set_module_config_file_slot, + (void *)APR_OFFSETOF(am_mod_cfg_rec, lock_file), +@@ -1571,8 +1578,11 @@ void *auth_mellon_server_config(apr_pool_t *p, server_rec *s) + mod->post_count = post_count; + mod->post_size = post_size; + ++ mod->entry_size = AM_CACHE_DEFAULT_ENTRY_SIZE; ++ + mod->init_cache_size = 0; + mod->init_lock_file = NULL; ++ mod->init_entry_size = 0; + + mod->cache = NULL; + mod->lock = NULL; +diff --git a/mod_mellon2/auth_mellon_util.c b/mod_mellon2/auth_mellon_util.c +index 6219c83..4a34acd 100644 +--- a/mod_mellon2/auth_mellon_util.c ++++ b/mod_mellon2/auth_mellon_util.c +@@ -307,7 +307,8 @@ int am_check_permissions(request_rec *r, am_cache_entry_t *session) + */ + if (ce->flags & AM_COND_FLAG_MAP) + varname = apr_hash_get(dir_cfg->envattr, +- session->env[j].varname, ++ am_cache_entry_get_string(session, ++ &session->env[j].varname), + APR_HASH_KEY_STRING); + + /* +@@ -315,12 +316,13 @@ int am_check_permissions(request_rec *r, am_cache_entry_t *session) + * sent by the IdP. + */ + if (varname == NULL) +- varname = session->env[j].varname; ++ varname = am_cache_entry_get_string(session, ++ &session->env[j].varname); + + if (strcmp(varname, ce->varname) != 0) + continue; + +- value = session->env[j].value; ++ value = am_cache_entry_get_string(session, &session->env[j].value); + + /* + * Substiture backrefs if available +diff --git a/mod_mellon2/configure.ac b/mod_mellon2/configure.ac +index 0c1a602..be74cbf 100644 +--- a/mod_mellon2/configure.ac ++++ b/mod_mellon2/configure.ac +@@ -1,5 +1,8 @@ + AC_INIT([mod_auth_mellon],[0.7.0],[olav.morken@uninett.no]) + ++# We require support for C99. ++AC_PROG_CC_C99 ++ + AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION()) + + +diff --git a/mod_mellon2/mod_auth_mellon.c b/mod_mellon2/mod_auth_mellon.c +index 86949a4..fc34962 100644 +--- a/mod_mellon2/mod_auth_mellon.c ++++ b/mod_mellon2/mod_auth_mellon.c +@@ -88,9 +88,13 @@ static int am_global_init(apr_pool_t *conf, apr_pool_t *log, + */ + mod->init_cache_size = mod->cache_size; + mod->init_lock_file = apr_pstrdup(conf, mod->lock_file); ++ mod->init_entry_size = mod->entry_size; ++ if (mod->init_entry_size < AM_CACHE_MIN_ENTRY_SIZE) { ++ mod->init_entry_size = AM_CACHE_MIN_ENTRY_SIZE; ++ } + + /* find out the memory size of the cache */ +- mem_size = sizeof(am_cache_entry_t) * mod->init_cache_size; ++ mem_size = mod->init_entry_size * mod->init_cache_size; + + + /* Create the shared memory, exit if it fails. */ diff --git a/mod_auth_mellon-0.7.0-handle-idp-errors.patch b/mod_auth_mellon-0.7.0-handle-idp-errors.patch new file mode 100644 index 0000000..27d83f5 --- /dev/null +++ b/mod_auth_mellon-0.7.0-handle-idp-errors.patch @@ -0,0 +1,137 @@ +diff --git a/mod_mellon2/README b/mod_mellon2/README +index 78b5f3f..eb48deb 100644 +--- a/mod_mellon2/README ++++ b/mod_mellon2/README +@@ -491,6 +491,15 @@ MellonPostCount 100 + # The default is that it is "Off". + # MellonPostReplay Off + ++ # Page to redirect to if the IdP sends an error in response to ++ # the authentication request. ++ # ++ # Example: ++ # MellonNoSuccessErrorPage https://sp.example.org/login_failed.html ++ # ++ # The default is to not redirect, but rather send a ++ # 401 Unautorized error. ++ + + + +diff --git a/mod_mellon2/auth_mellon.h b/mod_mellon2/auth_mellon.h +index f99cf6f..8347013 100644 +--- a/mod_mellon2/auth_mellon.h ++++ b/mod_mellon2/auth_mellon.h +@@ -210,6 +210,9 @@ typedef struct am_dir_cfg_rec { + /* No cookie error page. */ + const char *no_cookie_error_page; + ++ /* Authorization error page. */ ++ const char *no_success_error_page; ++ + /* Login path for IdP initiated logins */ + const char *login_path; + +@@ -276,6 +279,13 @@ typedef struct am_envattr_conf_t { + + extern const command_rec auth_mellon_commands[]; + ++typedef struct am_error_map_t { ++ int lasso_error; ++ int http_error; ++} am_error_map_t; ++ ++extern const am_error_map_t auth_mellon_errormap[]; ++ + /* When using a value from a directory configuration structure, a special value is used + * to state "inherit" from parent, when reading a value and the value is still inherit from, it + * means that no value has ever been set for this directive, in this case, we use the default +diff --git a/mod_mellon2/auth_mellon_config.c b/mod_mellon2/auth_mellon_config.c +index 855330a..36f6b96 100644 +--- a/mod_mellon2/auth_mellon_config.c ++++ b/mod_mellon2/auth_mellon_config.c +@@ -1046,6 +1046,15 @@ const command_rec auth_mellon_commands[] = { + " ha disabled cookies." + ), + AP_INIT_TAKE1( ++ "MellonNoSuccessErrorPage", ++ ap_set_string_slot, ++ (void *)APR_OFFSETOF(am_dir_cfg_rec, no_success_error_page), ++ OR_AUTHCFG, ++ "Web page to display if the idp posts with a failed" ++ " authentication error. We will return a 401 Unauthorized error" ++ " if this is unset and the idp posts such assertion." ++ ), ++ AP_INIT_TAKE1( + "MellonSPMetadataFile", + am_set_filestring_slot, + (void *)APR_OFFSETOF(am_dir_cfg_rec, sp_metadata_file), +@@ -1205,6 +1214,13 @@ const command_rec auth_mellon_commands[] = { + {NULL} + }; + ++const am_error_map_t auth_mellon_errormap[] = { ++ { LASSO_PROFILE_ERROR_STATUS_NOT_SUCCESS, HTTP_UNAUTHORIZED }, ++#ifdef LASSO_PROFILE_ERROR_REQUEST_DENIED ++ { LASSO_PROFILE_ERROR_REQUEST_DENIED, HTTP_UNAUTHORIZED }, ++#endif ++ { 0, 0 } ++}; + + /* Release a lasso_server object associated with this configuration. + * +@@ -1264,6 +1280,7 @@ void *auth_mellon_dir_config(apr_pool_t *p, char *d) + dir->session_length = -1; /* -1 means use default. */ + + dir->no_cookie_error_page = NULL; ++ dir->no_success_error_page = NULL; + + dir->sp_metadata_file = NULL; + dir->sp_private_key_file = NULL; +@@ -1418,6 +1435,10 @@ void *auth_mellon_dir_merge(apr_pool_t *p, void *base, void *add) + add_cfg->no_cookie_error_page : + base_cfg->no_cookie_error_page); + ++ new_cfg->no_success_error_page = (add_cfg->no_success_error_page != NULL ? ++ add_cfg->no_success_error_page : ++ base_cfg->no_success_error_page); ++ + + new_cfg->sp_metadata_file = (add_cfg->sp_metadata_file ? + add_cfg->sp_metadata_file : +diff --git a/mod_mellon2/auth_mellon_handler.c b/mod_mellon2/auth_mellon_handler.c +index 1d42fd7..1de217a 100644 +--- a/mod_mellon2/auth_mellon_handler.c ++++ b/mod_mellon2/auth_mellon_handler.c +@@ -1974,6 +1974,8 @@ static int am_handle_post_reply(request_rec *r) + LassoServer *server; + LassoLogin *login; + char *relay_state; ++ am_dir_cfg_rec *dir_cfg = am_get_dir_cfg(r); ++ int i, err; + + /* Make sure that this is a POST request. */ + if(r->method_number != M_POST) { +@@ -2040,7 +2042,21 @@ static int am_handle_post_reply(request_rec *r) + " Lasso error: [%i] %s", rc, lasso_strerror(rc)); + + lasso_login_destroy(login); +- return HTTP_BAD_REQUEST; ++ err = HTTP_BAD_REQUEST; ++ for (i = 0; auth_mellon_errormap[i].lasso_error != 0; i++) { ++ if (auth_mellon_errormap[i].lasso_error == rc) { ++ err = auth_mellon_errormap[i].http_error; ++ break; ++ } ++ } ++ if (err == HTTP_UNAUTHORIZED) { ++ if (dir_cfg->no_success_error_page != NULL) { ++ apr_table_setn(r->headers_out, "Location", ++ dir_cfg->no_success_error_page); ++ return HTTP_SEE_OTHER; ++ } ++ } ++ return err; + } + + /* Extract RelayState parameter. */ diff --git a/mod_auth_mellon.spec b/mod_auth_mellon.spec index d920da4..32facb6 100644 --- a/mod_auth_mellon.spec +++ b/mod_auth_mellon.spec @@ -1,7 +1,7 @@ Summary: A SAML 2.0 authentication module for the Apache Httpd Server Name: mod_auth_mellon Version: 0.7.0 -Release: 2%{?dist} +Release: 3%{?dist} Group: System Environment/Daemons Source0: https://modmellon.googlecode.com/files/%{name}-%{version}.tar.gz Source1: auth_mellon.conf @@ -14,6 +14,9 @@ Requires: httpd-mmn = %{_httpd_mmn} Requires: lasso >= 2.3.6 Url: https://code.google.com/p/modmellon/ +Patch01: mod_auth_mellon-0.7.0-handle-idp-errors.patch +Patch02: mod_auth_mellon-0.7.0-dynamic-session-storage.patch + %description The mod_auth_mellon module is an authentication service that implements the SAML 2.0 federation protocol. It grants access based on the attributes @@ -21,6 +24,8 @@ received in assertions generated by a IdP server. %prep %setup -q -n %{name}-%{version} +%patch01 -p2 +%patch02 -p2 %build export APXS=%{_httpd_apxs} @@ -57,6 +62,11 @@ install -m 755 %{SOURCE4} %{buildroot}/%{_libexecdir}/%{name} %dir /run/%{name}/ %changelog +* Fri Jun 20 2014 Simo Sorce 0.7.0-3 +- Backport of useful patches from upstream + - Better handling of IDP reported errors + - Better handling of session data storage size + * Sat Jun 07 2014 Fedora Release Engineering - 0.7.0-2 - Rebuilt for https://fedoraproject.org/wiki/Fedora_21_Mass_Rebuild