Blob Blame History Raw
Note: this patch has been sligtly modified from the origianal
commit to change the default signature algorithm back to sha1

commit 9b17e5c1078a9be90de1e9d03079b34ca4056e96
Author: John Dennis <jdennis@redhat.com>
Date:   Thu Jan 11 13:05:26 2018 -0500

    Add MellonSignatureMethod to control signature algorithm
    
    Previously there was no way to control the signature algorithm used
    when Mellon signed it's SAML messages. It simply defaulted to whatever
    the default was in the LassoServer server object. Currently the lasso
    default is LASSO_SIGNATURE_METHOD_RSA_SHA1. Some IdP's require a
    different or more secure method (e.g. ADFS). This patch allows
    controlling the signature method on a per directory basis via the
    MellonSignatureMethod configuration directive.
    
    It currently supports the following configuration values which map to
    these Lasso enumerated constants (provided these definition exist in
    Lasso):
    
    rsa-sha1:    LASSO_SIGNATURE_METHOD_RSA_SHA1
    rsa-sha256:  LASSO_SIGNATURE_METHOD_RSA_SHA256
    rsa-sha384:  LASSO_SIGNATURE_METHOD_RSA_SHA384
    rsa-sha512:  LASSO_SIGNATURE_METHOD_RSA_SHA512
    
    configure.ac was modified to test for the existence of the above
    Lasso definitions, support is only compiled into Mellon if they
    are defined at build time.
    
    The patch also includes a few corrections in the diagnostics code
    where it failed to use CFG_VALUE. Also fixed the diagnostics code when
    an unknown value was encounted to print what that unknown value was.
    
    Signed-off-by: John Dennis <jdennis@redhat.com>

diff --git a/README b/README
index 2660e9a..0a91dc5 100644
--- a/README
+++ b/README
@@ -583,6 +583,18 @@ MellonDiagnosticsEnable Off
         #  MellonRedirectDomains [self]
         MellonRedirectDomains [self]
 
+        # This option controls the signature method used to sign SAML
+        # messages generated by Mellon, it may be one of the following
+        # (depending if feature was supported when Mellon was built):
+        #
+        # rsa-sha1
+        # rsa-sha256
+        # rsa-sha384
+        # rsa-sha512
+        #
+        # Default: rsa-sha1
+        # MellonSignatureMethod
+
 </Location>

 
