[patch] login_yubikey: delete keys

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] login_yubikey: delete keys

Fritjof Bornebusch
Wipe out the key from "user.key".

--f.

Index: login_yubikey.c
===================================================================
RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 login_yubikey.c
--- login_yubikey.c     16 Jan 2015 06:39:50 -0000      1.10
+++ login_yubikey.c     31 Mar 2016 09:38:01 -0000
@@ -224,6 +224,8 @@ yubikey_login(const char *username, cons
        yubikey_hex_decode(uid, hexuid, YUBIKEY_UID_SIZE);
        yubikey_hex_decode(key, hexkey, YUBIKEY_KEY_SIZE);
 
+       explicit_bzero(hexkey, sizeof(hexkey));
+
        /*
         * Cycle through the key mapping table.
          * XXX brute force, unoptimized; a lookup table for valid mappings may
@@ -268,6 +270,8 @@ yubikey_login(const char *username, cons
                }
                break; /* only reached through the bottom of case 0 */
        }
+
+       explicit_bzero(key, sizeof(key));
 
        syslog(LOG_INFO, "user %s uid %s: %d matching keymaps (%d checked), "
            "%d crc ok", username, hexuid, mapok, i, crcok);

Reply | Threaded
Open this post in threaded view
|

Re: [patch] login_yubikey: delete keys

Sebastian Benoit-3
Hi Fritjof,

[hidden email]([hidden email]) on 2016.03.31 11:43:58 +0200:

> Wipe out the key from "user.key".
>
> --f.
>
> Index: login_yubikey.c
> ===================================================================
> RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.c,v
> retrieving revision 1.10
> diff -u -p -u -r1.10 login_yubikey.c
> --- login_yubikey.c     16 Jan 2015 06:39:50 -0000      1.10
> +++ login_yubikey.c     31 Mar 2016 09:38:01 -0000
> @@ -224,6 +224,8 @@ yubikey_login(const char *username, cons
>         yubikey_hex_decode(uid, hexuid, YUBIKEY_UID_SIZE);
>         yubikey_hex_decode(key, hexkey, YUBIKEY_KEY_SIZE);
>  
> +       explicit_bzero(hexkey, sizeof(hexkey));
> +
>         /*
>          * Cycle through the key mapping table.
>           * XXX brute force, unoptimized; a lookup table for valid mappings may
> @@ -268,6 +270,8 @@ yubikey_login(const char *username, cons
>                 }
>                 break; /* only reached through the bottom of case 0 */
>         }
> +
> +       explicit_bzero(key, sizeof(key));

The while loop above has return(AUTH_FAILED) so you dont zero in those
cases. Can you change that?

>  
>         syslog(LOG_INFO, "user %s uid %s: %d matching keymaps (%d checked), "
>             "%d crc ok", username, hexuid, mapok, i, crcok);
>

Also your diff does not apply, i think it has tab vs space issues.

Reply | Threaded
Open this post in threaded view
|

Re: [patch] login_yubikey: delete keys

Fritjof Bornebusch
On Thu, Mar 31, 2016 at 10:17:45PM +0200, Sebastian Benoit wrote:

> Hi Fritjof,
>
> [hidden email]([hidden email]) on 2016.03.31 11:43:58 +0200:
> > Wipe out the key from "user.key".
> >
> > --f.
> >
> The while loop above has return(AUTH_FAILED) so you dont zero in those
> cases. Can you change that?
>

Yeah, sure. See patch below.

>
> Also your diff does not apply, i think it has tab vs space issues.
>

Ah, shit. Should work now.


Index: login_yubikey.c
===================================================================
RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.c,v
retrieving revision 1.13
diff -u -r1.13 login_yubikey.c
--- login_yubikey.c 22 Oct 2015 23:56:30 -0000 1.13
+++ login_yubikey.c 31 Mar 2016 21:35:28 -0000
@@ -228,6 +228,8 @@
  yubikey_hex_decode(uid, hexuid, YUBIKEY_UID_SIZE);
  yubikey_hex_decode(key, hexkey, YUBIKEY_KEY_SIZE);
 
