Re: [Freeipa-devel] [PATCH] 955 sessions: use unique mod_auth_gssapi ccaches

2016-03-19 Thread Petr Vobornik 

On 03/10/2016 03:25 PM, Simo Sorce wrote:

On Thu, 2016-03-10 at 15:03 +0100, Petr Vobornik wrote:

Attaching also mod_auth_gssapi patch. If the approach is good, then I'd
send it as a push request to upstream git repo.

Copr build of mod_auth_gssapi with the patch:
https://copr.fedorainfracloud.org/coprs/pvoborni/freeipa-4-3/build/167157/

IPA patch attached uses the functionality.

https://fedorahosted.org/freeipa/ticket/5653


I think the mod_auth_gssapi patch needs more work.


New iteration, but not a final patch, mostly because of reaping of the 
files, but there are also some debug prints.




For one you are not storing the generated ccname in the cookie, which
means any following request using mod_auth_gssapi sessions will not be
able to point to the ccache file.


Do you mean session? Cookie should contain only session ID, right?



It is also not clear to me why you are using a timestamp and not just
call something like mkstemp() with a template, and add an option called
GssapiDelegCcacheTemplate instead.


I didn't think about that.



The templated part would have to be saved in the session so that
following requests can keep using the same ccache file.


Fixed (but not tested yet)



There are other minor niticks around naming stuff, but those can be
handled in the PR.

One thing I am still undecided about is deletion of the files, I'd like
to have a better option than "application must delete them", I was
thinking about keeping a record of the expiration time (not sure where
yet), and then provide a cron job or a systemd timer to clean up all
expired stuff.


I thought we won't need it and that it could be handled by apps, but 
that won't work.


Case 1: ipa kerberize entire /ipa directory so a request to a random 
resource might leave a ccache behind, e.g.:
  curl -v --negotiate -u : --cacert /etc/ipa/ca.crt 
https://$(hostname)/ipa/foo


leaves:
ls /var/run/httpd/ipa/clientcaches/
   ipacchache-sgwB9v

Case 2: custodia, it doesn't clean anything as well.

When sessions are not it play then, the plugin can remove the ccache at 
the end of request. AFAIK mod_auth_kerb does it.


With sessions, there needs to be a reaper.



Simo.


--
Petr Vobornik
From 888004fa23b66785029d5218134c21467f05e03f Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Tue, 8 Mar 2016 19:38:59 +0100
Subject: [PATCH] Support unique credential cache names

Add new configuration option:
  GssapiDelegCcacheTemplate ccacheXX

To enable unique names of credential caches. It allows consuming application
to work on top of the cache, without affecting other concurent http requests
of the same principal.
---
 README | 11 +
 src/asn1c/GSSSessionData.c | 17 ++---
 src/asn1c/GSSSessionData.h |  2 +-
 src/asn1c/Uint32.c |  1 -
 src/asn1c/Uint32.h |  1 -
 src/asn1c/session.asn1 |  3 ++-
 src/environ.c  | 12 +++--
 src/mod_auth_gssapi.c  | 61 --
 src/mod_auth_gssapi.h  |  8 --
 src/sessions.c |  8 ++
 10 files changed, 98 insertions(+), 26 deletions(-)

diff --git a/README b/README
index b4eca28..d9cf320 100644
--- a/README
+++ b/README
@@ -171,6 +171,17 @@ A user f...@example.com delegating its credentials would cause the server to
 create a ccache file named /var/run/httpd/clientcaches/f...@example.com
 
 
+### GssapiDelegCcacheTemplate
+
+A template for credential cache name. Must end with XX before file name
+suffix.
+
+**Note:** Consuming application must delete the ccache otherwise it will
+litter the filesystem.
+
+ Example
+GssapiDelegCcacheTemplate ccacheXX
+
 ### GssapiUseS4U2Proxy
 
 Enables the use of the s4u2Proxy Kerberos extension also known as
