[PATCH] KRB5: Call umask before mkstemp in the krb5 child code

[Jakub Hrozek]

Found by Coverity. Not really a huge issue, but as already agreed in a
private conversation, a nice-to-have.

[Lukas Slebodnik]

On (16/09/13 17:18), Jakub Hrozek wrote:
> Found by Coverity. Not really a huge issue, but as already agreed in a
> private conversation, a nice-to-have.
> From ad964e8d54f1a47ea0cf580a70de71f90f0f9140 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek 
> Date: Mon, 16 Sep 2013 17:02:39 +0200
> Subject: [PATCH] KRB5: Call umask before mkstemp in the krb5 child code
>
> src/providers/krb5/krb5_child.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
> index f896332d60626149e188703a7ddf182bdd47443d..032f5020ebe4c2ab5505c6f77e52d201366e50d8 100644
> --- a/src/providers/krb5/krb5_child.c
> +++ b/src/providers/krb5/krb5_child.c
> @@ -439,6 +439,7 @@ static errno_t handle_randomized(char *in)
>     char *ccname = NULL;
>     int ret;
>     int fd;
>
> mode_t old_umask;
>
> /* We only treat the FILE type case in a special way due to the history
> of storing FILE type ccache in /tmp and associated security issues */
>
> @@ -460,7 +461,9 @@ static errno_t handle_randomized(char *in)
>          * something races, we mostly care only about not accidentally use
>          * an existing name and thus failing in the process of saving the
>          * cache. Malicious races can only be avoided by libkrb5 itself. */
>    old_umask = umask(077);
>   fd = mkstemp(ccname);
>    umask(old_umask);
>   if (fd == -1) {
>       ret = errno;
>       DEBUG(SSSDBG_CRIT_FAILURE, ("mkstemp(\"%s\") failed!\n", ccname));

I checked Coverity and warning is fixed.

ACK

LS

[Jakub Hrozek]

On Tue, Sep 17, 2013 at 09:33:54AM +0200, Lukas Slebodnik wrote:
> On (16/09/13 17:18), Jakub Hrozek wrote:
> > Found by Coverity. Not really a huge issue, but as already agreed in a
> > private conversation, a nice-to-have.
> > From ad964e8d54f1a47ea0cf580a70de71f90f0f9140 Mon Sep 17 00:00:00 2001
> > From: Jakub Hrozek 
> > Date: Mon, 16 Sep 2013 17:02:39 +0200
> > Subject: [PATCH] KRB5: Call umask before mkstemp in the krb5 child code
> >
> > src/providers/krb5/krb5_child.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
> > index f896332d60626149e188703a7ddf182bdd47443d..032f5020ebe4c2ab5505c6f77e52d201366e50d8 100644
> > --- a/src/providers/krb5/krb5_child.c
> > +++ b/src/providers/krb5/krb5_child.c
> > @@ -439,6 +439,7 @@ static errno_t handle_randomized(char *in)
> >     char *ccname = NULL;
> >     int ret;
> >     int fd;
> >
> > mode_t old_umask;
> >
> > /* We only treat the FILE type case in a special way due to the history
> > of storing FILE type ccache in /tmp and associated security issues */
> >
> > @@ -460,7 +461,9 @@ static errno_t handle_randomized(char *in)
> >          * something races, we mostly care only about not accidentally use
> >          * an existing name and thus failing in the process of saving the
> >          * cache. Malicious races can only be avoided by libkrb5 itself. */
> >    old_umask = umask(077);
> >   fd = mkstemp(ccname);
> >    umask(old_umask);
> >   if (fd == -1) {
> >       ret = errno;
> >       DEBUG(SSSDBG_CRIT_FAILURE, ("mkstemp(\"%s\") failed!\n", ccname));
>
> I checked Coverity and warning is fixed.
>
> ACK
>
> LS

Pushed to master and sssd-1-11