This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] FIPS compliance and other crypt(3) improvements


On Sep  5, 2012, Andreas Jaeger <aj@suse.com> wrote:

> Please post the complete patch set again - and remove the Contributed by 
> lines you have in the new files:

Done.  lxoliva/crypt-fips-bz811753 updated, too.

Reject out-of-spec salt passed to DES crypt

From: Alexandre Oliva <aoliva@redhat.com>

for  ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* crypt/crypt-private.h: Include stdbool.h.
	(_ufc_setup_salt_r): Return bool.
	* crypt/crypt-entry.c: Include errno.h.
	(__crypt_r): Return NULL with EINVAL for bad salt.
	* crypt/crypt_util.c (bad_for_salt): New.
	(_ufc_setup_salt_r): Check that salt is long enough and within
	the specified alphabet.
	* crypt/badsalttest.c: New file.
	* crypt/Makefile (tests): Add it.
	($(objpfx)badsalttest): New.
---

 crypt/Makefile        |    2 +
 crypt/badsalttest.c   |   85 +++++++++++++++++++++++++++++++++++++++++++++++++
 crypt/crypt-entry.c   |    7 +++-
 crypt/crypt-private.h |    3 +-
 crypt/crypt_util.c    |   44 +++++++++++++++++++++++--
 5 files changed, 134 insertions(+), 7 deletions(-)
 create mode 100644 crypt/badsalttest.c


diff --git a/crypt/Makefile b/crypt/Makefile
index 3a61865..3d4f243 100644
--- a/crypt/Makefile
+++ b/crypt/Makefile
@@ -28,7 +28,7 @@ extra-libs-others := $(extra-libs)
 libcrypt-routines := crypt-entry md5-crypt sha256-crypt sha512-crypt crypt \
 		     crypt_util
 
-tests := cert md5c-test sha256c-test sha512c-test
+tests := cert md5c-test sha256c-test sha512c-test badsalttest
 
 include ../Makeconfig
 
diff --git a/crypt/badsalttest.c b/crypt/badsalttest.c
new file mode 100644
index 0000000..ec9abde
--- /dev/null
+++ b/crypt/badsalttest.c
@@ -0,0 +1,85 @@
+/* Test program for bad DES salt detection in crypt.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <crypt.h>
+
+static const char *tests[][2] = {
+  { "no salt", "" },
+  { "single char", "/" },
+  { "first char bad", "!x" },
+  { "second char bad", "Z%" },
+  { "both chars bad", ":@" },
+  { "un$upported algorithm", "$2$" },
+  { "unsupported_algorithm", "_1" },
+  { "end of page", NULL }
+};
+
+static int
+do_test (void)
+{
+  int result = 0;
+  struct crypt_data cd;
+  size_t n = sizeof (tests) / sizeof (*tests);
+  size_t pagesize = (size_t) sysconf (_SC_PAGESIZE);
+  char *page;
+
+  /* Check that crypt won't look at the second character if the first
+     one is invalid.  */
+  page = mmap (NULL, pagesize * 2, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (page == MAP_FAILED)
+    {
+      perror ("mmap");
+      n--;
+    }
+  else
+    {
+      if (mmap (page + pagesize, pagesize, 0,
+		MAP_PRIVATE | MAP_ANON | MAP_FIXED,
+		-1, 0) != page + pagesize)
+	perror ("mmap 2");
+      page[pagesize - 1] = '*';
+      tests[n - 1][1] = &page[pagesize - 1];
+    }
+
+  for (size_t i = 0; i < n; i++)
+    {
+      if (crypt (tests[i][0], tests[i][1]))
+	{
+	  result++;
+	  printf ("%s: crypt returned non-NULL with salt \"%s\"\n",
+		  tests[i][0], tests[i][1]);
+	}
+
+      if (crypt_r (tests[i][0], tests[i][1], &cd))
+	{
+	  result++;
+	  printf ("%s: crypt_r returned non-NULL with salt \"%s\"\n",
+		  tests[i][0], tests[i][1]);
+	}
+    }
+
+  return result;
+}
+
+#define TIMEOUT 5
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/crypt/crypt-entry.c b/crypt/crypt-entry.c
index 91e2c4e..9fb22bd 100644
--- a/crypt/crypt-entry.c
+++ b/crypt/crypt-entry.c
@@ -27,6 +27,7 @@
 #include <stdio.h>
 #endif
 #include <string.h>
+#include <errno.h>
 
 #ifndef STATIC
 #define STATIC static
@@ -108,7 +109,11 @@ __crypt_r (key, salt, data)
   /*
    * Hack DES tables according to salt
    */
