On (30/01/16 00:00), Jakub Hrozek wrote:
>On Fri, Jan 29, 2016 at 08:24:00PM +0100, Lukas Slebodnik wrote:
>> On (29/01/16 18:50), Jakub Hrozek wrote:
>> >On Fri, Jan 29, 2016 at 03:25:56PM +0100, Lukas Slebodnik wrote:
>> >> On (29/01/16 13:51), Pavel Reichl wrote:
>> >> >
>> >> >
>> >> >On 01/29/2016 01:41 PM, Lukas Slebodnik wrote:
>> >> >>https://fedorahosted.org/sssd/ticket/2931
>> >> >>---
>> >> >> src/providers/krb5/krb5_child.c | 17 +++++++++++++++++
>> >> >> 1 file changed, 17 insertions(+)
>> >> >>
>> >> >>diff --git a/src/providers/krb5/krb5_child.c
>> >> >>b/src/providers/krb5/krb5_child.c
>> >> >>index
>> >> >>12eb9e2093d2bdd7d67e8d029fec1455488aa67c..88bcaddc419c1e6dc5d9a0c69b50c45a45c95efc
>> >> >> 100644
>> >> >>--- a/src/providers/krb5/krb5_child.c
>> >> >>+++ b/src/providers/krb5/krb5_child.c
>> >> >>@@ -2675,6 +2675,23 @@ int main(int argc, const char *argv[])
>> >> >> goto done;
>> >> >> }
>> >> >>
>> >> >>+ ret = open("/etc/krb5.conf", O_RDONLY);
>> >> >>+ if (ret == EOK)
>> >> >
>> >> >I thought that open() returns file descriptor on success and and -1 in
>> >> >case of error. Was I wrong?
>> >> >
>> >> That's a fast codding style change swith->if after testing and before
>> >> sending
>> >> patch.
>> >
>> >It's better to test file access with open if you need to actually use
>> >the file to avoid TOCTOU-style races, but here I guess it doesn't
>> >matter. Nonetheless, I would prefer to use a variable named 'fd' and not
>> >'ret'
>> I was just lazy to declare another variable
>
>If you don't mind, I would prefer fd. When I quickly scan code, 'ret' is
>something we normally use for return codes. Here it's something that
>actually represents a resource that needs to be closed later.
>
>>
>> >and close the fd right away,
>> It's already in patch.
>
>You're right, sorry, I overlooked that.
Updated patch is attached.
LS
>From 45d09426d7f4ea7371e0ca3779f1c7714f9047c8 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lsleb...@redhat.com>
Date: Fri, 29 Jan 2016 13:30:49 +0100
Subject: [PATCH] krb5_child: Warn if user cannot read krb5.conf
Attached patch should siplify troubleshoting of
issues with permission of krb5.conf. It's not clear from
krb5_child.log even with full debug level.
[sss_get_ccache_name_for_principal] (0x4000):
Location: [FILE:/tmp/krb5cc_12069_XXXXXX]
[sss_get_ccache_name_for_principal] (0x2000):
krb5_cc_cache_match failed: [-1765328243]
[Can't find client principal u...@example.com in cache collection]
[create_ccache] (0x0020): 735: [13][Permission denied]
Resolves:
https://fedorahosted.org/sssd/ticket/2931
---
src/providers/krb5/krb5_child.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index
12eb9e2093d2bdd7d67e8d029fec1455488aa67c..28e10fef75ffe7dace214f1fdbcbf5b9a1ee635c
100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -2675,6 +2675,23 @@ int main(int argc, const char *argv[])
goto done;
}
+ ret = open("/etc/krb5.conf", O_RDONLY);
+ if (ret != -1) {
+ close(ret);
+ } else {
+ ret = errno;
+ if (ret == EPERM) {
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ "User with uid:%"SPRIuid" gid:%"SPRIgid" cannot read "
+ "/etc/krb5.conf. It might cause problems.",
+ geteuid(), getegid());
+ } else {
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ "Cannot open /etc/krb5.conf [%d]: %s\n",
+ ret, strerror(ret));
+ }
+ }
+
DEBUG(SSSDBG_TRACE_INTERNAL,
"Running as [%"SPRIuid"][%"SPRIgid"].\n", geteuid(), getegid());
--
2.5.0
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
