Dne 12.11.2014 v 08:58 Petr Spacek napsal(a):
On 11.11.2014 12:27, Jan Cholasta wrote:
Dne 11.11.2014 v 11:40 Alexander Bokovoy napsal(a):
On Tue, 11 Nov 2014, Jan Cholasta wrote:
 From 82d7d37ca310af015018ebb2da2f9a72c4dabcaa Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 10 Nov 2014 18:10:27 +0000
Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb

daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
index c8f6c76..debcd1b 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2071,6 +2071,9 @@ krb5_error_code ipadb_sign_authdata(krb5_context
                             NULL, NULL, &result) == 0) {
                 kerr = ipadb_reinit_mspac(ipactx, true);
+                if (kerr != 0 && kerr != ENOENT) {
+                    goto done;
+                }

I'm not sure we should drop the sign_authdata request here. If we were
able to re-initialize our view of trusted domains, we simply cannot
re-sign incoming PAC but this is handled in ipadb_verify_pac() and
ipadb_sign_pac() and if the former returns NULL value for PAC, we exit
with a return code of 0 while this change will fail a cross-realm TGT
request unconditionally.

OK, what would be a proper fix? Just ignore the return value of
ipadb_reinit_mspac here?

Guys, I did not see the code but all instances of "ignore return code" I have
seen were wrong, including cases where code comment explicitly said "we ignore
return code on purpose" :-)

At least log an error message if you can't think of anything better ...

I don't disagree, if that's the proper fix.

Alexander, or someone else, could you please finish the review of the patches? Thanks.

Jan Cholasta

Freeipa-devel mailing list

Reply via email to