Dne 24.11.2014 v 14:44 Alexander Bokovoy napsal(a):
On Tue, 18 Nov 2014, Jan Cholasta wrote:
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

https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
b/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
context,
                            ipactx->kdc_hostname,
strlen(ipactx->kdc_hostname),
                            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.
I've ACKed other patches but I'm not going to accept this patch. Rest is
fine.

That's fine with me, but I'm still going to need a little hint on how this should in fact be fixed.

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to