Re: [Freeipa-devel] python-kerberos patch
On Mon, 2014-01-13 at 14:45 -0500, Rob Crittenden wrote:
> In an effort to be able to to use GSS-Proxy as a client in IPA I've
> written a patch against python-kerberos to add a call to
> gss_cred_inquire so we can peek at the current principal name. This will
> replace the python-krbV call in ipapython/util.py::get_current_principal().
>
> The patch is pending review upstream at
> https://trac.calendarserver.org/ticket/830#comment:1 but it hasn't seen
> any activity yet and I'm impatient.
>
> Anyone have a few moments to take a look? I'm not super happy with the
> way one has to call it and some feedback would be helpful.
I do not like their python API, but ... we can;t change that.
One the C code:
you call it yadda_client_yadda but then you user server_creds as the
variable name, that's confusing.
You should simply initialize name and name_token to 0 and
unconditionally free/release them on error, and do that all at the end
based on the return you want.
Also you can simplify string copying..
How I'd rewrite the last 15 lines after gss_display_name()
if (GSS_ERROR(maj_stat)) {
set_gss_error(maj_stat, min_stat);
ret = AUTH_GSS_ERROR;
goto end;
}
state->username = strndup(name_token.value, name_token.length);
if (!state->username) {
set_gss_error(GSS_S_FAILURE, ENOMEM);
ret = AUTH_GSS_ERROR;
}
end:
(void)gss_release_cred(&min_stat, &server_creds);
(void)gss_release_name(&min_stat, &name);
(void)gss_release_buffer(&min_state, name_token);
return ret;
}
You could simplify even more without using ret, and jumping to end on
any maj_stat error and at end
if (maj_stat != GSS_S_COMPLETE) {
set_gss_error(maj_state, min_stat);
...
Simo.
--
Simo Sorce * Red Hat, Inc * New York
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] python-kerberos patch
In an effort to be able to to use GSS-Proxy as a client in IPA I've written a patch against python-kerberos to add a call to gss_cred_inquire so we can peek at the current principal name. This will replace the python-krbV call in ipapython/util.py::get_current_principal(). The patch is pending review upstream at https://trac.calendarserver.org/ticket/830#comment:1 but it hasn't seen any activity yet and I'm impatient. Anyone have a few moments to take a look? I'm not super happy with the way one has to call it and some feedback would be helpful. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 531-541 OTP UI
Petr, HOTP support should be trivial to add. In the CLI it is just --type=hotp. Everything else is the same, with the exception of the new optional --counter parameter. In soft tokens, this parameter should never be used. Nathaniel - Original Message - > Hi, > > these patches implements the OTP Web UI. > > Last 5 patches is the OTP UI. > > First 6 patches is a little refactoring/bug fixes needed for them. > General password dialog is introduced to avoid another implementation. > > Self-service UI is implemented to be very simple. Atm user can choose > only token name. Admin interface allows to enter all values. > > It's based on the RCUE work -> we need to push RCUE first. Thanks > Nathaniel for review of the last font package. It will speed things up. > > Know bugs: > - there is clash in id's of checkboxes preventing editation of > subsequently displayed ones with the same name. Will be fixed in > separate patch. > - bugs caused by bugs in API (adding/removal of own tokens in > self-service, inability to enter key on token creation - > https://fedorahosted.org/freeipa/ticket/4099) > - datetime format (widget+validator) will be implemented in separate patch > - no support of not reviewed CLI patches (HOTP..) > > Cgit: > http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/log/?h=otp > > https://fedorahosted.org/freeipa/ticket/3369 > -- > Petr Vobornik > > ___ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0084 Make sure state of services is preserved after client uninstall
Ana Krivokapic wrote: On 11/15/2013 05:03 PM, Tomas Babej wrote: On 11/07/2013 05:25 PM, Ana Krivokapic wrote: Hello, This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3790. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Looking good.. I have two questions: 1.) Nitpick: I'd suggest we rename the save_state(service) and restore_state(service) to more descriptive save_service_state/restore_service_state? Well, if the argument you are passing to these function does not have a name which suggests it is a service (which it should have anyway), you can do: `save_state(service=x)`. So I don't think `save_service_state(x)` is more readable. 2.) There are other places in ipa-client-install where we save and restore the state of the service. Having abstracted that into a function, should we use this at other places as well? I looked at other instances in ipa-client-install where service states are saved and restored. It seems that each of these cases includes some custom logic which does not make it possible to use the two functions I've added. It looks fine to me as-is. I couldn't find any other services to include in this which is indeed a shame as this significantly simplifies things. ACK rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0335 ipa-replica-install: Move check for existing host before DNS resolution check
Petr Viktorin wrote: See commit message & ticket for details. https://fedorahosted.org/freeipa/ticket/3889 If memory serves this was done so that both the replication and the host checks would be done so the admin wouldn't die a death of a thousand cuts. If a leftover agreement exists then the replica install will fail. You delete the agreement. The next install may fail too if the host exists. We should check for both before quitting. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0336 rpcserver: Consolidate __call__ in xmlclient and jsonclient_kerb
Petr Viktorin wrote: See commit message & ticket. https://fedorahosted.org/freeipa/ticket/4069 Our handling of XML-RPC introspection is iffy as it is and this would remove those methods completely. Can you add them back into the xmlserver class? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 13.1.2014 13:41, Jan Cholasta wrote:
Hi,
On 10.1.2014 21:21, Nathaniel McCallum wrote:
On Thu, 2014-01-09 at 16:30 +0100, Tomas Babej wrote:
Hi,
Adds a parameter that represents a DateTime format using
datetime.datetime
object from python's native datetime library.
In the CLI, accepts one of the following formats:
Accepts subset of values defined by ISO 8601:
%Y-%m-%dT%H:%M:%S
%Y-%m-%dT%H:%M
'%Y%m%dT%H:%M:%S'
'%Y%m%dT%H:%M'
Also accepts LDAP Generalized time in the following format:
'%Y%m%d%H%M%SZ'
As a simplification, it does not deal with timezone info and ISO 8601
values with timezone info (+-hhmm) are rejected. Values are expected
to be in the UTC timezone.
Values are saved to LDAP as LDAP Generalized time values in the format
'%Y%m%d%H%SZ' (no time fractions and UTC timezone is assumed). To avoid
confusion, in addition to subset of ISO 8601 values, the LDAP
generalized
time in the format '%Y%m%d%H%M%SZ' is also accepted as an input (as this
is the
format user will see on the output).
Part of: https://fedorahosted.org/freeipa/ticket/3306
The date/time syntax formats are not compliant with ISO 8601. You stated
they are expected to be in UTC timezone, but no 'Z' is expected in most
of them. This is not only non-standard, but would prevent you from every
supporting local time in the future.
+1, please require the trailing "Z".
I think generalized time format should be the preferred one, as it is
stored in LDAP and thus returned by commands. Also, do we really need
all of these other formats?
+1 That API should accept and always return generalized time.
But UIs(CLI, Web) should display something more human readable. Like:
%Y-%m-%dT%H:%M:%SZ or IMHO better (but not ISO 8601): %Y-%m-%d %H:%M:%SZ
ie. 2014-03-08 14:16:55Z compared to 2014-03-08T14:16:55Z or 20140308141655Z
Both should accept input value in the same format. Usage of different
output and input formats is confusing. Thus I agree with supporting more
input formats. Decision whether it should be done on API level or client
level is another matter.
I has been implementing the Web UI part and I think we should also
support a formats without time component. My initial impl. supports
combinations of:
var dates = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\d{4})(\d{2})(\d{2})$/]
];
var times = [
['HH:mm:ss', /^(\d\d):(\d\d):(\d\d)Z$/],
['HH:mm', /^(\d\d):(\d\d)Z$/],
['HH', /^(\d\d)Z$/]
];
+ generalized time
['MMDDHHmmssZ',/^(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})Z$/]
with /( |T)/ as divider and time optional.
Both UIs should also tell the user what is the expected format (at least
one of them). Getting incorrect format error without knowing it is annoying.
Few nitpicks about DateTime implementation:
* you should call Param._convert_scalar from DateTime._convert_scalar
* raise ConversionError instead of ValidationError and use
get_param_name() for name
* instead of "isinstance(value, str) or isinstance(value, unicode)"
use "isinstance(value, basestring)"
* don't use pythonisms in public error messages (""does not match any
of accepted formats: %s" % self.accepted_formats")
* public error messages should be translatable
It should look something like this (untested, from top of my head):
type = datetime.datetime
type_error = _('must be a date/time value')
def _convert_scalar(self, value, index=None):
if isinstance(value, basestring):
for date_format in self.accepted_formats:
try:
time = datetime.datetime.strptime(value, date_format)
return time
except ValueError:
pass
# If we get here, the strptime call did not succeed for any
# the accepted formats, therefore raise error
error = _("does not match any of accepted formats: %s") %
(', '.join(self.accepted_formats))
raise ConversionError(name=self.get_param_name(),
index=index,
error=error)
return super(DateTime, self)._convert_scalar(value, index)
+if isinstance(value, DateTime):
+return str(value)
Return datetime object here please, or at least unicode in generalized
time format (or whatever the preferred format is, see above).
+elif isinstance(val, datetime.datetime):
+return val.strftime("%Y%m%dT%H:%M:%S")
Again, please use the preferred format here please.
Honza
--
Petr Vobornik
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
Hi,
On 10.1.2014 21:21, Nathaniel McCallum wrote:
On Thu, 2014-01-09 at 16:30 +0100, Tomas Babej wrote:
Hi,
Adds a parameter that represents a DateTime format using datetime.datetime
object from python's native datetime library.
In the CLI, accepts one of the following formats:
Accepts subset of values defined by ISO 8601:
%Y-%m-%dT%H:%M:%S
%Y-%m-%dT%H:%M
'%Y%m%dT%H:%M:%S'
'%Y%m%dT%H:%M'
Also accepts LDAP Generalized time in the following format:
'%Y%m%d%H%M%SZ'
As a simplification, it does not deal with timezone info and ISO 8601
values with timezone info (+-hhmm) are rejected. Values are expected
to be in the UTC timezone.
Values are saved to LDAP as LDAP Generalized time values in the format
'%Y%m%d%H%SZ' (no time fractions and UTC timezone is assumed). To avoid
confusion, in addition to subset of ISO 8601 values, the LDAP generalized
time in the format '%Y%m%d%H%M%SZ' is also accepted as an input (as this
is the
format user will see on the output).
Part of: https://fedorahosted.org/freeipa/ticket/3306
The date/time syntax formats are not compliant with ISO 8601. You stated
they are expected to be in UTC timezone, but no 'Z' is expected in most
of them. This is not only non-standard, but would prevent you from every
supporting local time in the future.
+1, please require the trailing "Z".
I think generalized time format should be the preferred one, as it is
stored in LDAP and thus returned by commands. Also, do we really need
all of these other formats?
Few nitpicks about DateTime implementation:
* you should call Param._convert_scalar from DateTime._convert_scalar
* raise ConversionError instead of ValidationError and use
get_param_name() for name
* instead of "isinstance(value, str) or isinstance(value, unicode)"
use "isinstance(value, basestring)"
* don't use pythonisms in public error messages (""does not match any
of accepted formats: %s" % self.accepted_formats")
* public error messages should be translatable
It should look something like this (untested, from top of my head):
type = datetime.datetime
type_error = _('must be a date/time value')
def _convert_scalar(self, value, index=None):
if isinstance(value, basestring):
for date_format in self.accepted_formats:
try:
time = datetime.datetime.strptime(value, date_format)
return time
except ValueError:
pass
# If we get here, the strptime call did not succeed for any
# the accepted formats, therefore raise error
error = _("does not match any of accepted formats: %s") %
(', '.join(self.accepted_formats))
raise ConversionError(name=self.get_param_name(),
index=index,
error=error)
return super(DateTime, self)._convert_scalar(value, index)
+if isinstance(value, DateTime):
+return str(value)
Return datetime object here please, or at least unicode in generalized
time format (or whatever the preferred format is, see above).
+elif isinstance(val, datetime.datetime):
+return val.strftime("%Y%m%dT%H:%M:%S")
Again, please use the preferred format here please.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 125] CLDAP: do not prepend \\
Hi,
Scott found that the fix for
https://fedorahosted.org/freeipa/ticket/4028 is not complete. After some
checks and comparisons with samba and AD behaviour I came to the
conclusion that the two \\ at the beginning of the NetBIOS name of the
IPA server is not needed in the response of NETLOGON_NT_VERSION_5EX
requests which is the only type we handle so far.
In general AD seems to be smart enough the handle the \\ even in those
responses but if the NetBIOS name has 15 characters the response is not
accepted anymore.
Please check if you can see any regressions with this change.
During testing I came across two things related to samba.
While looking at network trace Scott recorded it looked like Samba does
not cut a long hostname for the NetBIOS name. This might be in agreement
to what Metze recently posted in his master4-schannel-ok branch for
netlogon_creds_cli_context_global(). As usual Metze is smarter than us
and tried to minimize the chance for name collisions with the help of
Jenkins hash. I just wonder why he treats the NetBIOS name only here
this way and not generally? With respect to IPA we might want to
consider to set 'netbios name' in the samba config explicitly to avoid
disconnects?
While testing against AD with other request types I've seen that in some
cases the NetBIOS name was returned with the two additional \ in the
beginning, even if the AD NetBIOS name already had 15 characters.
Strange the name was even encoded in UCS-2 in this case. Unfortunately I
was not able to find good documentation on the specifics of those
packages. If you know some good docs please send me the link otherwise
we might what to ask MSFT for clarification.
bye,
Sumit
From 0b782064945352ad488e92b457bbfda2270ddf66 Mon Sep 17 00:00:00 2001
From: Sumit Bose
Date: Mon, 13 Jan 2014 10:43:33 +0100
Subject: [PATCH] CLDAP: do not prepend \\
For NETLOGON_NT_VERSION_5EX requests the prepended \\ is not expected in
the PDC NetBIOS name. In general AD seems to be smart enough to handle
the two \ signs. But if the NetBIOS name reaches the maximum of 15
character AD does not accept the responses anymore.
Fixes https://fedorahosted.org/freeipa/ticket/4028
---
daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
index
9ba05829418a0d1de46f2c7776cc15c54a9eab1c..c03172d474589ddee84f1cfa5395c23fdba83bcb
100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
@@ -163,7 +163,7 @@ static int ipa_cldap_encode_netlogon(char *fq_hostname,
char *domain,
nlr->domain_name = name;
/* copy the first 15 characters of the fully qualified hostname*/
-pdc_name = talloc_asprintf(nlr, "%.*s", NETBIOS_NAME_MAX, fq_hostname);
+pdc_name = talloc_asprintf(nlr, "%.*s", NETBIOS_NAME_MAX, fq_hostname);
for (p = pdc_name; *p; p++) {
/* Create the NetBIOS name from the first segment of the hostname */
--
1.8.1.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA OTP End-to-End
On Sun, Jan 12, 2014 at 10:07:49PM +0200, Alexander Bokovoy wrote:
> >>There seem to be two parts, one is covered by this bug and another one
> >>is related to SSSD/logind communication:
> >>
> >>allow sssd_t systemd_logind_var_run_t:dir search;
> >>allow sssd_t systemd_logind_var_run_t:file { read getattr open };
> >
> >Interesting, which version are you running? The logind support is
> >currently only present in master (aka 1.12 dev)
> I'm running master, of course ;)
Great, then would you mind submitting against Rawhide SELinux policy?
1.12 is coming up in F-21 first, so Rawhide would be the place to fix
the policy.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