diff --git a/src/asn1c/GSSSessionData.c b/src/asn1c/GSSSessionData.c
index 12a98e3..f0dcc2e 100644
--- a/src/asn1c/GSSSessionData.c
+++ b/src/asn1c/GSSSessionData.c
@@ -2,7 +2,6 @@
  * Generated by asn1c-0.9.27 (http://lionet.info/asn1c)
  * From ASN.1 module "GssapiSessionModule"
  * 	found in "session.asn1"
- * 	`asn1c -fskeletons-copy`
  */
 
 #include "GSSSessionData.h"
@@ -62,6 +61,15 @@ static asn_TYPE_member_t asn_MBR_GSSSessionData_1[] = {
 		0,
 		"basichash"
 		},
+	{ ATF_NOFLAGS, 0, offsetof(struct GSSSessionData, ccname),
+		(ASN_TAG_CLASS_CONTEXT | (6 << 2)),
+		+1,	/* EXPLICIT tag at current level */
+		_DEF_OCTET_STRING,
+		0,	/* Defer constraints checking to the member type */
+		0,	/* PER is not compiled, use -gen-PER */
+		0,
+		"ccname"
+		},
 };
 static ber_tlv_tag_t asn_DEF_GSSSessionData_tags_1[] = {
 	(ASN_TAG_CLASS_UNIVERSAL | (16 << 2))
@@ -72,13 +80,14 @@ static asn_TYPE_tag2member_t asn_MAP_GSSSessionData_tag2el_1[] = {
 { (ASN_TAG_CLASS_CONTEXT | (2 << 2)), 2, 0, 0 }, /* expiration */
 { (ASN_TAG_CLASS_CONTEXT | (3 << 2)), 3, 0, 0 }, /* username */
 { (ASN_TAG_CLASS_CONTEXT | (4 << 2)), 4, 0, 0 }, /* gssname */
-{ (ASN_TAG_CLASS_CONTEXT | (5 << 2)),

Re: [Freeipa-devel] [PATCH] 955 sessions: use unique mod_auth_gssapi ccaches

2016-03-19 Thread Simo Sorce 
- Original Message -
> From: "Petr Vobornik" 
> To: "Simo Sorce" 
> Cc: "freeipa-devel" 
> Sent: Wednesday, March 16, 2016 12:16:02 PM
> Subject: Re: [PATCH] 955 sessions: use unique mod_auth_gssapi ccaches
> 
> On 03/10/2016 03:25 PM, Simo Sorce wrote:
> > On Thu, 2016-03-10 at 15:03 +0100, Petr Vobornik wrote:
> >> Attaching also mod_auth_gssapi patch. If the approach is good, then I'd
> >> send it as a push request to upstream git repo.
> >>
> >> Copr build of mod_auth_gssapi with the patch:
> >> https://copr.fedorainfracloud.org/coprs/pvoborni/freeipa-4-3/build/167157/
> >>
> >> IPA patch attached uses the functionality.
> >>
> >> https://fedorahosted.org/freeipa/ticket/5653
> >
> > I think the mod_auth_gssapi patch needs more work.
> 
> New iteration, but not a final patch, mostly because of reaping of the
> files, but there are also some debug prints.
> 
> >
> > For one you are not storing the generated ccname in the cookie, which
> > means any following request using mod_auth_gssapi sessions will not be
> > able to point to the ccache file.
> 
> Do you mean session? Cookie should contain only session ID, right?

No, in other to avoid having to keep state on the server we create a session 
storage
structure, encrypt it in a key known only to the server, and send it as a 
cookie.

This way we do not have to store anything on the server side.

> >
> > It is also not clear to me why you are using a timestamp and not just
> > call something like mkstemp() with a template, and add an option called
> > GssapiDelegCcacheTemplate instead.
> 
> I didn't think about that.
> 
> >
> > The templated part would have to be saved in the session so that
> > following requests can keep using the same ccache file.
> 
> Fixed (but not tested yet)
> 
> >
> > There are other minor niticks around naming stuff, but those can be
> > handled in the PR.
> >
> > One thing I am still undecided about is deletion of the files, I'd like
> > to have a better option than "application must delete them", I was
> > thinking about keeping a record of the expiration time (not sure where
> > yet), and then provide a cron job or a systemd timer to clean up all
> > expired stuff.
> 
> I thought we won't need it and that it could be handled by apps, but
> that won't work.
> 
> Case 1: ipa kerberize entire /ipa directory so a request to a random
> resource might leave a ccache behind, e.g.:
>curl -v --negotiate -u : --cacert /etc/ipa/ca.crt
> https://$(hostname)/ipa/foo
> 
> leaves:
> ls /var/run/httpd/ipa/clientcaches/
> ipacchache-sgwB9v
> 
> Case 2: custodia, it doesn't clean anything as well.
> 
> When sessions are not it play then, the plugin can remove the ccache at
> the end of request. AFAIK mod_auth_kerb does it.
> 
> With sessions, there needs to be a reaper.

Exactly.

Simo.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 955 sessions: use unique mod_auth_gssapi ccaches

2016-03-10 Thread Simo Sorce 
On Thu, 2016-03-10 at 15:03 +0100, Petr Vobornik wrote:
> Attaching also mod_auth_gssapi patch. If the approach is good, then I'd 
> send it as a push request to upstream git repo.
> 
> Copr build of mod_auth_gssapi with the patch: 
> https://copr.fedorainfracloud.org/coprs/pvoborni/freeipa-4-3/build/167157/
> 
> IPA patch attached uses the functionality.
> 
> https://fedorahosted.org/freeipa/ticket/5653

I think the mod_auth_gssapi patch needs more work.

For one you are not storing the generated ccname in the cookie, which
means any following request using mod_auth_gssapi sessions will not be
able to point to the ccache file.

It is also not clear to me why you are using a timestamp and not just
call something like mkstemp() with a template, and add an option called
GssapiDelegCcacheTemplate instead.

The templated part would have to be saved in the session so that
following requests can keep using the same ccache file.

There are other minor niticks around naming stuff, but those can be
handled in the PR.

One thing I am still undecided about is deletion of the files, I'd like
to have a better option than "application must delete them", I was
thinking about keeping a record of the expiration time (not sure where
yet), and then provide a cron job or a systemd timer to clean up all
expired stuff.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] 955 sessions: use unique mod_auth_gssapi ccaches

2016-03-10 Thread Petr Vobornik 
Attaching also mod_auth_gssapi patch. If the approach is good, then I'd 
send it as a push request to upstream git repo.


Copr build of mod_auth_gssapi with the patch: 
https://copr.fedorainfracloud.org/coprs/pvoborni/freeipa-4-3/build/167157/


IPA patch attached uses the functionality.

https://fedorahosted.org/freeipa/ticket/5653
--
Petr Vobornik
From 069842cc93aa17a8e3be66b785678ee862b5ab24 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 10 Mar 2016 14:53:43 +0100
Subject: [PATCH] sessions: use unique mod_auth_gssapi ccaches

Same ccache name causes issue in concurrent http requests because it is
deleted at the end of request so other requests might not have it available
and then fail.

https://fedorahosted.org/freeipa/ticket/5653
---
 freeipa.spec.in   | 2 +-
 install/conf/ipa.conf | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index bc47df4c916bd8f091fc2f70330d95bd116ad187..e04df39a8d73f99cf4e189b042d5beb4720fab04 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -140,7 +140,7 @@ Requires: cyrus-sasl-gssapi%{?_isa}
 Requires: ntp
 Requires: httpd >= 2.4.6-6
 Requires: mod_wsgi
-Requires: mod_auth_gssapi >= 1.3.0-2
+Requires: mod_auth_gssapi >= 1.3.2-2
 Requires: mod_nss >= 1.0.8-26
 Requires: python-ldap >= 2.4.15
 Requires: python-gssapi >= 1.1.2
diff --git a/install/conf/ipa.conf b/install/conf/ipa.conf
index 8d4fea35e9c5aa15a611d90fe005090abb1d8e50..3ce3d0e05bffae86687dea1a26231ec968417bda 100644
--- a/install/conf/ipa.conf
+++ b/install/conf/ipa.conf
@@ -1,5 +1,5 @@
 #
-# VERSION 19 - DO NOT REMOVE THIS LINE
+# VERSION 20 - DO NOT REMOVE THIS LINE
 #
 # This file may be overwritten on upgrades.
 #
@@ -65,6 +65,7 @@ WSGIScriptReloading Off
   GssapiCredStore keytab:/etc/httpd/conf/ipa.keytab
   GssapiCredStore client_keytab:/etc/httpd/conf/ipa.keytab
   GssapiDelegCcacheDir /var/run/httpd/ipa/clientcaches
+  GssapiUseUniqueCcacheName On
   GssapiUseS4U2Proxy on
   GssapiAllowedMech krb5
   Require valid-user
-- 
2.5.0

From bb7d486b41891860b5ec39c1b7f7ade5a8d641b1 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Tue, 8 Mar 2016 19:38:59 +0100
Subject: [PATCH] Support unique credential cache names

Add new configuration option:
  GssapiUseUniqueCcacheName On

To enable unique names of credential caches. It allows consuming application
to work on top of the cache, without affecting other concurent http requests
of the same principal.
---
 README| 11 +++
 src/environ.c | 12 +++-
 src/mod_auth_gssapi.c | 35 +++
 src/mod_auth_gssapi.h |  6 --
 4 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/README b/README
index b4eca28..708532f 100644
--- a/README
+++ b/README
@@ -171,6 +171,17 @@ A user f...@example.com delegating its credentials would cause the server to
 create a ccache file named /var/run/httpd/clientcaches/f...@example.com
 
 
+### GssapiUseUniqueCcacheName
+
+If credentials are exported to private directory set by GssapiDelegCcacheDir,
+the delegate credentials will use unique credentials cache name.
+
+**Note:** Consuming application must delete the ccache otherwise it will
+litter the filesystem.
+
+ Example
+GssapiUseUniqueCcacheName On
+
 ### GssapiUseS4U2Proxy
 
 Enables the use of the s4u2Proxy Kerberos extension also known as
diff --git a/src/environ.c b/src/environ.c
index f9bbf30..edf31cc 100644
--- a/src/environ.c
+++ b/src/environ.c
@@ -243,7 +243,7 @@ static void mag_set_name_attributes(request_rec *req, struct mag_conn *mc)
 }
 }
 