diff --git a/auth_mellon.h b/auth_mellon.h
index a10bc4c..3b23537 100644
--- a/auth_mellon.h
+++ b/auth_mellon.h
@@ -239,6 +239,7 @@ typedef struct am_dir_cfg_rec {
     apr_hash_t *envattr;
     const char *userattr;
     const char *idpattr;
+    LassoSignatureMethod signature_method;
     int dump_session;
     int dump_saml_response;
 
@@ -416,6 +417,14 @@ static const int inherit_post_replay = -1;
 static const int default_ecp_send_idplist = 0;
 static const int inherit_ecp_send_idplist = -1;
 
+/* Algorithm to use when signing Mellon SAML messages */
+static const LassoSignatureMethod default_signature_method =
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA256
+    LASSO_SIGNATURE_METHOD_RSA_SHA1;
+#else
+    LASSO_SIGNATURE_METHOD_RSA_SHA1;
+#endif
+static const int inherit_signature_method = -1;
 
 void *auth_mellon_dir_config(apr_pool_t *p, char *d);
 void *auth_mellon_dir_merge(apr_pool_t *p, void *base, void *add);
diff --git a/auth_mellon_config.c b/auth_mellon_config.c
index 66966fc..396ff1e 100644
--- a/auth_mellon_config.c
+++ b/auth_mellon_config.c
@@ -105,6 +105,7 @@ static const int default_env_vars_count_in_n = -1;
 /* The default list of trusted redirect domains. */
 static const char * const default_redirect_domains[] = { "[self]", NULL };
 
+
 /* This function handles configuration directives which set a 
  * multivalued string slot in the module configuration (the destination
  * strucure is a hash).
@@ -1139,6 +1140,63 @@ static const char *am_set_redirect_domains(cmd_parms *cmd,
     return NULL;
 }
 
+/* This function handles the MellonSignatureMethod configuration directive.
+ * This directive can be set to one of:
+ *
+ * Parameters:
+ *  cmd_parms *cmd       The command structure for this configuration
+ *                       directive.
+ *  void *struct_ptr     Pointer to the current directory configuration.
+ *  const char *arg      The string argument following this configuration
+ *                       directive in the configuraion file.
+ *
+ * Returns:
+ *  NULL on success or an error string if the argument is wrong.
+ */
+static const char *am_set_signature_method_slot(cmd_parms *cmd,
+                                                void *struct_ptr,
+                                                const char *arg)
+{
+    am_dir_cfg_rec *d = (am_dir_cfg_rec *)struct_ptr;
+    char *valid_methods = "rsa-sha1"
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA256
+        " rsa-sha256"
+#endif
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA384
+        " rsa-sha384"
+#endif
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA512
+        " rsa-sha512"
+#endif
+        ;
+
+    if (!strcasecmp(arg, "rsa-sha1")) {
+        d->signature_method = LASSO_SIGNATURE_METHOD_RSA_SHA1;
+    }
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA256
+    else if (!strcasecmp(arg, "rsa-sha256")) {
+        d->signature_method = LASSO_SIGNATURE_METHOD_RSA_SHA256;
+    }
+#endif
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA384
+    else if (!strcasecmp(arg, "rsa-sha384")) {
+        d->signature_method = LASSO_SIGNATURE_METHOD_RSA_SHA384;
+    }
+#endif
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA512
+    else if (!strcasecmp(arg, "rsa-sha512")) {
+        d->signature_method = LASSO_SIGNATURE_METHOD_RSA_SHA512;
+    }
+#endif
+    else {
+        return apr_psprintf(cmd->pool,
+                            "%s: Invalid method \"%s\", must be one of: %s",
+                            cmd->cmd->name, arg, valid_methods);
+    }
+
+    return NULL;
+}
+
 /* This array contains all the configuration directive which are handled
  * by auth_mellon.
  */    
@@ -1587,6 +1645,13 @@ const command_rec auth_mellon_commands[] = {
         OR_AUTHCFG,
         "List of domains we can redirect to."
         ),
+    AP_INIT_TAKE1(
+        "MellonSignatureMethod",
+        am_set_signature_method_slot,
+        NULL,
+        OR_AUTHCFG,
+        "Signature method used to sign SAML messages sent by Mellon"
+        ),
     {NULL}
 };
 
@@ -1651,6 +1716,7 @@ void *auth_mellon_dir_config(apr_pool_t *p, char *d)
     dir->envattr   = apr_hash_make(p);
     dir->userattr  = default_user_attribute;
     dir->idpattr  = NULL;
+    dir->signature_method = inherit_signature_method;
     dir->dump_session = default_dump_session;
     dir->dump_saml_response = default_dump_saml_response;
 
@@ -1810,6 +1876,8 @@ void *auth_mellon_dir_merge(apr_pool_t *p, void *base, void *add)
                         add_cfg->idpattr :
                         base_cfg->idpattr);
 
+    new_cfg->signature_method = CFG_MERGE(add_cfg, base_cfg, signature_method);
+
     new_cfg->dump_session = (add_cfg->dump_session != default_dump_session ?
                              add_cfg->dump_session :
                              base_cfg->dump_session);
diff --git a/auth_mellon_diagnostics.c b/auth_mellon_diagnostics.c
index 289a878..8919b7d 100644
--- a/auth_mellon_diagnostics.c
+++ b/auth_mellon_diagnostics.c
@@ -39,14 +39,17 @@ static const char *
 am_diag_cond_flag_str(request_rec *r, am_cond_flag_t flags);
 
 static const char *
