Re: [Freeipa-devel] [PATCH] 479 optimize modify principal operations
Rob Crittenden wrote: Simo Sorce wrote: We were unconditionally searching the LDAP database to find the principal on modification. By using the stored entry_dn when available we avoid a costly round-trip to the LDAP server just to save back some modified attributes. This is an important performance improvement for the KDC given now we save data each time an AS request is performed. Simo. ACK Pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 480 Do not store LastPwdChange unless it really changed
Rob Crittenden wrote: Simo Sorce wrote: Due to an idiosyncrasy of kadmin, the right flag to indicate krbLastPwdChange is changed is not set. The previous check ended up always saving the data in all cases because the data was always present. Restrict it to store a password change when there is actually new key material. This prevents also audit operations to cause replications. Simo. ACK Pushed to master and ipa-2-2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] ignore pylint warning
A false pylint warning was being printed on F16+. I pushed a patch to
master and ipa-2-2 to disable it under the 1-liner rule.
rob
From 06627a542ff45a9eaa3aa1f125cb86b5aa273b16 Mon Sep 17 00:00:00 2001
From: Rob Crittenden
Date: Wed, 15 Feb 2012 10:33:11 -0500
Subject: [PATCH] Disable false pylint error in freeipa-systemd-upgrade
---
init/systemd/freeipa-systemd-upgrade |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/init/systemd/freeipa-systemd-upgrade
b/init/systemd/freeipa-systemd-upgrade
index e662d28..1f1a616 100755
--- a/init/systemd/freeipa-systemd-upgrade
+++ b/init/systemd/freeipa-systemd-upgrade
@@ -80,7 +80,7 @@ try:
# when enabling DS instances we'll also do configure
/etc/sysconfig/dirsrv.systemd
# which comes with 389-ds-base-1.2.10-0.8.a7 on F-16 and later.
This is handled in
# fedora16 platform code
-realm = config.config.default_realm.upper().replace('.','-')
+realm = config.config.default_realm.upper().replace('.','-')
#pylint: disable=E1103
print "Re-enable Directory server instances PKI-IPA and %s " %
(realm)
if os.path.exists('/etc/systemd/system/dirsrv@.service'):
os.unlink('/etc/systemd/system/dirsrv@.service')
--
1.7.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] configure ipa_memcached on upgraded instances
The ipa_memcached service wasn't being added to upgraded installations. We also were never unconfiguring it on uninstall and we were missing this fact, both fixed. rob freeipa-rcrit-951-memcacheupgrade.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 61] Cache authentication in session
On Wed, 2012-02-15 at 11:24 -0600, Endi Sukma Dewata wrote: > On 2/15/2012 8:01 AM, Simo Sorce wrote: > >> If someone already has a session then changes to another principal, will > >> he get a new session and then reauthenticate against the login URL? Or > >> will he use the same session but just reauthenticate? > > > > I guess it depends on what you mean by "changes to another principal". > > If you mean a simple kdestroy/kinit, then the interface will not see the > > change until the session expires. (Which is why I want a session > > expiration much shorter for Negotiate auth). > > Even if we make it shorter (e.g. 5 minutes) there's still a window where > the browser is still using the old session. Will this be OK since it's > not about somebody leaving the machine idle, but in this case someone > else is taking over the machine while there's an active session? Also, > both users might not be aware about the length of this window. My preferred way would be to have it configurable, but I think 5 min. would be a reasonable compromise. > > However if the user presses the logout button in the UI his session will > > be destroyed and so the next attempt will cause a new negotiation to > > happen and the UI will change to use the new principal. > > Right, this is not the concern. > > >> What if he then changes back to the previous principal, will he reuse > >> the old session? If so will he be required to authenticate again? > > > > What old session do you refer to ? > > The previous session of the same user has been destroyed, the other user > > session is still valid until it expires or the user presses log out, so > > nothing changes there. > > Assuming there is a way for the server to detect principal change > earlier than session expiration, There is none without a Negotiate operation which we now do only when the session expires (hence my request for short expiration times). > the server could either (a) reuse the > same session but with new credentials, or (b) it could create a new > session for the new user. In option (b) there's a possibility to keep > the old session (thus the question about reusing the old session), but I > don't think we want to do that. > > If the assumption is wrong, the only possible option is (b), but in this > case the old session is no longer available. Right, the old session is always destroyed. > That's true if the above assumption is wrong. We're already planning to > execute a whoami operation after each authentication so the UI can > detect the change. ok > >> Would it be possible for someone from another machine to randomly guess > >> a session ID and then take over an existing authenticated session? > > > > The session ID is currently a 48bit random number (we ask John to make > > it a 128bit number). *Guessing* that number is going to be *very* though > > (read technically impossible if the randomness is properly handled at > > the creation of the session ID in the server. > > Hey... many people have won the lottery :) I know nobody, do you? :-) > > However if somehow the sessionid is exposed than an attacker can hijack > > the connection. The risk is extremely low, but it is another reason why > > having a short expiration for Negotiate auth would be better, than > > treating it the same as form based auth. > > Agreed, although the form based auth is still exposed to the same > hijacking issue. Yes form based auth is just a leap of faith, you base all your security on the SSL connection, and there isn't much you can do beyond that. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 61] Cache authentication in session
On 2/15/2012 8:01 AM, Simo Sorce wrote: If someone already has a session then changes to another principal, will he get a new session and then reauthenticate against the login URL? Or will he use the same session but just reauthenticate? I guess it depends on what you mean by "changes to another principal". If you mean a simple kdestroy/kinit, then the interface will not see the change until the session expires. (Which is why I want a session expiration much shorter for Negotiate auth). Even if we make it shorter (e.g. 5 minutes) there's still a window where the browser is still using the old session. Will this be OK since it's not about somebody leaving the machine idle, but in this case someone else is taking over the machine while there's an active session? Also, both users might not be aware about the length of this window. However if the user presses the logout button in the UI his session will be destroyed and so the next attempt will cause a new negotiation to happen and the UI will change to use the new principal. Right, this is not the concern. What if he then changes back to the previous principal, will he reuse the old session? If so will he be required to authenticate again? What old session do you refer to ? The previous session of the same user has been destroyed, the other user session is still valid until it expires or the user presses log out, so nothing changes there. Assuming there is a way for the server to detect principal change earlier than session expiration, the server could either (a) reuse the same session but with new credentials, or (b) it could create a new session for the new user. In option (b) there's a possibility to keep the old session (thus the question about reusing the old session), but I don't think we want to do that. If the assumption is wrong, the only possible option is (b), but in this case the old session is no longer available. I think if the principal change is always preceded by reauthentication, the UI will only need to redo the whoami after successful login. Otherwise, the UI will need to check the principal change after each operation like in the current code. The UI will know when the principal changes because it will get an error and it will be told to go to the re-authentication URL. When you re-authenticate you should always check if the principal changed. That's true if the above assumption is wrong. We're already planning to execute a whoami operation after each authentication so the UI can detect the change. Would it be possible for someone from another machine to randomly guess a session ID and then take over an existing authenticated session? The session ID is currently a 48bit random number (we ask John to make it a 128bit number). *Guessing* that number is going to be *very* though (read technically impossible if the randomness is properly handled at the creation of the session ID in the server. Hey... many people have won the lottery :) However if somehow the sessionid is exposed than an attacker can hijack the connection. The risk is extremely low, but it is another reason why having a short expiration for Negotiate auth would be better, than treating it the same as form based auth. Agreed, although the form based auth is still exposed to the same hijacking issue. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 61] Cache authentication in session
On 2/15/2012 2:51 AM, Dmitri Pal wrote: On 02/14/2012 01:01 PM, Endi Sukma Dewata wrote: If someone already has a session then changes to another principal, will he get a new session and then reauthenticate against the login URL? Or will he use the same session but just reauthenticate? AFAIU the session ID will be (should be) lost when you switch the principal and new ID returned to the browser. The old session is still valid for 20 min if you did not log out but it should not be accessible. I looked at the code briefly, if I understand correctly it looks like the server will get the session ID from the cookie and use it to find the existing session data from memcached. So if someone does a kinit to change principal, the browser will still send the same session ID because it doesn't know about the change, so the server will continue to use the old session with the old credentials until the session expires (which could be a long time if the server keeps resetting the expiration time). Previously without sessions the UI will detect the change as soon as the user executes another operation in the UI. It seems that everything depends on what is the key for the internal cache. I would think that the key should be principal+session ID and session ID is long and strong. We said in one of the earlier discussions that it should be at least 128 bits of entropy. Was that done? If not please log a ticket. Assuming the principal will be stored in the cookie, it might not work because the browser will not know about the principal change so it's still sending the same principal. So am I right that the key is principal+session ID? If it is just principal then yes the session can be resurrected but I do not think we want this. If the key is just the ID there is a potential of the privilege escalation if one principal tries to guess the ID for another active session. Very unlikely but it seems that it is safer to have it as a composit key. May be we should file an RFE on this. Right now the key is just a 48-bit ID. What if he then changes back to the previous principal, will he reuse the old session? If so will he be required to authenticate again? Old session should not be accessible. The question assumes that if the user changes principal he will get a new session, which doesn't seem to be the case. But suppose that issue is fixed, I agree that we need to make sure the old session is destroyed. Would it be possible for someone from another machine to randomly guess a session ID and then take over an existing authenticated session? Then we need to make ID even longer. I believe John is going to make it 128-bit in the next patch. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 950 update replicas for s4u2proxy
On Wed, 2012-02-15 at 11:22 -0500, Rob Crittenden wrote: > I handled installing a new replica for s4u2proxy but not upgrading a > previous one. This should take care of adding the right delegation > permissions. > > rob ACK. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 086 Fixed problem when attributes_widget was displaying empty option
Attribute table was modified to skip creation of option for empty value.
https://fedorahosted.org/freeipa/ticket/2291
--
Petr Vobornik
From fb0df6e11f9d105e58ae6e08c15ff270f8c2206a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Voborn=C3=ADk?=
Date: Wed, 15 Feb 2012 17:47:53 +0100
Subject: [PATCH] Fixed problem when attributes_widget was displaying empty
option
Attribute table was modified to skip creation of option for empty value.
https://fedorahosted.org/freeipa/ticket/2291
---
install/ui/aci.js |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index fc72b51929e94bb73dae5c01b6e1ecf50b6e8303..02e03f309f279d62f411e3b9ffd8bae9fe09dc06 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -487,7 +487,12 @@ IPA.attributes_widget = function(spec) {
values = values || [];
for (var i=0; i___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 949 remove Apache ccache on upgrade
On Wed, 2012-02-15 at 09:58 -0500, Rob Crittenden wrote: > Remove any Apache ccache on upgrade. We have no way of knowing what is > in there so play it safe. > > rob ACK. This fixes the upgrade issue. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 947 fix synxtax in 30-s4u2proxy.update
On Wed, 2012-02-15 at 11:23 +0100, Martin Kosek wrote:
> On Tue, 2012-02-14 at 15:51 -0500, Rob Crittenden wrote:
> > Martin Kosek wrote:
> > > On Mon, 2012-02-13 at 11:43 -0500, Rob Crittenden wrote:
> > >> Remove quotes around a value in 30-s4u2proxy.update. The update was
> > >> failing to apply.
> > >>
> > >> I also noticed that FQDN wasn't being set properly in all cases in
> > >> sub_dict. This should fix it.
> > >>
> > >> rob
> > >
> > > This patch did not apply for me. I guess it depends on some other patch
> > > that fixes wrong DN in s4u2proxy ipaAllowedTargets:
> > >
> > > -default: ipaAllowedTarget:
> > > 'cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX'
> > > +default: ipaAllowedTarget:
> > > cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX
> > >
> > > Current update file says:
> > >
> > > default: ipaAllowedTarget: 'cn=ipa-ldap-delegation-targets,cn=etc,$SUFFIX'
> > >
> > > which is a non-existent DN.
> > >
> > > Martin
> > >
> >
> > It relies on patch 941
>
> Yeah, that's the one.
>
> I am now testing all the upgrade patches, but I s4u2proxy does not work
> for me yet on upgraded server instance (tested on F16). krb5kdc keeps
> reporting decrypt errors:
>
> /var/log/krb5kdc.log:
> Feb 15 04:25:43 vm-068.idm.lab.bos.redhat.com krb5kdc[872](info):
> TGS_REQ (4 etypes {18 17 16 23}) 10.16.78.68: PROCESS_TGS: authtime 0,
> for , Decrypt integrity check failed
> Feb 15 04:25:43 vm-068.idm.lab.bos.redhat.com krb5kdc[872](info):
> TGS_REQ (4 etypes {18 17 16 23}) 10.16.78.68: PROCESS_TGS: authtime 0,
> for , Decrypt integrity check failed
>
> New installs on the same machine work though. I am still trying to find
> out the root cause of this.
>
> Martin
>
Ok, we found out the root cause. The problem was that Apache CCACHE from
previous install was not removed. Rob's patch 949 fixes that.
ACK for this patch. Pushed to master, ipa-2-2.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 941 fix replica s4u2proxy delegation
On Wed, 2012-02-15 at 11:30 +0100, Martin Kosek wrote: > On Mon, 2012-02-06 at 15:12 -0500, Rob Crittenden wrote: > > Replicas weren't being added to the S4U2Proxy configuration so weren't > > allowed to delegate tickets. This fixes things. > > > > rob > > This would work fine for new replicas which adds replica-s4u2proxy.ldif > but what about upgrading the current ones? > > I am not sure that 30-s4u2proxy.update would add their principals to > s4u2proxy configuration. > > Martin > Discussed with Rob, it is really an issue. He plans to deal with it in another ticket. In that case its ACK. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 950 update replicas for s4u2proxy
I handled installing a new replica for s4u2proxy but not upgrading a previous one. This should take care of adding the right delegation permissions. rob freeipa-rcrit-950-replica.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] s4u2proxy support
On Wed, 2012-01-04 at 15:11 -0500, Rob Crittenden wrote:
> Alexander Bokovoy wrote:
> > On Wed, 14 Dec 2011, Rob Crittenden wrote:
> >
> >> Dmitri Pal wrote:
> >>> On 12/12/2011 07:15 PM, Simo Sorce wrote:
> On Mon, 2011-12-12 at 15:22 -0500, Rob Crittenden wrote:
> > This patch adds support for s4u2proxy. This means that the Apache
> > server
> > will obtain the ldap service ticket on behalf of the user rather than
> > the using having to send their TGT. The user's ticket still needs to
> > be
> > forwardable, we just don't require it to be forwarded any more.
>
> Should we make the patch allow the old behavior by using a switch that
> revert to forwarding the TGT ?
>
> It would be useful during upgrades if some of your servers still need
> forwarded TGTs, or if you want to use a newer client against an old
> server while you have the newer stuff under test.
> (And to test in general).
>
> Simo.
> >>> +1
> >>>
> >>
> >> Updated patch attached.
> >>
> >> rob
> >
> >> > From 03a2c9a536811437e4847e1c6b11d2ac0eff98f2 Mon Sep 17 00:00:00 2001
> >> From: Rob Crittenden
> >> Date: Thu, 8 Dec 2011 14:23:18 -0500
> >> Subject: [PATCH] Don't set delegation flag in client, we're using S4U2Proxy
> >> now
> >>
> >> A forwardable ticket is still required but we no longer need to send
> >> the TGT to the IPA server. A new flag, --delegation, is available if
> >> the old behavior is required.
> > A minor point: please fix commit message to use proper option name:
> >
> > --delegate
> >
> >> +parser.add_option('--delegate', action='store_true',
> >> +help='Delegate the TGT to the IPA server',
> >> +)
> >
> > Otherwise ACK.
> >
>
> Updated both patches. The first (914) to address Alexander's concern.
> The second to add a new global lock directive. I updated the
> mod_auth_kerb patch based on feedback from the package maintainer.
>
> rob
ACK for patch 914-4. Pushed to master, ipa-2-2.
In reality, it was really sent in the thread for patch 947. I just
renamed it and created a rebased version for master branch. Both patches
are attached.
Martin
>From d78756d9bcaf1d75cc592fc621e25c4d1df980a3 Mon Sep 17 00:00:00 2001
From: Rob Crittenden
Date: Thu, 8 Dec 2011 14:23:18 -0500
Subject: [PATCH] Don't set delegation flag in client, we're using S4U2Proxy
now
A forwardable ticket is still required but we no longer need to send
the TGT to the IPA server. A new flag, --delegate, is available if
the old behavior is required.
Set the minimum n-v-r for mod_auth_kerb and krb5-server to pick up
needed patches for S4U2Proxy to work.
https://fedorahosted.org/freeipa/ticket/1098
https://fedorahosted.org/freeipa/ticket/2246
---
freeipa.spec.in | 15 +++
install/share/bootstrap-template.ldif |2 +-
ipa.1 |3 +++
ipalib/backend.py |2 +-
ipalib/constants.py |1 +
ipalib/plugable.py|5 -
ipalib/rpc.py | 24 +---
7 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 6c92747..06e7d9e 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -92,17 +92,13 @@ Requires(pre): 389-ds-base >= 1.2.10-0.5.a5
Requires: openldap-clients
Requires: nss
Requires: nss-tools
-%if 0%{?fedora} >= 16
-Requires: krb5-server >= 1.9.1-15
-%else
-Requires: krb5-server
-%endif
+Requires: krb5-server >= 1.9.2-6
Requires: krb5-pkinit-openssl
Requires: cyrus-sasl-gssapi%{?_isa}
Requires: ntp
Requires: httpd
Requires: mod_wsgi
-Requires: mod_auth_kerb >= 5.4-9
+Requires: mod_auth_kerb >= 5.4-8
Requires: mod_nss >= 1.0.8-10
Requires: python-ldap
Requires: python-krbV
@@ -665,11 +661,14 @@ fi
%ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
%changelog
-* Wed Jan 11 2012 Simo Sorce - 2.2.0-8
+- Set min for krb5-server to 1.9.2-6 to pick up needed s4u2proxy patches
+
+* Wed Jan 11 2012 Simo Sorce - 2.2.0-7
- Remove dependency on samba4 libs
* Wed Jan 11 2012 Rob Crittenden - 2.2.0-6
-- Set min for mod_auth_kerb to 5.4-9 to pick up s4u2proxy support
+- Set min for mod_auth_kerb to 5.4-8 to pick up s4u2proxy support
* Tue Jan 10 2012 Alexander Bokovoy - 2.2.0-5
- Fix dependency for samba4-devel package
diff --git a/install/share/bootstrap-template.ldif b/install/share/bootstrap-template.ldif
index b58bfd7..e33f065 100644
--- a/install/share/bootstrap-template.ldif
+++ b/install/share/bootstrap-template.ldif
@@ -174,7 +174,7 @@ objectClass: groupOfPrincipals
objectClass: top
cn: ipa-http-delegation
memberPrincipal: HTTP/$HOST@$REALM
-ipaAllowedTarget: cn=ipa-ldap-delegation-targets,cn=etc,$SUFFIX
+ipaAllowedTarget: cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX
dn: cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX
changetype: add
diff --git a/i
Re: [Freeipa-devel] [PATCH] 480 Do not store LastPwdChange unless it really changed
Simo Sorce wrote: Due to an idiosyncrasy of kadmin, the right flag to indicate krbLastPwdChange is changed is not set. The previous check ended up always saving the data in all cases because the data was always present. Restrict it to store a password change when there is actually new key material. This prevents also audit operations to cause replications. Simo. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 479 optimize modify principal operations
Simo Sorce wrote: We were unconditionally searching the LDAP database to find the principal on modification. By using the stored entry_dn when available we avoid a costly round-trip to the LDAP server just to save back some modified attributes. This is an important performance improvement for the KDC given now we save data each time an AS request is performed. Simo. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 940 apply some validation to some classes only
On 15.2.2012 15:33, Rob Crittenden wrote: Jan Cholasta wrote: On 14.2.2012 22:16, Rob Crittenden wrote: Jan Cholasta wrote: On 14.2.2012 16:44, Jan Cholasta wrote: On 7.2.2012 20:25, Rob Crittenden wrote: Rob Crittenden wrote: Jan Cholasta wrote: Dne 7.2.2012 09:27, Martin Kosek napsal(a): On Mon, 2012-02-06 at 11:52 -0500, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2012-02-03 at 16:58 -0500, Rob Crittenden wrote: There is some validation that we only need to apply when an entry is being created, namely the key itself. This is to allow us to manage an otherwise illegal entry that finds its way into the system (i.e. migration). Consider this. We migrate a group with a space in it. This isn't allowed in IPA but we also provide no way to delete it because the cn regex kicks out the group-del command. The trick is adding appropriate context so we can know during validation how we got here. A command object has a bases field which contains the base classes associated with it, which appears to contain only the leaf baseclass. So using this we can tell how we got to validation and can skip based on that baseclass name. I went back and forth a bit on where the right place to put this was, I'm open to more fine tuning. I initially skipped just the pattern validation then expanded it to apply to all validation in the Parameter. rob 1) This patch breaks API.txt, it needs re-generating. 2) This may be a matter of opinion but I think that + if self.onlyvalidateclasses and \ + not any(x in self.onlyvalidateclasses for x in classbases): + return would be more readable than: + if self.onlyvalidateclasses and \ + not [x for x in classbases if x in self.onlyvalidateclasses]: + return 3) I would move the entire self.onlyvalidateclasses up to the validate() method. It would have several benefits: - the validation skip would be done just once, not for every value as the decision does not use the value at all - we would not have to modify _validate_scalar() methods at all since they won't need classbases 4) I think it would be better to keep validation for --rename options. As it is generated from RDN attribute parameter, it inherits onlyvalidateclasses list as well. Otherwise your patch would let user to rename a valid object to an invalid one: # ipa user-mod fbar --rename="bad boy" Modified user "fbar" User login: bad boy First name: Foo Last name: Bar Home directory: /home/fbar Login shell: /bin/sh UID: 480800040 GID: 480800040 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False Martin This should address your concerns. rob Its almost OK, there is just one part that's not that OK: @@ -831,6 +836,9 @@ class Param(ReadOnly): else: raise RequirementError(name=self.name) return + if 'rename' not in (self.name, self.cli_name): + if self.onlyvalidateclasses and not [x for x in self.onlyvalidateclasses if x in classbases]: #pylint: disable=E1101 + return if self.multivalue: if type(value) is not tuple: raise TypeError( I don't think that hard-coding this skip for onlyvalidateclasses based just on parameter name is a good thing to do and may cause problems in the future. For example if we create some option called "rename" and deliberately set onlyvalidateclasses for the option - it would then be skipped as well. I think it would be a better solution to just update _get_rename_option() in baseldap.py to set onlyvalidateclasses to () Martin I must say I'm not a big fan of this approach. You are healing the symptoms (custom validation on some parameters makes it impossible to manipulate existing LDAP entries with invalid attribute values => add a way to mark parameters to be validated only in certain commands to prevent that), but the real issue here is that we should not perform custom validation when referring to existing LDAP attribute values at all (or only partially), no matter what parameter and command. Fixing this would make the problem go away for all commands, present or future, without the need for adding a list of command classes to each parameter that is affected. Honza It is all about context, and parameters have very little of it. The bottom line is we only want to do validation on new values. I think I can bump this up a level by adding a validate boolean to Param and Command both defaulting to False. crud.Create will override this to True and cloning rename could perhaps also set this flag. Then in validation if the parent object has validation set or the parameter does we validate, otherwise we skip it. This will require some other changes for Enums, they may always require validation (I'll need to be sure --delattr can remove a bad one). rob Updated patch. The primary key will be validated only on adds. The values will be validated on adds and mods. It turns out there already is infrastructure which does exactly the same thing: the "query" kwarg of the Parameter class. W
Re: [Freeipa-devel] Fix build issues in master with krb5 1.10
Simo Sorce wrote: The following 2 patches are need to have a functioning kdc. Without them building against krb5 1.10 produces a ipadb.so module that fails to load due to missing symbols leaving kadmin.local and krb5kdc without a database. The reason this happens is that during development of this code MIT had some necessary API functions marked private and didn't expose headers. These functions have been made public in 1.10 and renamed. Headers with functions declaration and defines are also available now. They are needed only in master as in 2.2 this code is commented out and builds fine against 1.9 Simo. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 949 remove Apache ccache on upgrade
Remove any Apache ccache on upgrade. We have no way of knowing what is in there so play it safe. rob freeipa-rcrit-949-ccache.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 947 fix synxtax in 30-s4u2proxy.update
Martin Kosek wrote:
On Tue, 2012-02-14 at 15:51 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2012-02-13 at 11:43 -0500, Rob Crittenden wrote:
Remove quotes around a value in 30-s4u2proxy.update. The update was
failing to apply.
I also noticed that FQDN wasn't being set properly in all cases in
sub_dict. This should fix it.
rob
This patch did not apply for me. I guess it depends on some other patch
that fixes wrong DN in s4u2proxy ipaAllowedTargets:
-default: ipaAllowedTarget:
'cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX'
+default: ipaAllowedTarget:
cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX
Current update file says:
default: ipaAllowedTarget: 'cn=ipa-ldap-delegation-targets,cn=etc,$SUFFIX'
which is a non-existent DN.
Martin
It relies on patch 941
Yeah, that's the one.
I am now testing all the upgrade patches, but I s4u2proxy does not work
for me yet on upgraded server instance (tested on F16). krb5kdc keeps
reporting decrypt errors:
/var/log/krb5kdc.log:
Feb 15 04:25:43 vm-068.idm.lab.bos.redhat.com krb5kdc[872](info):
TGS_REQ (4 etypes {18 17 16 23}) 10.16.78.68: PROCESS_TGS: authtime 0,
for, Decrypt integrity check failed
Feb 15 04:25:43 vm-068.idm.lab.bos.redhat.com krb5kdc[872](info):
TGS_REQ (4 etypes {18 17 16 23}) 10.16.78.68: PROCESS_TGS: authtime 0,
for, Decrypt integrity check failed
New installs on the same machine work though. I am still trying to find
out the root cause of this.
Martin
Turned out a stale Apache ccache was in /tmp. I've created a new ticket
and patch for that.
Martin also noticed that allowedTargets wasn't being set properly on new
installs. Updated patch attached.
rob
freeipa-rcrit-947-2-upgrade.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 940 apply some validation to some classes only
Jan Cholasta wrote: On 14.2.2012 22:16, Rob Crittenden wrote: Jan Cholasta wrote: On 14.2.2012 16:44, Jan Cholasta wrote: On 7.2.2012 20:25, Rob Crittenden wrote: Rob Crittenden wrote: Jan Cholasta wrote: Dne 7.2.2012 09:27, Martin Kosek napsal(a): On Mon, 2012-02-06 at 11:52 -0500, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2012-02-03 at 16:58 -0500, Rob Crittenden wrote: There is some validation that we only need to apply when an entry is being created, namely the key itself. This is to allow us to manage an otherwise illegal entry that finds its way into the system (i.e. migration). Consider this. We migrate a group with a space in it. This isn't allowed in IPA but we also provide no way to delete it because the cn regex kicks out the group-del command. The trick is adding appropriate context so we can know during validation how we got here. A command object has a bases field which contains the base classes associated with it, which appears to contain only the leaf baseclass. So using this we can tell how we got to validation and can skip based on that baseclass name. I went back and forth a bit on where the right place to put this was, I'm open to more fine tuning. I initially skipped just the pattern validation then expanded it to apply to all validation in the Parameter. rob 1) This patch breaks API.txt, it needs re-generating. 2) This may be a matter of opinion but I think that + if self.onlyvalidateclasses and \ + not any(x in self.onlyvalidateclasses for x in classbases): + return would be more readable than: + if self.onlyvalidateclasses and \ + not [x for x in classbases if x in self.onlyvalidateclasses]: + return 3) I would move the entire self.onlyvalidateclasses up to the validate() method. It would have several benefits: - the validation skip would be done just once, not for every value as the decision does not use the value at all - we would not have to modify _validate_scalar() methods at all since they won't need classbases 4) I think it would be better to keep validation for --rename options. As it is generated from RDN attribute parameter, it inherits onlyvalidateclasses list as well. Otherwise your patch would let user to rename a valid object to an invalid one: # ipa user-mod fbar --rename="bad boy" Modified user "fbar" User login: bad boy First name: Foo Last name: Bar Home directory: /home/fbar Login shell: /bin/sh UID: 480800040 GID: 480800040 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False Martin This should address your concerns. rob Its almost OK, there is just one part that's not that OK: @@ -831,6 +836,9 @@ class Param(ReadOnly): else: raise RequirementError(name=self.name) return + if 'rename' not in (self.name, self.cli_name): + if self.onlyvalidateclasses and not [x for x in self.onlyvalidateclasses if x in classbases]: #pylint: disable=E1101 + return if self.multivalue: if type(value) is not tuple: raise TypeError( I don't think that hard-coding this skip for onlyvalidateclasses based just on parameter name is a good thing to do and may cause problems in the future. For example if we create some option called "rename" and deliberately set onlyvalidateclasses for the option - it would then be skipped as well. I think it would be a better solution to just update _get_rename_option() in baseldap.py to set onlyvalidateclasses to () Martin I must say I'm not a big fan of this approach. You are healing the symptoms (custom validation on some parameters makes it impossible to manipulate existing LDAP entries with invalid attribute values => add a way to mark parameters to be validated only in certain commands to prevent that), but the real issue here is that we should not perform custom validation when referring to existing LDAP attribute values at all (or only partially), no matter what parameter and command. Fixing this would make the problem go away for all commands, present or future, without the need for adding a list of command classes to each parameter that is affected. Honza It is all about context, and parameters have very little of it. The bottom line is we only want to do validation on new values. I think I can bump this up a level by adding a validate boolean to Param and Command both defaulting to False. crud.Create will override this to True and cloning rename could perhaps also set this flag. Then in validation if the parent object has validation set or the parameter does we validate, otherwise we skip it. This will require some other changes for Enums, they may always require validation (I'll need to be sure --delattr can remove a bad one). rob Updated patch. The primary key will be validated only on adds. The values will be validated on adds and mods. It turns out there already is infrastructure which does exactly the same thing: the "query" kwarg of the Parameter class. We should fix its behavior rather than reinv
Re: [Freeipa-devel] [PATCH] 948 handle ipa_kpasswd on upgrades
On Wed, 2012-02-15 at 08:45 -0500, Rob Crittenden wrote: > Rob Crittenden wrote: > > Martin Kosek wrote: > >> On Mon, 2012-02-13 at 11:46 -0500, Rob Crittenden wrote: > >>> When upgrading from 2.1 to 2.2 we need to kill any ipa_kpasswd processes > >>> and uninstall it. > >>> > >>> We also need to update the dbmodules section in /etc/krb5.conf to > >>> reflect the new kdb. > >>> > >>> rob > >> > >> The patch works fine, I just have few comments. > >> > >> This line seems a bit confusing (at least for me): > >> + if not enabled is None and not enabled: > >> + ipa_kpasswd.remove() > >> > >> I would prefer > >> + if enabled is not None and not enabled: > >> + ipa_kpasswd.remove() > >> > >> I know, its just a nitpick. > > > > I saw that too and was tempted to change it but this is the way it is > > done currently in services, didn't want to mess with success :-) > > > > I'm not opposed. > > > >> > >> I checked update_dbmodules() function. It works but as we discussed, > >> there is a lot of hard-coding and a success of this function depends on > >> proper indentation etc. I would accept this fix as a short-term > >> solution, in the future we may want to use some better means to update > >> our configs like augeas that was already discussed before. > > > > Yes, ideally we'll switch to using the ipaChangeConf module but it > > doesn't support searching within subsections yet. > > Updated patch moving position of not. > > rob Thanks for the update. Now, my nitpick mastery is complete... ACK. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 61] Cache authentication in session
On Tue, 2012-02-14 at 12:01 -0600, Endi Sukma Dewata wrote: > If someone already has a session then changes to another principal, will > he get a new session and then reauthenticate against the login URL? Or > will he use the same session but just reauthenticate? I guess it depends on what you mean by "changes to another principal". If you mean a simple kdestroy/kinit, then the interface will not see the change until the session expires. (Which is why I want a session expiration much shorter for Negotiate auth). However if the user presses the logout button in the UI his session will be destroyed and so the next attempt will cause a new negotiation to happen and the UI will change to use the new principal. > What if he then changes back to the previous principal, will he reuse > the old session? If so will he be required to authenticate again? What old session do you refer to ? The previous session of the same user has been destroyed, the other user session is still valid until it expires or the user presses log out, so nothing changes there. > I think if the principal change is always preceded by reauthentication, > the UI will only need to redo the whoami after successful login. > Otherwise, the UI will need to check the principal change after each > operation like in the current code. The UI will know when the principal changes because it will get an error and it will be told to go to the re-authentication URL. When you re-authenticate you should always check if the principal changed. > Would it be possible for someone from another machine to randomly guess > a session ID and then take over an existing authenticated session? The session ID is currently a 48bit random number (we ask John to make it a 128bit number). *Guessing* that number is going to be *very* though (read technically impossible if the randomness is properly handled at the creation of the session ID in the server. However if somehow the sessionid is exposed than an attacker can hijack the connection. The risk is extremely low, but it is another reason why having a short expiration for Negotiate auth would be better, than treating it the same as form based auth. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 948 handle ipa_kpasswd on upgrades
Rob Crittenden wrote: Martin Kosek wrote: On Mon, 2012-02-13 at 11:46 -0500, Rob Crittenden wrote: When upgrading from 2.1 to 2.2 we need to kill any ipa_kpasswd processes and uninstall it. We also need to update the dbmodules section in /etc/krb5.conf to reflect the new kdb. rob The patch works fine, I just have few comments. This line seems a bit confusing (at least for me): + if not enabled is None and not enabled: + ipa_kpasswd.remove() I would prefer + if enabled is not None and not enabled: + ipa_kpasswd.remove() I know, its just a nitpick. I saw that too and was tempted to change it but this is the way it is done currently in services, didn't want to mess with success :-) I'm not opposed. I checked update_dbmodules() function. It works but as we discussed, there is a lot of hard-coding and a success of this function depends on proper indentation etc. I would accept this fix as a short-term solution, in the future we may want to use some better means to update our configs like augeas that was already discussed before. Yes, ideally we'll switch to using the ipaChangeConf module but it doesn't support searching within subsections yet. Updated patch moving position of not. rob freeipa-rcrit-948-2-kpasswd.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 948 handle ipa_kpasswd on upgrades
Martin Kosek wrote: On Mon, 2012-02-13 at 11:46 -0500, Rob Crittenden wrote: When upgrading from 2.1 to 2.2 we need to kill any ipa_kpasswd processes and uninstall it. We also need to update the dbmodules section in /etc/krb5.conf to reflect the new kdb. rob The patch works fine, I just have few comments. This line seems a bit confusing (at least for me): +if not enabled is None and not enabled: +ipa_kpasswd.remove() I would prefer +if enabled is not None and not enabled: +ipa_kpasswd.remove() I know, its just a nitpick. I saw that too and was tempted to change it but this is the way it is done currently in services, didn't want to mess with success :-) I'm not opposed. I checked update_dbmodules() function. It works but as we discussed, there is a lot of hard-coding and a success of this function depends on proper indentation etc. I would accept this fix as a short-term solution, in the future we may want to use some better means to update our configs like augeas that was already discussed before. Yes, ideally we'll switch to using the ipaChangeConf module but it doesn't support searching within subsections yet. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 948 handle ipa_kpasswd on upgrades
On Mon, 2012-02-13 at 11:46 -0500, Rob Crittenden wrote: > When upgrading from 2.1 to 2.2 we need to kill any ipa_kpasswd processes > and uninstall it. > > We also need to update the dbmodules section in /etc/krb5.conf to > reflect the new kdb. > > rob The patch works fine, I just have few comments. This line seems a bit confusing (at least for me): +if not enabled is None and not enabled: +ipa_kpasswd.remove() I would prefer +if enabled is not None and not enabled: +ipa_kpasswd.remove() I know, its just a nitpick. I checked update_dbmodules() function. It works but as we discussed, there is a lot of hard-coding and a success of this function depends on proper indentation etc. I would accept this fix as a short-term solution, in the future we may want to use some better means to update our configs like augeas that was already discussed before. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0010 Use stricter semantics when checking IP address for DNS records
On Wed, 2012-02-15 at 11:20 +0100, Petr Viktorin wrote: > This fixes https://fedorahosted.org/freeipa/ticket/2379 by using > inet_pton instead of inet_aton. > Yeah, this would fix the stricter checking. I planed to improve A/ validation in a scope of this ticket, I plan to use CheckedIPAddress to be more consistent with the rest of the plugin. I made the change you just did in CheckedIPAddress already. My point is that we may want to be even stricter and forbid for example broadcast or multicast addresses to be placed to A/ records. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 946 Upgrade schema from 2.1 to 2.2
On Mon, 2012-02-13 at 11:42 -0500, Rob Crittenden wrote: > Add missing schema when upgrading from 2.1 to 2.2 This schema relates to > delegation for S4U2Proxy. > > rob Although I wasn't able to make s4u2proxy update work on my updated FreeIPA instance yet this patch is OK. Schema is updated correctly. ACK. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 940 apply some validation to some classes only
On 14.2.2012 22:16, Rob Crittenden wrote: Jan Cholasta wrote: On 14.2.2012 16:44, Jan Cholasta wrote: On 7.2.2012 20:25, Rob Crittenden wrote: Rob Crittenden wrote: Jan Cholasta wrote: Dne 7.2.2012 09:27, Martin Kosek napsal(a): On Mon, 2012-02-06 at 11:52 -0500, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2012-02-03 at 16:58 -0500, Rob Crittenden wrote: There is some validation that we only need to apply when an entry is being created, namely the key itself. This is to allow us to manage an otherwise illegal entry that finds its way into the system (i.e. migration). Consider this. We migrate a group with a space in it. This isn't allowed in IPA but we also provide no way to delete it because the cn regex kicks out the group-del command. The trick is adding appropriate context so we can know during validation how we got here. A command object has a bases field which contains the base classes associated with it, which appears to contain only the leaf baseclass. So using this we can tell how we got to validation and can skip based on that baseclass name. I went back and forth a bit on where the right place to put this was, I'm open to more fine tuning. I initially skipped just the pattern validation then expanded it to apply to all validation in the Parameter. rob 1) This patch breaks API.txt, it needs re-generating. 2) This may be a matter of opinion but I think that + if self.onlyvalidateclasses and \ + not any(x in self.onlyvalidateclasses for x in classbases): + return would be more readable than: + if self.onlyvalidateclasses and \ + not [x for x in classbases if x in self.onlyvalidateclasses]: + return 3) I would move the entire self.onlyvalidateclasses up to the validate() method. It would have several benefits: - the validation skip would be done just once, not for every value as the decision does not use the value at all - we would not have to modify _validate_scalar() methods at all since they won't need classbases 4) I think it would be better to keep validation for --rename options. As it is generated from RDN attribute parameter, it inherits onlyvalidateclasses list as well. Otherwise your patch would let user to rename a valid object to an invalid one: # ipa user-mod fbar --rename="bad boy" Modified user "fbar" User login: bad boy First name: Foo Last name: Bar Home directory: /home/fbar Login shell: /bin/sh UID: 480800040 GID: 480800040 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False Martin This should address your concerns. rob Its almost OK, there is just one part that's not that OK: @@ -831,6 +836,9 @@ class Param(ReadOnly): else: raise RequirementError(name=self.name) return + if 'rename' not in (self.name, self.cli_name): + if self.onlyvalidateclasses and not [x for x in self.onlyvalidateclasses if x in classbases]: #pylint: disable=E1101 + return if self.multivalue: if type(value) is not tuple: raise TypeError( I don't think that hard-coding this skip for onlyvalidateclasses based just on parameter name is a good thing to do and may cause problems in the future. For example if we create some option called "rename" and deliberately set onlyvalidateclasses for the option - it would then be skipped as well. I think it would be a better solution to just update _get_rename_option() in baseldap.py to set onlyvalidateclasses to () Martin I must say I'm not a big fan of this approach. You are healing the symptoms (custom validation on some parameters makes it impossible to manipulate existing LDAP entries with invalid attribute values => add a way to mark parameters to be validated only in certain commands to prevent that), but the real issue here is that we should not perform custom validation when referring to existing LDAP attribute values at all (or only partially), no matter what parameter and command. Fixing this would make the problem go away for all commands, present or future, without the need for adding a list of command classes to each parameter that is affected. Honza It is all about context, and parameters have very little of it. The bottom line is we only want to do validation on new values. I think I can bump this up a level by adding a validate boolean to Param and Command both defaulting to False. crud.Create will override this to True and cloning rename could perhaps also set this flag. Then in validation if the parent object has validation set or the parameter does we validate, otherwise we skip it. This will require some other changes for Enums, they may always require validation (I'll need to be sure --delattr can remove a bad one). rob Updated patch. The primary key will be validated only on adds. The values will be validated on adds and mods. It turns out there already is infrastructure which does exactly the same thing: the "query" kwarg of the Parameter class. We should fix its behavior rather than reinvent the wheel. This
Re: [Freeipa-devel] [PATCH] 944 upgrade files for selinuxusermap
On Tue, 2012-02-14 at 17:45 -0500, Rob Crittenden wrote: > Rob Crittenden wrote: > > Rob Crittenden wrote: > >> The update files were missing for SELinuxUserMap support, this adds them. > > > > Rebased patch > > > > rob > > Sorry, sent the wrong patch. Here is the right one. > > rob I guess you forgot something :-) Anyway, I rebased the original patch 944 and its OK. SELinux update works fine. ACK. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 941 fix replica s4u2proxy delegation
On Mon, 2012-02-06 at 15:12 -0500, Rob Crittenden wrote: > Replicas weren't being added to the S4U2Proxy configuration so weren't > allowed to delegate tickets. This fixes things. > > rob This would work fine for new replicas which adds replica-s4u2proxy.ldif but what about upgrading the current ones? I am not sure that 30-s4u2proxy.update would add their principals to s4u2proxy configuration. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 947 fix synxtax in 30-s4u2proxy.update
On Tue, 2012-02-14 at 15:51 -0500, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Mon, 2012-02-13 at 11:43 -0500, Rob Crittenden wrote:
> >> Remove quotes around a value in 30-s4u2proxy.update. The update was
> >> failing to apply.
> >>
> >> I also noticed that FQDN wasn't being set properly in all cases in
> >> sub_dict. This should fix it.
> >>
> >> rob
> >
> > This patch did not apply for me. I guess it depends on some other patch
> > that fixes wrong DN in s4u2proxy ipaAllowedTargets:
> >
> > -default: ipaAllowedTarget:
> > 'cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX'
> > +default: ipaAllowedTarget:
> > cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX
> >
> > Current update file says:
> >
> > default: ipaAllowedTarget: 'cn=ipa-ldap-delegation-targets,cn=etc,$SUFFIX'
> >
> > which is a non-existent DN.
> >
> > Martin
> >
>
> It relies on patch 941
Yeah, that's the one.
I am now testing all the upgrade patches, but I s4u2proxy does not work
for me yet on upgraded server instance (tested on F16). krb5kdc keeps
reporting decrypt errors:
/var/log/krb5kdc.log:
Feb 15 04:25:43 vm-068.idm.lab.bos.redhat.com krb5kdc[872](info):
TGS_REQ (4 etypes {18 17 16 23}) 10.16.78.68: PROCESS_TGS: authtime 0,
for , Decrypt integrity check failed
Feb 15 04:25:43 vm-068.idm.lab.bos.redhat.com krb5kdc[872](info):
TGS_REQ (4 etypes {18 17 16 23}) 10.16.78.68: PROCESS_TGS: authtime 0,
for , Decrypt integrity check failed
New installs on the same machine work though. I am still trying to find
out the root cause of this.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0010 Use stricter semantics when checking IP address for DNS records
This fixes https://fedorahosted.org/freeipa/ticket/2379 by using inet_pton instead of inet_aton. -- Petr³ >From 5f81db8537b7af6e23acc1ad07d3a8a5118d41b7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 15 Feb 2012 05:11:24 -0500 Subject: [PATCH] Use stricter semantics when checking IP address for DNS records https://fedorahosted.org/freeipa/ticket/2379 --- ipalib/plugins/dns.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index fe32efccdf9b0e0d7d8d58dd9896205ef369ec5a..1582f7b122b0f35e1fd156883e28cea1e5402f42 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -196,7 +196,7 @@ def _reverse_zone_name(netstr): def _validate_ipaddr(ugettext, ipaddr, ip_version=None): try: -ip = netaddr.IPAddress(ipaddr) +ip = netaddr.IPAddress(ipaddr, flags=netaddr.INET_PTON) if ip_version is not None: if ip.version != ip_version: -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 61] Cache authentication in session
On 02/14/2012 01:01 PM, Endi Sukma Dewata wrote: > > If someone already has a session then changes to another principal, > will he get a new session and then reauthenticate against the login > URL? Or will he use the same session but just reauthenticate? > AFAIU the session ID will be (should be) lost when you switch the principal and new ID returned to the browser. The old session is still valid for 20 min if you did not log out but it should not be accessible. It seems that everything depends on what is the key for the internal cache. I would think that the key should be principal+session ID and session ID is long and strong. We said in one of the earlier discussions that it should be at least 128 bits of entropy. Was that done? If not please log a ticket. So am I right that the key is principal+session ID? If it is just principal then yes the session can be resurrected but I do not think we want this. If the key is just the ID there is a potential of the privilege escalation if one principal tries to guess the ID for another active session. Very unlikely but it seems that it is safer to have it as a composit key. May be we should file an RFE on this. > What if he then changes back to the previous principal, will he reuse > the old session? If so will he be required to authenticate again? > Old session should not be accessible. > I think if the principal change is always preceded by > reauthentication, the UI will only need to redo the whoami after > successful login. Otherwise, the UI will need to check the principal > change after each operation like in the current code. > > Would it be possible for someone from another machine to randomly > guess a session ID and then take over an existing authenticated session? > Then we need to make ID even longer. -- Thank you, Dmitri Pal Sr. Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 076 UI support for ssh keys
On 02/11/2012 12:36 AM, Endi Sukma Dewata wrote: On 2/6/2012 8:15 AM, Petr Vobornik wrote: ipasshpubkey attribute was added to user and host details pages. New widget for ssh public keys was created. Static preview: http://pvoborni.fedorapeople.org/ssh/#identity=user&navigation=identity&user-facet=default&user-pkey=aortiz https://fedorahosted.org/freeipa/ticket/2340 Everything works, so it's ACKed. I have some comments: Pushed to master, ipa-2-2. 1. When you click Add to add a new SSH public key the UI creates a new space for the key. The space is still empty, but it shows an undo button, implying there's a change that can be saved. However, clicking Update will generate an error "no modifications to be performed". Some possible solutions: (a) Show the "Set SSH key" dialog when you click Add. Once you entered the key, it creates the new space that already contains the new key. (b) When you click Update, remove all empty new space and do not generate the modify operation unless there are other changes. This should be addressed in save and test_dirty_method (omit empty keys in save). 1a is a nice idea but user can still set an empty key - it doesn't solve the problem. 2. If we decided to do #1a, the dialog title would be "Add SSH Public Key" and the button would be Add. For edit the title would be "Edit SSH Public Key" and the button would be Update. 3. Instead of showing "New: key set" or "New: key not set" we probably can show the first few characters of the key itself. This way if you're entering multiple keys you know which ones are already added. Like it. 4. It's also possible to use a table widget, which will work like solution #1a except that all changes are saved immediately so we don't need to show the partial key like in #3. I don't think table fits in the facet. Table would be better candidate for attribute with more than one displayed column. 5. Instead of using a "Show/Set key" link, we probably can link the fingerprint (for existing keys) or the partial key like in #3 (for new keys). If we did the table like in #4, it can be an Edit link. Might work. 6. When modifying a key, changing the fingerprint to "Modified" is probably not necessary because the undo button will imply that the key has been modified and we can't see the original fingerprint anymore. Maybe. I didn't want to show the fingerprint because it is not valid for the modified value. 7. When you edit an existing key and set it into empty, it will say "Modified: key not set". I think we can just strikeout the fingerprint because it's the same as deleting the key. Agree. 8. This is a minor server issue, not sure whether it's already fixed in the latest patch. When adding a new user with a key, the output of the user-add command contains the key too. In user-show the key is only shown with the --all option. 1-7 - I'll address them later. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 077 Redirection to PTR records from A, AAAA records
On 02/14/2012 04:35 PM, Endi Sukma Dewata wrote: On 2/14/2012 8:01 AM, Petr Vobornik wrote: 1. After redirection the breadcrumb doesn't link to the correct zone. ... This is an existing issue so let's do this separately. So the patch is ACKed, feel free to push. Pushed to master, ipa-2-2. The other improvements can be done separately if you want. I'll create separate tickets for them. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
