Dne 9.4.2015 v 14:41 Simo Sorce napsal(a):
On Wed, 2015-03-25 at 11:52 +0100, Martin Babinsky wrote:
On 03/23/2015 03:13 PM, Simo Sorce wrote:
On Mon, 2015-03-23 at 14:22 +0100, Petr Spacek wrote:
On 23.3.2015 14:08, Simo Sorce wrote:
On Mon, 2015-03-23 at 12:48 +0100, Martin Babinsky wrote:
On 03/17/2015 06:00 PM, Simo Sorce wrote:
On Mon, 2015-03-16 at 13:30 +0100, Martin Babinsky wrote:
On 03/16/2015 12:15 PM, Martin Kosek wrote:
On 03/13/2015 05:37 PM, Martin Babinsky wrote:
Attaching the next iteration of patches.
I have tried my best to reword the ipa-client-install man page bit about the
new option. Any suggestions to further improve it are welcome.
I have also slightly modified the 'kinit_keytab' function so that in Kerberos
errors are reported for each attempt and the text of the last error is retained
when finally raising exception.
The approach looks very good. I think that my only concern with this patch is
this part:
+ ccache.init_creds_keytab(keytab=ktab, principal=princ)
...
+ except krbV.Krb5Error as e:
+ last_exc = str(e)
+ root_logger.debug("Attempt %d/%d: failed: %s"
+ % (attempt, attempts, last_exc))
+ time.sleep(1)
+
+ root_logger.debug("Maximum number of attempts (%d) reached"
+ % attempts)
+ raise StandardError("Error initializing principal %s: %s"
+ % (principal, last_exc))
The problem here is that this function will raise the super-generic
StandardError instead of the proper with all the context and information about
the error that the caller can then process.
I think that
except krbV.Krb5Error as e:
if attempt == max_attempts:
log something
raise
would be better.
Yes that seems reasonable. I'm just thinking whether we should re-raise
Krb5Error or raise ipalib.errors.KerberosError? the latter options makes
more sense to me as we would not have to additionally import Krb5Error
everywhere and it would also make the resulting errors more consistent.
I am thinking about someting like this:
except krbV.Krb5Error as e:
if attempt == attempts:
# log that we have reaches maximum number of attempts
raise KerberosError(minor=str(e))
What do you think?
Are you retrying on any error ?
Please do *not* do that, if you retry many times on an error that
indicates the password is wrong you may end up locking an administrative
account. If you want to retry you should do it only for very specific
timeout errors.
Simo.
I have taken a look at the logs attached to the original BZ
(https://bugzilla.redhat.com/show_bug.cgi?id=1161722).
In ipaclient-install.log the kinit error is:
"Cannot contact any KDC for realm 'ITW.USPTO.GOV' while getting initial
credentials"
which can be translated to krbV.KRB5_KDC_UNREACH error. However,
krb5kdc.log (http://pastebin.test.redhat.com/271394) reports errors
which are seemingly unrelated to the root cause (kinit timing out on
getting host TGT).
Thus I'm not quite sure which errors should we chceck against in this
case, anyone care to advise? These are potential candidates:
KRB5KDC_ERR_SVC_UNAVAILABLE, "A service is not available that is
required to process the request"
KRB5KRB_ERR_RESPONSE_TOO_BIG, "Response too big for UDP, retry with TCP"
KRB5_REALM_UNKNOWN, "Cannot find KDC for requested realm"
KRB5_KDC_UNREACH, "Cannot contact any KDC for requested realm"
The only ones that you should retry on, at first glance are
KRB5_KDC_UNREACH, KRB5KDC_ERR_SVC_UNAVAILABLE.
You should never see KRB5KRB_ERR_RESPONSE_TOO_BIG in the script as it
should be handled automatically by the library, and if you get
KRB5_REALM_UNKNOWN I do not think that retrying will make any
difference.
I might be wrong but I was under the impression that this feature was also for
workarounding replication delay - service is not available / key is not
present / something like that.
(This could happen if host/principal was added to one server but then the
client connected to another server or so.)
If we have that problem we should instead use a temporary krb5.conf file
that lists explicitly only the server we are joining.
Simo.
This is already done since ipa-3-0: by default only one server/KDC is
used during client install so there are actually no problems with
replication delay, only with KDC timeouts.
Anyway I'm sending updated patches.
LGTM!
Simo.
Some comments:
Patch 15:
1) The functions should be as similar as possible:
a) kinit_password() should have a 'ccache_path' argument instead of
passing the path in KRB5CCNAME in the 'env' argument.
b) I don't think kinit_password() should have the 'env' argument at
all. You can always call kinit with LC_ALL=C and set other variables in
os.environ if you want.
c) The arguments should have the same ordering.
d) Either set KRB5CCNAME in both kinit_keytab() and
kinit_password() or in none of them.
e) Either rename armor_ccache to armor_ccache_path or ccache_path
to ccache.
2) Space before comma in docstring:
+ Given a ccache_path , keytab file and a principal kinit as that user.
3) I would prefer if the default value of 'armor_ccache' in
kinit_password() was None.
Patch 16:
1) The callback should not be named 'validate_kinit_attempts_option',
but rather 'kinit_attempts_callback', as it doesn't just validate the value.
2) Why is there the sys.maxint upper bound on --kinit-attempts again? A
comment with explanation would be nice.
Patch 17:
1) Is there a reason for the ccache filename changes in DNSSEC code?
--
Jan Cholasta
--
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