-am_diag_enable_str(am_enable_t enable);
+am_diag_enable_str(request_rec *r, am_enable_t enable);
 
 static const char *
-am_diag_samesite_str(am_samesite_t samesite);
+am_diag_samesite_str(request_rec *r, am_samesite_t samesite);
 
 static const char *
 am_diag_httpd_error_level_str(request_rec *r, int level);
 
+static const char *
+am_diag_signature_method_str(request_rec *r,
+                             LassoSignatureMethod signature_method);
 static apr_size_t
 am_diag_time_t_to_8601_buf(char *buf, apr_size_t buf_size, apr_time_t t);
 
@@ -191,26 +194,28 @@ am_diag_cond_flag_str(request_rec *r, am_cond_flag_t flags)
 }
 
 static const char *
-am_diag_enable_str(am_enable_t enable)
+am_diag_enable_str(request_rec *r, am_enable_t enable)
 {
     switch(enable) {
     case am_enable_default: return "default";
     case am_enable_off:     return "off";
     case am_enable_info:    return "info";
     case am_enable_auth:    return "auth";
-    default:                return "unknown";
+    default:
+        return apr_psprintf(r->pool, "unknown (%d)", enable);
     }
 
 }
 
 static const char *
-am_diag_samesite_str(am_samesite_t samesite)
+am_diag_samesite_str(request_rec *r, am_samesite_t samesite)
 {
     switch(samesite) {
     case am_samesite_default: return "default";
     case am_samesite_lax:     return "lax";
     case am_samesite_strict:  return "strict";
-    default:                  return "unknown";
+    default:
+        return apr_psprintf(r->pool, "unknown (%d)", samesite);
     }
 }
 
@@ -239,6 +244,26 @@ am_diag_httpd_error_level_str(request_rec *r, int level)
     }
 }
 
+static const char *
+am_diag_signature_method_str(request_rec *r,
+                             LassoSignatureMethod signature_method)
+{
+    switch(signature_method) {
+    case LASSO_SIGNATURE_METHOD_RSA_SHA1:    return "rsa-sha1";
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA256
+    case LASSO_SIGNATURE_METHOD_RSA_SHA256:  return "rsa-sha256";
+#endif
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA384
+    case LASSO_SIGNATURE_METHOD_RSA_SHA384:  return "rsa-sha384";
+#endif
+#if HAVE_DECL_LASSO_SIGNATURE_METHOD_RSA_SHA512
+    case LASSO_SIGNATURE_METHOD_RSA_SHA512:  return "rsa-sha512";
+#endif
+    default:
+        return apr_psprintf(r->pool, "unknown (%d)", signature_method);
+    }
+}
+
 static apr_size_t
 am_diag_time_t_to_8601_buf(char *buf, apr_size_t buf_size, apr_time_t t)
 {
@@ -388,7 +413,7 @@ am_diag_log_dir_cfg(request_rec *r, int level, am_dir_cfg_rec *cfg,
 
     apr_file_printf(diag_cfg->fd,
                     "%sMellonEnable (enable): %s\n",
-                    indent(level+1), am_diag_enable_str(cfg->enable_mellon));
+                    indent(level+1), am_diag_enable_str(r, cfg->enable_mellon));
     apr_file_printf(diag_cfg->fd,
                     "%sMellonVariable (varname): %s\n",
                     indent(level+1), cfg->varname);
@@ -416,7 +441,7 @@ am_diag_log_dir_cfg(request_rec *r, int level, am_dir_cfg_rec *cfg,
     apr_file_printf(diag_cfg->fd,
                     "%sMellonCookieSameSite (cookie_samesite): %s\n",
                     indent(level+1),
-                    am_diag_samesite_str(cfg->cookie_samesite));
+                    am_diag_samesite_str(r, cfg->cookie_samesite));
 
     apr_file_printf(diag_cfg->fd,
                     "%sMellonCond (cond): %d items\n",
@@ -597,7 +622,7 @@ am_diag_log_dir_cfg(request_rec *r, int level, am_dir_cfg_rec *cfg,
                     "%sMellonSubjectConfirmationDataAddressCheck"
                     " (subject_confirmation_data_address_check): %s\n",
                     indent(level+1),
-                    cfg->subject_confirmation_data_address_check ? "On":"Off");
+                    CFG_VALUE(cfg, subject_confirmation_data_address_check) ? "On":"Off");
 
     apr_file_printf(diag_cfg->fd,
                     "%sMellonDoNotVerifyLogoutSignature"
@@ -622,13 +647,13 @@ am_diag_log_dir_cfg(request_rec *r, int level, am_dir_cfg_rec *cfg,
                     "%sMellonSendCacheControlHeader"
                     " (send_cache_control_header): %s\n",
                     indent(level+1),
-                    cfg->send_cache_control_header ? "On":"Off");
+                    CFG_VALUE(cfg, send_cache_control_header) ? "On":"Off");
     apr_file_printf(diag_cfg->fd,
                     "%sMellonPostReplay (post_replay): %s\n",
-                    indent(level+1), cfg->post_replay ? "On":"Off");
+                    indent(level+1), CFG_VALUE(cfg, post_replay) ? "On":"Off");
     apr_file_printf(diag_cfg->fd,
                     "%sMellonECPSendIDPList (ecp_send_idplist): %s\n",
-                    indent(level+1), cfg->ecp_send_idplist ? "On":"Off");
+                    indent(level+1), CFG_VALUE(cfg, ecp_send_idplist) ? "On":"Off");
 
     for (n_items = 0; cfg->redirect_domains[n_items] != NULL; n_items++);
     apr_file_printf(diag_cfg->fd,
@@ -640,6 +665,11 @@ am_diag_log_dir_cfg(request_rec *r, int level, am_dir_cfg_rec *cfg,
                         indent(level+2), cfg->redirect_domains[i]);
     }
 