+ explicit_bzero(hexkey, sizeof(hexkey));
+
  /*
  * Cycle through the key mapping table.
          * XXX brute force, unoptimized; a lookup table for valid mappings may
@@ -239,6 +241,7 @@
  case EMSGSIZE:
  syslog(LOG_INFO, "user %s failed: password too short.",
     username);
+ explicit_bzero(key, sizeof(key));
  return (AUTH_FAILED);
  case EINVAL: /* keyboard mapping invalid */
  continue;
@@ -264,14 +267,18 @@
  syslog(LOG_INFO, "user %s: could not decode password "
     "with any keymap (%d crc ok)",
     username, crcok);
+ explicit_bzero(key, sizeof(key));
  return (AUTH_FAILED);
  default:
  syslog(LOG_DEBUG, "user %s failed: %s",
     username, strerror(r));
+ explicit_bzero(key, sizeof(key));
  return (AUTH_FAILED);
  }
  break; /* only reached through the bottom of case 0 */
  }
+
+ explicit_bzero(key, sizeof(key));
 
  syslog(LOG_INFO, "user %s uid %s: %d matching keymaps (%d checked), "
     "%d crc ok", username, hexuid, mapok, i, crcok);

Reply | Threaded
Open this post in threaded view
|

Re: [patch] login_yubikey: delete keys

Sebastian Benoit-3
[hidden email]([hidden email]) on 2016.03.31 23:43:54 +0200:

> On Thu, Mar 31, 2016 at 10:17:45PM +0200, Sebastian Benoit wrote:
> > Hi Fritjof,
> >
> > [hidden email]([hidden email]) on 2016.03.31 11:43:58 +0200:
> > > Wipe out the key from "user.key".
> > >
> > > --f.
> > >
> > The while loop above has return(AUTH_FAILED) so you dont zero in those
> > cases. Can you change that?
> >
>
> Yeah, sure. See patch below.
>
> >
> > Also your diff does not apply, i think it has tab vs space issues.
> >
>
> Ah, shit. Should work now.

Thanks.

ok benno@
 

> Index: login_yubikey.c
> ===================================================================
> RCS file: /cvs/src/libexec/login_yubikey/login_yubikey.c,v
> retrieving revision 1.13
> diff -u -r1.13 login_yubikey.c
> --- login_yubikey.c 22 Oct 2015 23:56:30 -0000 1.13
> +++ login_yubikey.c 31 Mar 2016 21:35:28 -0000
> @@ -228,6 +228,8 @@
>   yubikey_hex_decode(uid, hexuid, YUBIKEY_UID_SIZE);
>   yubikey_hex_decode(key, hexkey, YUBIKEY_KEY_SIZE);
>  
> + explicit_bzero(hexkey, sizeof(hexkey));
> +
>   /*
>   * Cycle through the key mapping table.
>           * XXX brute force, unoptimized; a lookup table for valid mappings may
> @@ -239,6 +241,7 @@
>   case EMSGSIZE:
>   syslog(LOG_INFO, "user %s failed: password too short.",
>      username);
> + explicit_bzero(key, sizeof(key));
>   return (AUTH_FAILED);
>   case EINVAL: /* keyboard mapping invalid */
>   continue;
> @@ -264,14 +267,18 @@
>   syslog(LOG_INFO, "user %s: could not decode password "
>      "with any keymap (%d crc ok)",
>      username, crcok);
> + explicit_bzero(key, sizeof(key));
>   return (AUTH_FAILED);
>   default:
>   syslog(LOG_DEBUG, "user %s failed: %s",
>      username, strerror(r));
> + explicit_bzero(key, sizeof(key));
>   return (AUTH_FAILED);
>   }
>   break; /* only reached through the bottom of case 0 */
>   }
> +
> + explicit_bzero(key, sizeof(key));
>  
>   syslog(LOG_INFO, "user %s uid %s: %d matching keymaps (%d checked), "
>      "%d crc ok", username, hexuid, mapok, i, crcok);
>

--