On 02/19/2013 10:33 PM, Rob Crittenden wrote:
Tomas Babej wrote:
On 02/06/2013 07:57 PM, Rob Crittenden wrote:
Tomas Babej wrote:
Hi,
this pair of patches improves HBAC rule handling in selinuxusermap
commands.
Patch 0031 deals with:
https://fedorahosted.org/freeipa/ticket/3349
Patch 0032 takes care of:
https://fedorahosted.org/freeipa/ticket/3348
and is to be applied on top of Patch 0031.
See commit messages for detailed info.
Tomas
ACK for patch 0032.
For patch 0031 we can't change the data type of an existing attribute.
It will break backwards compatibility. Can you test with an older
client to see if it cares (it may not care about the name of the
type). If older clients will work then this is probably ok.
I gather that seealso detected as a DN attribute and converted into a
DN class and this is blowing up the Str validator?
Yes, that was exactly the case.
rob
I added a workaround for older client versions, tested it with
freeipa-client/admintools 2.2, works as expeceted.
However, this only should be issue if there is older admintools package
on the client than on the server.
Outline is such as follows: I added a new flag for DNParam seelalso
attribute, called 'allow_malformed' that allows any string to be passed
to DNParam. Its value gets wrapped in 'malformed=yes,value=<value>'.
This allows to parse out the string in selinuxusermap-add/mod
pre_callback out of the DN and search for the rule with such name so
that it's DN gets in LDAP instead.
Updated patch attached.
Tomas
I like where you're going with this, just a couple of comments:
1. Should we come up with a more universal name for allow_malformed?
Is this something that we should allow at a higher level? I was
thinking allow_raw, or allow_non_dn, or something like that.
To me, allow_non_dn sounds is just as specific as allow_malformed,
they both refer to DN specifically.
I'd go with allow_raw, if need for such pattern may eventually arise for
other parameter classes than DNParam.
What do you mean by higher level, turning this hack into a feature
Param class? I don't see how this would work, each parameter
class that implements its own type validation as DNParam needs
to override _convert_scalar(). And in every such class we would need
to wrap our raw value so that it is represented in the type of this
parameter,
as we do with DN(('malformed','yes'),('value',value)) now.
Maybe we could skip type validation in _convert_scalar default
implementation or catch the error raised somehow, and let the type be
invalid, but I'm not aware of the consenquences. I would need to
investigate.
Wouldn't it cause failure deeper in the framework?
Or did you by higher level mean simply picking a more general name for the
flag so it can be reused in other parameter classes with the same name?
2. I think that if a bad dn is passed in as a Str the conversion into
a DN won't be handled:
+ if 'allow_malformed' in self.flags:
+ dn = DN(('malformed','yes'),('value',value))
Should this be wrapped in a try/except to raise a ConversionError if
it fails?
Yes, thanks for that catch.
rob
Tomas
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