+    apr_file_printf(diag_cfg->fd,
+                    "%sMellonSignatureMethod (signature_method): %s\n",
+                    indent(level+1),
+                    am_diag_signature_method_str(r, CFG_VALUE(cfg, signature_method)));
+
     apr_file_flush(diag_cfg->fd);
 }
 
diff --git a/auth_mellon_handler.c b/auth_mellon_handler.c
index 030abe7..3f15c0a 100644
--- a/auth_mellon_handler.c
+++ b/auth_mellon_handler.c
@@ -372,6 +372,8 @@ static LassoServer *am_get_lasso_server(request_rec *r)
 	    apr_thread_mutex_unlock(cfg->server_mutex);
 	    return NULL;
 	}
+
+        cfg->server->signature_method = CFG_VALUE(cfg, signature_method);
     }
 
     apr_thread_mutex_unlock(cfg->server_mutex);
diff --git a/configure.ac b/configure.ac
index dcee35a..3ab7176 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,6 +102,14 @@ AC_CHECK_HEADER([lasso/utils.h], LASSO_CFLAGS="$LASSO_CFLAGS -DHAVE_LASSO_UTILS_
 CFLAGS=$saved_CFLAGS
 CPPFLAGS=$saved_CPPFLAGS
 
+# Determine what definitions exist in Lasso
+saved_CFLAGS=$CFLAGS
+CFLAGS="$CFLAGS $pkg_cv_LASSO_CFLAGS"
+AC_CHECK_DECLS([LASSO_SIGNATURE_METHOD_RSA_SHA256,
+                LASSO_SIGNATURE_METHOD_RSA_SHA384,
+                LASSO_SIGNATURE_METHOD_RSA_SHA512,
+               ], [], [], [#include <lasso/xml/xml.h>])
+CFLAGS=$saved_CFLAGS
 
 # Create Makefile from Makefile.in
 AC_CONFIG_FILES([Makefile])