On 10/04/2013 09:48 AM, Jakub Hrozek wrote:
On Tue, Oct 01, 2013 at 10:10:57PM +0200, Jakub Hrozek wrote:
On Tue, Oct 01, 2013 at 09:50:07PM +0200, Jakub Hrozek wrote:
On Tue, Oct 01, 2013 at 09:45:37PM +0200, Jakub Hrozek wrote:
On Thu, Sep 26, 2013 at 08:25:12PM +0200, Jakub Hrozek wrote:
On Wed, Sep 25, 2013 at 05:50:16PM -0400, Simo Sorce wrote:
On Wed, 2013-09-25 at 17:37 -0400, Simo Sorce wrote:
On Tue, 2013-09-24 at 12:52 +0200, Jakub Hrozek wrote:
On Fri, Sep 20, 2013 at 01:09:34PM -0400, Simo Sorce wrote:
On Mon, 2013-09-09 at 11:22 -0400, Simo Sorce wrote:
On Mon, 2013-09-09 at 13:41 +0200, Jakub Hrozek wrote:
On Mon, Sep 09, 2013 at 01:25:30PM +0200, Sumit Bose wrote:
On Tue, Sep 03, 2013 at 10:52:14PM -0400, Simo Sorce wrote:
Attached an untested possible alternative for #2071
The other option is the last patch of my previous patchset in the thread
named: "[SSSD] [PATCHES] Simplify credential krb5 cache manipulation".
Let's discuss if any of these approaches is ok, or if a third way needs
to be found.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
(a short summary of the two patches in discussion)
The difference between the two approaches is that the patch in this
tread just sets the permission on all newly created directories to
"new_dir_mode = 0700" making them private by default. The patch in the
other thread tries to improve the current scheme where SSSD tries to
figure out which directory which must be created is a public or a
private one and sets the mode accordingly.
I agree with 'Setting up public directories is the job of the admin' and
like the fact that the patch is making the SSSD code much simpler. But I
assume that there are SSSD setups in use which depend on the feature of
creating the directories with the "right" permissions. I can imagine
automatic installations e.g. with kickstart which does not create the
needed directories but rely on SSSD to create them or installations
where the credential caches are stored into some RAM based file system
where directories must be recreated on every restart. I do not want to
discuss the correctness of those approaches or if the paths used to
store the credential caches are chosen wisely my point is just that a
change might break existing installations.
For this reason I would prefer to just improve the existing behaviour.
But if a majority says that it is time for a change I will not argue
against it.
bye,
Sumit
That's a good point.
Could we get away with keeping the existing behaviour and printing a log
message whenever we create a public directory saying that this feature
might go away in a later release?
I do not really think it is a good point.
A) I really do not think there is any user fo this feature, the only 2
places where people put stuff today are /tmp (FILE or DIR) and /run/user
(DIR).
/tmp is already set up correctly (if not many other things will break
and SSSD should *not* mask the mess by creating the dir and further
confusing the admin.
/run/user is a private directory so making it public would be wrong.
I think a change of behavior is in the best interest of our community
even if it ends up breaking 1 rare install that happened to know of this
new behavior and intentionally used it.
Bump and attached find the 2 patches rebased both on top of my latest
patchset sent to the list earlier.
0004 tries to maintain behavior, just saner.
0001 is the option to remove it completely.
As agreed yesterday on a call, we're going to remove the option
completely. The patch no longer applies cleanly, not even with a 3-way
merge. I had to apply it with patch(1), so can you please re-send? One
minor comment inline:
It doesn't apply because, it depends on the patch from the other
patchset that you did not apply ("krb5: Improve ccache name checks")
Ok fixed the issues and rebase before the other patch, should apply
cleanly on master now.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
There still must be some patch missing in between, I can't compile this
version:
CC src/providers/krb5/krb5_init_shared.lo
/home/remote/jhrozek/devel/sssd/src/providers/krb5/krb5_auth.c:295:17: warning:
unused variable 'realm' [-Wunused-variable]
const char *realm;
^
2 warnings generated.
/home/remote/jhrozek/devel/sssd/src/tests/krb5_child-test.c:249:57: error: too
many arguments to function call, expected 5, have 6
true, true, &private);
^~~~~~~~
/home/remote/jhrozek/devel/sssd/src/providers/krb5/krb5_utils.h:48:1: note:
'expand_ccname_template' declared here
char *expand_ccname_template(TALLOC_CTX *mem_ctx, struct krb5child_req *kr,
^
/home/remote/jhrozek/devel/sssd/src/tests/krb5_child-test.c:265:55: error: too
many arguments to function call, expected 4, have 5
kr->uid, kr->gid, private);
^~~~~~~
Ugh, I must have misapplied the patch. Now everything seems to have
applied and built correctly. My initial testing also passed.
ACK
I will just squash in the following change before pushing:
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index a16b539..a4183dc 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -292,7 +292,6 @@ static errno_t krb5_auth_prepare_ccache_name(struct
krb5child_req *kr,
struct be_ctx *be_ctx)
{
const char *ccname_template;
- const char *realm;
errno_t ret;
if (!kr->is_offline) {
Sorry for talking to myself, but I have to nack the patch after all, the
unit tests are not passing.
I'd like to apply the attached patch on top of yours.
Hi,
ack to the unit tests. Nack to the Simo's patch - it contains unused
variable. Can you squash attached patch in?
From d7a1c2274a3ecde7fba12749682f1331a5b027be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Fri, 4 Oct 2013 13:13:54 +0200
Subject: [PATCH 1/2] remove unused variable warning
---
src/providers/krb5/krb5_auth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index a16b5395d21c40e53a5e69519141cbd3c47d7907..a4183dcaca6597c84b2b407e252b32a16f79773b 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -292,7 +292,6 @@ static errno_t krb5_auth_prepare_ccache_name(struct krb5child_req *kr,
struct be_ctx *be_ctx)
{
const char *ccname_template;
- const char *realm;
errno_t ret;
if (!kr->is_offline) {
--
1.7.11.7
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