-static void mag_set_KRB5CCANME(request_rec *req, char *ccname)
+static void mag_set_KRB5CCANME(request_rec *req, const char *ccname)
 {
 apr_status_t status;
 apr_finfo_t finfo;
@@ -277,14 +277,8 @@ void mag_set_req_data(request_rec *req,
 }
 
 #ifdef HAVE_CRED_STORE
-if (cfg->deleg_ccache_dir && mc->delegated) {
-char *ccname;
-ccname = mag_gss_name_to_ccache_name(req,
- cfg->deleg_ccache_dir,
- mc->gss_name);
-if (ccname) {
-mag_set_KRB5CCANME(req, ccname);
-}
+if (cfg->deleg_ccache_dir && mc->delegated && mc->ccname) {
+mag_set_KRB5CCANME(req, mc->ccname);
 }
 #endif
 }
diff --git a/src/mod_auth_gssapi.c b/src/mod_auth_gssapi.c
index 97e365c..41b7a56 100644
--- a/src/mod_auth_gssapi.c
+++ b/src/mod_auth_gssapi.c
@@ -225,9 +225,10 @@ static char *escape(apr_pool_t *pool, const char *name,
 }
 
 char *mag_gss_name_to_ccache_name(request_rec *req,
-  char *dir, const char *gss_name)
+  char *dir, const char *gss_name, bool unique)
 {
 char *escaped;
+struct timespec now;
 
 /* We need to escape away '/', we can't have path separators in
  * a ccache file name */
@@ -236,22 +237,25 @@ char