-  _ufc_setup_salt_r (salt, data);
+  if (!_ufc_setup_salt_r (salt, data))
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
 
   /*
    * Setup key schedule
diff --git a/crypt/crypt-private.h b/crypt/crypt-private.h
index b4bfa8b..54418fc 100644
--- a/crypt/crypt-private.h
+++ b/crypt/crypt-private.h
@@ -26,6 +26,7 @@
 #define CRYPT_PRIVATE_H	1
 
 #include <features.h>
+#include <stdbool.h>
 
 /* crypt.c */
 extern void _ufc_doit_r (ufc_long itr, struct crypt_data * __restrict __data,
@@ -36,7 +37,7 @@ extern void _ufc_doit_r (ufc_long itr, struct crypt_data * __restrict __data,
 extern void __init_des_r (struct crypt_data * __restrict __data);
 extern void __init_des (void);
 
-extern void _ufc_setup_salt_r (const char *s,
+extern bool _ufc_setup_salt_r (const char *s,
 			       struct crypt_data * __restrict __data);
 extern void _ufc_mk_keytab_r (const char *key,
 			      struct crypt_data * __restrict __data);
diff --git a/crypt/crypt_util.c b/crypt/crypt_util.c
index a1ff88b..269371e 100644
--- a/crypt/crypt_util.c
+++ b/crypt/crypt_util.c
@@ -57,6 +57,7 @@ STATIC void shuffle_sb (long32 *k, ufc_long saltbits);
 #else
 STATIC void shuffle_sb (long64 *k, ufc_long saltbits);
 #endif
+static bool bad_for_salt (char c);
 
 
 /*
@@ -596,23 +597,56 @@ shuffle_sb(k, saltbits)
 #endif
 
 /*
+ * Return false iff C is in the specified alphabet for crypt salt.
+ */
+
+static bool
+bad_for_salt (c)
+     char c;
+{
+  switch (c)
+    {
+    case '0' ... '9':
+    case 'A' ... 'Z':
+    case 'a' ... 'z':
+    case '.': case '/':
+      return false;
+
+    default:
+      return true;
+    }
+}
+
+/*
  * Setup the unit for a new salt
  * Hopefully we'll not see a new salt in each crypt call.
+ * Return false if an unexpected character was found in s[0] or s[1].
  */
 
-void
+bool
 _ufc_setup_salt_r(s, __data)
      const char *s;
      struct crypt_data * __restrict __data;
 {
   ufc_long i, j, saltbits;
+  char s0, s1;
 
   if(__data->initialized == 0)
     __init_des_r(__data);
 
-  if(s[0] == __data->current_salt[0] && s[1] == __data->current_salt[1])
-    return;
-  __data->current_salt[0] = s[0]; __data->current_salt[1] = s[1];
+  s0 = s[0];
+  if(bad_for_salt (s0))
+    return false;
+
+  s1 = s[1];
+  if(bad_for_salt (s1))
+    return false;
+
+  if(s0 == __data->current_salt[0] && s1 == __data->current_salt[1])
+    return true;
+
+  __data->current_salt[0] = s0;
+  __data->current_salt[1] = s1;
 
   /*
    * This is the only crypt change to DES:
@@ -646,6 +680,8 @@ _ufc_setup_salt_r(s, __data)
   shuffle_sb((LONGG)__data->sb3, __data->current_saltbits ^ saltbits);
 
   __data->current_saltbits = saltbits;
+
+  return true;
 }
 
 void
Disable MD5 and DES crypt in FIPS mode

From: Alexandre Oliva <aoliva@redhat.com>

for  ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* crypt/crypt-entry.c: Include fips-private.h.
	(__crypt_r, __crypt): Disable MD5 and DES if FIPS is enabled.
	* crypt/md5c-test.c (main): Tolerate disabled MD5.
	* sysdeps/unix/sysv/linux/fips-private.h: New file.
	* sysdeps/generic/fips-private.h: New file, dummy fallback.
---

 crypt/crypt-entry.c                    |   24 +++++++++--
 crypt/md5c-test.c                      |    5 ++
 sysdeps/generic/fips-private.h         |   36 ++++++++++++++++
 sysdeps/unix/sysv/linux/fips-private.h |   73 ++++++++++++++++++++++++++++++++
 4 files changed, 134 insertions(+), 4 deletions(-)
 create mode 100644 sysdeps/generic/fips-private.h
 create mode 100644 sysdeps/unix/sysv/linux/fips-private.h


diff --git a/crypt/crypt-entry.c b/crypt/crypt-entry.c
index 9fb22bd..89c22e6 100644
--- a/crypt/crypt-entry.c
+++ b/crypt/crypt-entry.c
@@ -28,6 +28,7 @@
 #endif
 #include <string.h>
 #include <errno.h>
+#include <fips-private.h>
 
 #ifndef STATIC
 #define STATIC static
@@ -92,8 +93,16 @@ __crypt_r (key, salt, data)
 #ifdef _LIBC
   /* Try to find out whether we have to use MD5 encryption replacement.  */
   if (strncmp (md5_salt_prefix, salt, sizeof (md5_salt_prefix) - 1) == 0)
-    return __md5_crypt_r (key, salt, (char *) data,
-			  sizeof (struct crypt_data));
+    {
+      /* FIPS rules out MD5 password encryption.  */
+      if (fips_enabled_p ())
+	{
+	  __set_errno (EPERM);
+	  return NULL;
+	}
+      return __md5_crypt_r (key, salt, (char *) data,
+			    sizeof (struct crypt_data));
+    }
 
   /* Try to find out whether we have to use SHA256 encryption replacement.  */
   if (strncmp (sha256_salt_prefix, salt, sizeof (sha256_salt_prefix) - 1) == 0)
@@ -115,6 +124,13 @@ __crypt_r (key, salt, data)
       return NULL;
     }
 
+  /* FIPS rules out DES password encryption.  */
+  if (fips_enabled_p ())
+    {
+      __set_errno (EPERM);
+      return NULL;
+    }
+
   /*
    * Setup key schedule
    */
@@ -148,7 +164,9 @@ crypt (key, salt)
 {
 #ifdef _LIBC
   /* Try to find out whether we have to use MD5 encryption replacement.  */
-  if (strncmp (md5_salt_prefix, salt, sizeof (md5_salt_prefix) - 1) == 0)
+  if (strncmp (md5_salt_prefix, salt, sizeof (md5_salt_prefix) - 1) == 0
+      /* Let __crypt_r deal with the error code if FIPS is enabled.  */
+      && !fips_enabled_p ())
     return __md5_crypt (key, salt);
 
   /* Try to find out whether we have to use SHA256 encryption replacement.  */
diff --git a/crypt/md5c-test.c b/crypt/md5c-test.c
index f56d0eb..c80e402 100644
--- a/crypt/md5c-test.c
+++ b/crypt/md5c-test.c
@@ -9,7 +9,10 @@ main (int argc, char *argv[])
   int result = 0;
 
   cp = crypt ("Hello world!", salt);
-  result |= strcmp ("$1$saltstri$YMyguxXMBpd2TEZ.vS/3q1", cp);
+
+  /* MD5 is disabled in FIPS mode.  */
+  if (cp)
+    result |= strcmp ("$1$saltstri$YMyguxXMBpd2TEZ.vS/3q1", cp);
 
   return result;
 }
diff --git a/sysdeps/generic/fips-private.h b/sysdeps/generic/fips-private.h
new file mode 100644
index 0000000..0dff087
--- /dev/null
+++ b/sysdeps/generic/fips-private.h
@@ -0,0 +1,36 @@
+/* Dummy implementation of FIPS compliance status test.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _FIPS_PRIVATE_H
+#define _FIPS_PRIVATE_H
+
+#include <stdbool.h>
+
+/* Return true if compliance with the FIPS security standards is
+   enabled.
+
+   This is only relevant within crypt, to tell whether MD5 and DES
+   algorithms should be rejected.  */
+
+static inline bool
+fips_enabled_p (void)
+{
+  return false;
+}
+
+#endif /* _FIPS_PRIVATE_H */
diff --git a/sysdeps/unix/sysv/linux/fips-private.h b/sysdeps/unix/sysv/linux/fips-private.h
new file mode 100644
index 0000000..0f24925
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/fips-private.h
@@ -0,0 +1,73 @@
+/* FIPS compliance status test for GNU/Linux systems.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _FIPS_PRIVATE_H
+#define _FIPS_PRIVATE_H
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <not-cancel.h>
+#include <stdbool.h>
+
+/* Return true if FIPS mode is enabled.  See
+   sysdeps/generic/fips-private.h for more information.  */
+
+static inline bool
+fips_enabled_p (void)
+{
+  static enum {
+    FIPS_UNTESTED = 0,
+    FIPS_ENABLED = 1,
+    FIPS_DISABLED = -1,
+    FIPS_TEST_FAILED = -2
+  } checked;
+
+  if (checked == FIPS_UNTESTED)
+    {
+      int fd = open_not_cancel_2 ("/proc/sys/crypto/fips_enabled", O_RDONLY);
+
+      if (fd != -1)
+	{
+	  /* This is more than enough, the file contains a single integer.  */
+	  char buf[32];
+	  ssize_t n;
+	  n = TEMP_FAILURE_RETRY (read_not_cancel (fd, buf, sizeof (buf) - 1));
+	  close_not_cancel_no_status (fd);
+
+	  if (n > 0)
+	    {
+	      /* Terminate the string.  */
+	      buf[n] = '\0';
+
+	      char *endp;
+	      long int res = strtol (buf, &endp, 10);
+	      if (endp != buf && (*endp == '\0' || *endp == '\n'))
+		checked = (res > 0) ? FIPS_ENABLED : FIPS_DISABLED;
+	    }
+	}
+
+      if (checked == FIPS_UNTESTED)
+	checked = FIPS_TEST_FAILED;
+    }
+
+  return checked > FIPS_UNTESTED;
+}
+
+#endif /* _FIPS_PRIVATE_H */

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]