From f1952b74ed9bc95982b8128a809957ed4783dc7e Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 14 Oct 2020 15:47:56 -0400 Subject: [PATCH 1/2] Add test that exercises S4u2Proxy code This test shows that currently GssapiAcceptor {HOSTNAME} option will break the S4U2Proxy case. Signed-off-by: Simo Sorce [rharwood@redhat.com: nits] --- tests/httpd.conf | 15 +++++++++++++++ tests/magtests.py | 36 +++++++++++++++++++++--------------- tests/t_hostname_acceptor.py | 4 ++-- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/tests/httpd.conf b/tests/httpd.conf index 79be816..4672cde 100644 --- a/tests/httpd.conf +++ b/tests/httpd.conf @@ -238,6 +238,21 @@ CoreDumpDirectory "{HTTPROOT}" Require valid-user + + AuthType GSSAPI + AuthName "Login" + GssapiSSLonly Off + GssapiCredStore ccache:{HTTPROOT}/httpd_krb5_ccache + GssapiCredStore client_keytab:{HTTPROOT}/http.keytab + GssapiCredStore keytab:{HTTPROOT}/http.keytab + GssapiBasicAuth Off + GssapiAllowedMech krb5 + GssapiAcceptorName {{HOSTNAME}} + GssapiUseS4U2Proxy On + GssapiDelegCcacheDir {HTTPROOT}/delegccachedir + Require valid-user + + AuthType GSSAPI AuthName "Required Name Attributes" diff --git a/tests/magtests.py b/tests/magtests.py index ee17b5d..7316788 100755 --- a/tests/magtests.py +++ b/tests/magtests.py @@ -691,26 +691,32 @@ def test_no_negotiate(testdir, testenv, logfile): def test_hostname_acceptor(testdir, testenv, logfile): - hdir = os.path.join(testdir, 'httpd', 'html', 'hostname_acceptor') + plain_test_name = 'hostname_acceptor' + hdir = os.path.join(testdir, 'httpd', 'html', plain_test_name) os.mkdir(hdir) shutil.copy('tests/index.html', hdir) + proxy_test_name = 'hostname_proxy' + hdir = os.path.join(testdir, 'httpd', 'html', proxy_test_name) + os.mkdir(hdir) + shutil.copy('tests/index.html', hdir) + ddir = os.path.join(testdir, 'httpd', 'delegccachedir') + os.mkdir(ddir) + failed = False - for (name, fail) in [(WRAP_HOSTNAME, False), - (WRAP_ALIASNAME, False), - (WRAP_FAILNAME, True)]: - res = subprocess.Popen(["tests/t_hostname_acceptor.py", name], - stdout=logfile, stderr=logfile, - env=testenv, preexec_fn=os.setsid) - res.wait() - if fail: - if res.returncode == 0: - failed = True - else: - if res.returncode != 0: + for test_name in [plain_test_name, proxy_test_name]: + for (name, fail) in [(WRAP_HOSTNAME, False), + (WRAP_ALIASNAME, False), + (WRAP_FAILNAME, True)]: + res = subprocess.Popen(["tests/t_hostname_acceptor.py", + name, test_name], + stdout=logfile, stderr=logfile, + env=testenv, preexec_fn=os.setsid) + res.wait() + if (fail and res.returncode == 0) or \ + (not fail and res.returncode != 0): failed = True - if failed: - break + break if failed: sys.stderr.write('HOSTNAME ACCEPTOR: FAILED\n') diff --git a/tests/t_hostname_acceptor.py b/tests/t_hostname_acceptor.py index bb85700..0a07a0f 100755 --- a/tests/t_hostname_acceptor.py +++ b/tests/t_hostname_acceptor.py @@ -9,7 +9,7 @@ if __name__ == '__main__': sess = requests.Session() - url = 'http://%s/hostname_acceptor/' % sys.argv[1] + url = 'http://{}/{}/'.format(sys.argv[1], sys.argv[2]) r = sess.get(url, auth=HTTPKerberosAuth(delegate=True)) if r.status_code != 200: - raise ValueError('Hostname-based acceptor failed') + raise ValueError('Hostname acceptor ({}) failed'.format(sys.argv[2])) From 5a6e6be995995fea3d479e32dc8334f37529c579 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 14 Oct 2020 15:23:42 -0400 Subject: [PATCH 2/2] Special ccache handling for {HOSTNAME} acceptor This applies only to the case when GssapiS4U2Proxy is enabled. When using the {HOSTNAME} acceptor, the principal used in the server ccache can vary with each request. GSSAPI does not handle gracefully a request to resolve a ccache if there is already another credential under a different name. Even with ccache collections GSSAPI will resolve an existing ccache from the collection if any is available and throw an error if it does not match the desired_name. This even if there is a client_keytab that could be used to initiate a new cache in the collection with the right name. Therefore in case GssapiAcceptor is set to the special value {HOSTNAME}, instead of using the provided ccache or the process default ccache we create a new ccache named after the hostname in the delegated ccache directory. This directory is required when the S4U2Proxy mode is enabled so we are guaranteed to have it available an writable. Signed-off-by: Simo Sorce [rharwood@redhat.com: nits] --- src/mod_auth_gssapi.c | 62 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/src/mod_auth_gssapi.c b/src/mod_auth_gssapi.c index 2883ed9..99de884 100644 --- a/src/mod_auth_gssapi.c +++ b/src/mod_auth_gssapi.c @@ -194,6 +194,9 @@ static bool mag_conn_is_https(conn_rec *c) return false; } +static char *get_ccache_name(request_rec *req, char *dir, const char *name, + bool use_unique, apr_pool_t *pool); + static bool mag_acquire_creds(request_rec *req, struct mag_config *cfg, gss_OID_set desired_mechs, @@ -226,7 +229,52 @@ static bool mag_acquire_creds(request_rec *req, } #ifdef HAVE_CRED_STORE - gss_const_key_value_set_t store = cfg->cred_store; + gss_const_key_value_set_t store = NULL; + + /* When using multiple names, we need to use individual separate ccaches + * for each principal or gss_acquire_cred() on the default ccache will + * fail when names don't match. This is needed only for the s4u2proxy + * case, where we try to acquire proxy credentials. The lucky thing is + * that in this case we require the use of a delegated creedntials + * directory, so we just use this directory to also hold permanent ccaches + * for individual acceptor names. */ + if (cfg->acceptor_name_from_req && cfg->use_s4u2proxy && + cfg->deleg_ccache_dir) { + + gss_key_value_set_desc *s; + bool add = true; + char *ccname; + char *special_name; + + special_name = apr_psprintf(req->pool, "acceptor_%s", req->hostname); + ccname = get_ccache_name(req, cfg->deleg_ccache_dir, special_name, + false, req->pool); + + s = apr_pcalloc(req->pool, sizeof(gss_key_value_set_desc)); + s->count = cfg->cred_store->count; + s->elements = apr_pcalloc(req->pool, + (s->count + 1) * + sizeof(gss_key_value_element_desc)); + for (size_t i = 0; i < s->count; i++) { + gss_key_value_element_desc *el = &cfg->cred_store->elements[i]; + s->elements[i].key = el->key; + if (strcmp(el->key, "ccache") == 0) { + s->elements[i].value = ccname; + add = false; + } else { + s->elements[i].value = el->value; + } + } + if (add) { + s->elements[s->count].key = "ccache"; + s->elements[s->count].value = ccname; + s->count++; + } + + store = s; + } else { + store = cfg->cred_store; + } maj = gss_acquire_cred_from(&min, acceptor_name, GSS_C_INDEFINITE, desired_mechs, cred_usage, store, creds, @@ -287,8 +335,8 @@ static char *escape(apr_pool_t *pool, const char *name, return escaped; } -static char *get_ccache_name(request_rec *req, char *dir, const char *gss_name, - bool use_unique, struct mag_conn *mc) +static char *get_ccache_name(request_rec *req, char *dir, const char *name, + bool use_unique, apr_pool_t *pool) { char *ccname, *escaped; int ccachefd; @@ -297,15 +345,15 @@ static char *get_ccache_name(request_rec *req, char *dir, const char *gss_name, /* We need to escape away '/', we can't have path separators in * a ccache file name */ /* first double escape the esacping char (~) if any */ - escaped = escape(req->pool, gss_name, '~', "~~"); + escaped = escape(req->pool, name, '~', "~~"); /* then escape away the separator (/) if any */ escaped = escape(req->pool, escaped, '/', "~"); if (use_unique == false) { - return apr_psprintf(mc->pool, "%s/%s", dir, escaped); + return apr_psprintf(pool, "%s/%s", dir, escaped); } - ccname = apr_psprintf(mc->pool, "%s/%s-XXXXXX", dir, escaped); + ccname = apr_psprintf(pool, "%s/%s-XXXXXX", dir, escaped); umask_save = umask(0177); ccachefd = mkstemp(ccname); @@ -1297,7 +1345,7 @@ static int mag_complete(struct mag_req_cfg *req_cfg, struct mag_conn *mc, "requester: %s", mc->gss_name); ccache_path = get_ccache_name(req, cfg->deleg_ccache_dir, mc->gss_name, - cfg->deleg_ccache_unique, mc); + cfg->deleg_ccache_unique, mc->pool); if (ccache_path == NULL) { goto done; }