ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
LS
On Thu, Apr 04, 2013 at 12:14:31PM +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
LS
This is an interesting idea, but I wonder it is safe to drop and restore euid like this. At the very least, you need to restore the ID back after krb5_cc_resolve no matter what and I would prefer if this mechanism happened in a self-contained function with one return path.
But in general, I wonder if it was better to move calling krb5_cc_resolve completely to krb5_child and always call the child. If the ccache was active then the child would not kinit but just return the reused ccache.
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
1. Before calling krb5_cc_resolve() do a stat. 2. If the file/dir does not exist just return an error w/o calling any krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
Simo.
On (04/04/13 12:09), Simo Sorce wrote:
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
- Before calling krb5_cc_resolve() do a stat.
function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK)
- If the file/dir does not exist just return an error w/o calling any
krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
cc_dir_remove call cc_file_remove, so directory will not be removed.
Simo.
Rewritten patch attached.
LS
On Wed, 2013-04-10 at 11:45 +0200, Lukas Slebodnik wrote:
On (04/04/13 12:09), Simo Sorce wrote:
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
- Before calling krb5_cc_resolve() do a stat.
function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK)
Time to un-ignore it :-)
- If the file/dir does not exist just return an error w/o calling any
krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
cc_dir_remove call cc_file_remove, so directory will not be removed.
That is fine, we nee to check for dir/primary in dir case I think.
Simo.
On Wed, Apr 10, 2013 at 08:48:08AM -0400, Simo Sorce wrote:
On Wed, 2013-04-10 at 11:45 +0200, Lukas Slebodnik wrote:
On (04/04/13 12:09), Simo Sorce wrote:
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
- Before calling krb5_cc_resolve() do a stat.
function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK)
Time to un-ignore it :-)
- If the file/dir does not exist just return an error w/o calling any
krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
cc_dir_remove call cc_file_remove, so directory will not be removed.
That is fine, we nee to check for dir/primary in dir case I think.
Simo.
This patch fixes the error by the way. Basic sanity testing also went fine and the code looks good to me.
On Wed, 2013-04-10 at 15:09 +0200, Jakub Hrozek wrote:
On Wed, Apr 10, 2013 at 08:48:08AM -0400, Simo Sorce wrote:
On Wed, 2013-04-10 at 11:45 +0200, Lukas Slebodnik wrote:
On (04/04/13 12:09), Simo Sorce wrote:
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
- Before calling krb5_cc_resolve() do a stat.
function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK)
Time to un-ignore it :-)
- If the file/dir does not exist just return an error w/o calling any
krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
cc_dir_remove call cc_file_remove, so directory will not be removed.
That is fine, we nee to check for dir/primary in dir case I think.
Simo.
This patch fixes the error by the way. Basic sanity testing also went fine and the code looks good to me.
Sorry, meant to say it looks good to me too, but haven't tested.
Simo.
On (10/04/13 08:48), Simo Sorce wrote:
On Wed, 2013-04-10 at 11:45 +0200, Lukas Slebodnik wrote:
On (04/04/13 12:09), Simo Sorce wrote:
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
- Before calling krb5_cc_resolve() do a stat.
function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK)
Time to un-ignore it :-)
- If the file/dir does not exist just return an error w/o calling any
krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
cc_dir_remove call cc_file_remove, so directory will not be removed.
That is fine, we nee to check for dir/primary in dir case I think.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
Attaching updated patch, which also check dir/primary.
LS
On Wed, Apr 10, 2013 at 06:57:07PM +0200, Lukas Slebodnik wrote:
On (10/04/13 08:48), Simo Sorce wrote:
On Wed, 2013-04-10 at 11:45 +0200, Lukas Slebodnik wrote:
On (04/04/13 12:09), Simo Sorce wrote:
On Thu, 2013-04-04 at 12:14 +0200, Lukas Slebodnik wrote:
ehlo,
In krb5-libs 1.11, function krb5_cc_resolve verify if credential cache dir exists. If it doesn't exist, than it will be created. We have to ensure, that it will be created with right uid, gid and permissions.
There is check in krb_auth_send, whether cached user data in ldb contains SYSDB_CCACHE_FILE attribute. If it is available, then old ccache is checked in function check_old_ccache. Gdm clean user directory (in /run/user/) after logout. This is the reason why SYSDB_CCACHE_FILE attribute was found in ldb cache and directory did not exist and then in function krb5_cc_resolve was created with wrong permissions.
Patch attached.
Although this approach may work I am not really comfortable with it.
Here is an alternative approach:
- Before calling krb5_cc_resolve() do a stat.
function cc_residual_is_used call stat, but nonexisting file(directory) was ignored (handled like EOK)
Time to un-ignore it :-)
- If the file/dir does not exist just return an error w/o calling any
krb5 function at all.
I think this should also solve the issue, and will not involve change of uid/gid.
This method has a very small race that can hardly be triggered though, it would require someone to remove the cache after the stat and before krb5_cc_resolve() is called. In that case auth would fail, but we should be able to recover on the next authentication by removing the file if it has the wrong permissions (I think this is already done, if not we can fix it with a second patch that adds checking for uid/gid after the stat and a call to cc_file_rmove/cc_dir_remove).
cc_dir_remove call cc_file_remove, so directory will not be removed.
That is fine, we nee to check for dir/primary in dir case I think.
Simo.
-- Simo Sorce * Red Hat, Inc * New York
Attaching updated patch, which also check dir/primary.
LS
Just one nitpick, can you change this block:
+ primary_file = talloc_strdup_append(discard_const_p(char, dir), + "/primary"); + if (!primary_file) { + DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n")); + ret = ENOMEM; + goto done; + }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
I wonder how the code works correctly for you, I would have expected it to work only if primary_file was already a talloc pointer.
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
On Thu, Apr 11, 2013 at 12:06:22PM +0200, Lukas Slebodnik wrote:
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
Ack, works fine. I will just change one thing before pushing, no need to re-send:
ret = cc_residual_is_used(uid, dir, SSS_KRB5_TYPE_DIR, &active);
- talloc_free(tmp); if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. ""Will create a new one.\n"));
if (ret != ENOENT) {DEBUG(SSSDBG_OP_FAILURE,("Could not check if ccache is active.\n"));}
I will just change the debug level here to something more verbose. I think we should let the user know why the login failed.
On Fri, Apr 12, 2013 at 05:31:33PM +0200, Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 12:06:22PM +0200, Lukas Slebodnik wrote:
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
Ack, works fine. I will just change one thing before pushing, no need to re-send:
ret = cc_residual_is_used(uid, dir, SSS_KRB5_TYPE_DIR, &active);
- talloc_free(tmp); if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. ""Will create a new one.\n"));
if (ret != ENOENT) {DEBUG(SSSDBG_OP_FAILURE,("Could not check if ccache is active.\n"));}I will just change the debug level here to something more verbose. I think we should let the user know why the login failed.
Pushed to master.
Lukas, can you also send a version that applies cleanly on the sssd-1-9 branch? I think we should fix this bug in 1.9 too.
On (12/04/13 18:06), Jakub Hrozek wrote:
On Fri, Apr 12, 2013 at 05:31:33PM +0200, Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 12:06:22PM +0200, Lukas Slebodnik wrote:
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
Ack, works fine. I will just change one thing before pushing, no need to re-send:
ret = cc_residual_is_used(uid, dir, SSS_KRB5_TYPE_DIR, &active);
- talloc_free(tmp); if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. ""Will create a new one.\n"));
if (ret != ENOENT) {DEBUG(SSSDBG_OP_FAILURE,("Could not check if ccache is active.\n"));}I will just change the debug level here to something more verbose. I think we should let the user know why the login failed.
Pushed to master.
Lukas, can you also send a version that applies cleanly on the sssd-1-9 branch? I think we should fix this bug in 1.9 too.
Attaching patch for branch sssd-1-9. I did not test patch on RHEL 6, I have to create VM.
LS
On (13/04/13 00:31), Lukas Slebodnik wrote:
On (12/04/13 18:06), Jakub Hrozek wrote:
On Fri, Apr 12, 2013 at 05:31:33PM +0200, Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 12:06:22PM +0200, Lukas Slebodnik wrote:
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
Ack, works fine. I will just change one thing before pushing, no need to re-send:
ret = cc_residual_is_used(uid, dir, SSS_KRB5_TYPE_DIR, &active);
- talloc_free(tmp); if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. ""Will create a new one.\n"));
if (ret != ENOENT) {DEBUG(SSSDBG_OP_FAILURE,("Could not check if ccache is active.\n"));}I will just change the debug level here to something more verbose. I think we should let the user know why the login failed.
Pushed to master.
Lukas, can you also send a version that applies cleanly on the sssd-1-9 branch? I think we should fix this bug in 1.9 too.
Attaching patch for branch sssd-1-9. I did not test patch on RHEL 6, I have to create VM.
LS
Patch for sssd-1-9 works fine on RHEL6, but branch sssd-1-9 does not compile on with krb5-libs 1.11 (fedora 19).
You should also backport patch 4e78fab6a1b2e9653a7959cbdb7d54bb750041d0 "krb5: include backwards compatible declaration of krb5_trace_info" from master to sssd-1-9.
LS
On Mon, Apr 15, 2013 at 10:56:49AM +0200, Lukas Slebodnik wrote:
On (13/04/13 00:31), Lukas Slebodnik wrote:
On (12/04/13 18:06), Jakub Hrozek wrote:
On Fri, Apr 12, 2013 at 05:31:33PM +0200, Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 12:06:22PM +0200, Lukas Slebodnik wrote:
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote: > Just one nitpick, can you change this block: > > + primary_file = talloc_strdup_append(discard_const_p(char, dir), > + "/primary"); > + if (!primary_file) { > + DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n")); > + ret = ENOMEM; > + goto done; > + } > > Change dir to be "char *" not "const char*" and use > talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
Ack, works fine. I will just change one thing before pushing, no need to re-send:
ret = cc_residual_is_used(uid, dir, SSS_KRB5_TYPE_DIR, &active);
- talloc_free(tmp); if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. ""Will create a new one.\n"));
if (ret != ENOENT) {DEBUG(SSSDBG_OP_FAILURE,("Could not check if ccache is active.\n"));}I will just change the debug level here to something more verbose. I think we should let the user know why the login failed.
Pushed to master.
Lukas, can you also send a version that applies cleanly on the sssd-1-9 branch? I think we should fix this bug in 1.9 too.
Attaching patch for branch sssd-1-9. I did not test patch on RHEL 6, I have to create VM.
LS
Patch for sssd-1-9 works fine on RHEL6, but branch sssd-1-9 does not compile on with krb5-libs 1.11 (fedora 19).
You should also backport patch 4e78fab6a1b2e9653a7959cbdb7d54bb750041d0 "krb5: include backwards compatible declaration of krb5_trace_info" from master to sssd-1-9.
Yes, luckily that patch applies fine without backporting so I will just push it.
On Sat, Apr 13, 2013 at 12:31:24AM +0200, Lukas Slebodnik wrote:
On (12/04/13 18:06), Jakub Hrozek wrote:
On Fri, Apr 12, 2013 at 05:31:33PM +0200, Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 12:06:22PM +0200, Lukas Slebodnik wrote:
On (11/04/13 10:29), Jakub Hrozek wrote:
On Thu, Apr 11, 2013 at 10:28:01AM +0200, Jakub Hrozek wrote:
Just one nitpick, can you change this block:
- primary_file = talloc_strdup_append(discard_const_p(char, dir),
"/primary");- if (!primary_file) {
DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_strdup_append failed.\n"));ret = ENOMEM;goto done;- }
Change dir to be "char *" not "const char*" and use talloc_asprintf(tmp_ctx, "%s/%s", dir);
talloc_asprintf(tmp_ctx, "%s/primary", dir);
sorry.
Patch attached.
LS
Ack, works fine. I will just change one thing before pushing, no need to re-send:
ret = cc_residual_is_used(uid, dir, SSS_KRB5_TYPE_DIR, &active);
- talloc_free(tmp); if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, ("Could not check if ccache is active. ""Will create a new one.\n"));
if (ret != ENOENT) {DEBUG(SSSDBG_OP_FAILURE,("Could not check if ccache is active.\n"));}I will just change the debug level here to something more verbose. I think we should let the user know why the login failed.
Pushed to master.
Lukas, can you also send a version that applies cleanly on the sssd-1-9 branch? I think we should fix this bug in 1.9 too.
Attaching patch for branch sssd-1-9. I did not test patch on RHEL 6, I have to create VM.
LS
Ack
On Mon, Apr 15, 2013 at 11:44:07AM +0200, Jakub Hrozek wrote:
Pushed to master.
Lukas, can you also send a version that applies cleanly on the sssd-1-9 branch? I think we should fix this bug in 1.9 too.
Attaching patch for branch sssd-1-9. I did not test patch on RHEL 6, I have to create VM.
LS
Ack
Pushed to sssd-1-9
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Simo Sorce