[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patches] [PATCH] EGLIBC_CRYPT and EGLIBC_CRYPT_UFC option groups



Joseph S. Myers wrote:
On Sat, 14 Nov 2009, Maxim Kuvyrkov wrote:

The second option group enables removal of the ultra-fast implementation of
the DES algorithm (MD5 will be used by default instead).  The two reasons for

Could you clarify this "MD5 will be used by default instead" instead?

I thought that MD5 can serve as a drop in replacement for DES, apparently, that's not the case; e.g., invalid DES salts like "$$" may produce strange results.

The different algorithms have different salt syntax. If DES support has been disabled, the use of a salt with DES syntax should make crypt return an error rather than doing something with MD5; only a salt starting $1$ should cause MD5 to be used. Will such errors be properly given if DES is disabled with this patch?

Thank you for pointing this out. I've fixed the crypt[_r] functions to fail when DES is requested but not available, the fail condition is the same as with DES not being available due to export restrictions: result == NULL, errno set to ENOSYS.

Both incremental and full updated patches are attached. Any further comments?

Regards,

--
Maxim Kuvyrkov
CodeSourcery
maxim@xxxxxxxxxxxxxxxx
(650) 331-3385 x724
diff --git a/crypt/crypt-entry.c b/crypt/crypt-entry.c
index 145c40f..838a4c8 100644
--- a/crypt/crypt-entry.c
+++ b/crypt/crypt-entry.c
@@ -29,6 +29,7 @@
 #endif
 #include <string.h>
 #include <gnu/option-groups.h>
+#include <errno.h>
 
 #ifndef STATIC
 #define STATIC static
@@ -141,8 +142,8 @@ __crypt_r (key, salt, data)
   _ufc_output_conversion_r (res[0], res[1], salt, data);
   return data->crypt_3_buf;
 #else /* __OPTION_EGLIBC_CRYPT_UFC */
-  return __md5_crypt_r (key, salt, (char *) data,
-			sizeof (struct crypt_data));
+  __set_errno (ENOSYS);
+  return NULL;
 #endif /* __OPTION_EGLIBC_CRYPT_UFC */
 }
 weak_alias (__crypt_r, crypt_r)
@@ -169,7 +170,8 @@ crypt (key, salt)
 #if __OPTION_EGLIBC_CRYPT_UFC
   return __crypt_r (key, salt, &_ufc_foobar);
 #else /* __OPTION_EGLIBC_CRYPT_UFC */
-  return __md5_crypt (key, salt);
+  __set_errno (ENOSYS);
+  return NULL;
 #endif /* __OPTION_EGLIBC_CRYPT_UFC */
 }
 
diff --git a/option-groups.def b/option-groups.def
index 4d13a0a..177634a 100644
--- a/option-groups.def
+++ b/option-groups.def
@@ -212,8 +212,9 @@ config OPTION_EGLIBC_CRYPT_UFC
        This option group provides ultra fast DES-based implementation of
        the `crypt' function.  When this option group is disabled,
        (a) the library will not provide the setkey[_r] and encrypt[_r]
-       functions and (b) the crypt[_r] function will use the MD5 algorithm
-       by default.
+       functions and (b) the crypt[_r] function will return NULL and set the
+       errno to ENOSYS if /salt/ passed does not correspond to either MD5,
+       SHA-256 or SHA-512 algorithm.
 
 config OPTION_EGLIBC_DB_ALIASES
    bool "Functions for accessing the mail aliases database"
2009-11-14  Maxim Kuvyrkov  <maxim@xxxxxxxxxxxxxxxx>

	OPTION_EGLIBC_CRYPT and OPTION_EGLIBC_CRYPT_UFC.
	
	* option-groups.def: Describe new option groups.
	* option-groups.defaults: Set defaults.
	* crypt/Makefile: Don't build libcrypt if OPTION_EGLIBC_CRYPT is not
	selected, don't compile UFC implementation if OPTION_EGLIBC_CRYPT_UFC
	is not selected; adjust tests accordingly.
	* crypt/crypt-entry.c: Include <gnu/option-groups.h> and <errno.h>.
	(__crypt_r, crypt): Handle OPTION_EGLIBC_CRYPT_UFC, fail if DES
	is not available.
diff --git a/crypt/Makefile b/crypt/Makefile
index b9c8797..2585e39 100644
--- a/crypt/Makefile
+++ b/crypt/Makefile
@@ -19,24 +19,28 @@
 #
 #	Sub-makefile for crypt() portion of the library.
 #
+include ../option-groups.mak
+
 subdir	:= crypt
 
 headers := crypt.h
 
-extra-libs := libcrypt
-extra-libs-others := $(extra-libs)
+extra-libs-$(OPTION_EGLIBC_CRYPT) := libcrypt
+extra-libs-others-y := $(extra-libs-y)
 
-libcrypt-routines := crypt-entry md5-crypt sha256-crypt sha512-crypt crypt \
-		     crypt_util
+libcrypt-routines := crypt-entry md5-crypt sha256-crypt sha512-crypt
+libcrypt-routines-$(OPTION_EGLIBC_CRYPT_UFC) := crypt_util crypt
+libcrypt-routines += $(libcrypt-routines-y)
 
-tests := cert md5c-test sha256c-test sha512c-test
+tests-$(OPTION_EGLIBC_CRYPT) := md5c-test sha256c-test sha512c-test
+tests-$(OPTION_EGLIBC_CRYPT_UFC) += cert
 
 distribute := ufc-crypt.h crypt-private.h ufc.c speeds.c README.ufc-crypt \
 	      Banner md5.h sha256.h sha512.h
 
 include ../Makeconfig
 
-ifeq ($(crypt-in-libc),yes)
+ifeq ($(crypt-in-libc)$(OPTION_EGLIBC_CRYPT),yesy)
 routines += $(libcrypt-routines)
 endif
 
@@ -48,7 +52,7 @@ LDLIBS-crypt.so = -lfreebl3
 else
 libcrypt-routines += md5 sha256 sha512
 
-tests += md5test sha256test sha512test
+tests-$(OPTION_EGLIBC_CRYPT) += md5test sha256test sha512test
 
 $(objpfx)md5test: $(objpfx)md5.o
 $(objpfx)sha256test: $(objpfx)sha256.o
@@ -57,6 +61,7 @@ endif
 
 include ../Rules
 
+ifeq ($(OPTION_EGLIBC_CRYPT),y)
 ifeq (yes,$(build-shared))
 $(addprefix $(objpfx),$(tests)): $(objpfx)libcrypt.so
 else
@@ -65,6 +70,7 @@ endif
 ifeq (yes,$(build-bounded))
 $(tests:%=$(objpfx)%-bp): $(objpfx)libcrypt_b.a
 endif
+endif # eglibc: OPTION_EGLIBC_CRYPT
 
 # Depend on libc.so so a DT_NEEDED is generated in the shared objects.
 # This ensures they will load libc.so for needed symbols if loaded by
diff --git a/crypt/crypt-entry.c b/crypt/crypt-entry.c
index fdddad2..838a4c8 100644
--- a/crypt/crypt-entry.c
+++ b/crypt/crypt-entry.c
@@ -28,6 +28,8 @@
 #include <stdio.h>
 #endif
 #include <string.h>
+#include <gnu/option-groups.h>
+#include <errno.h>
 
 #ifndef STATIC
 #define STATIC static
@@ -87,9 +89,11 @@ __crypt_r (key, salt, data)
      const char *salt;
      struct crypt_data * __restrict data;
 {
+#if __OPTION_EGLIBC_CRYPT_UFC
   ufc_long res[4];
   char ktab[9];
   ufc_long xx = 25; /* to cope with GCC long long compiler bugs */
+#endif /*__OPTION_EGLIBC_CRYPT_UFC*/
 
 #ifdef _LIBC
   /* Try to find out whether we have to use MD5 encryption replacement.  */
@@ -108,6 +112,7 @@ __crypt_r (key, salt, data)
 			     sizeof (struct crypt_data));
 #endif
 
+#if __OPTION_EGLIBC_CRYPT_UFC
   /*
    * Hack DES tables according to salt
    */
@@ -136,6 +141,10 @@ __crypt_r (key, salt, data)
    */
   _ufc_output_conversion_r (res[0], res[1], salt, data);
   return data->crypt_3_buf;
+#else /* __OPTION_EGLIBC_CRYPT_UFC */
+  __set_errno (ENOSYS);
+  return NULL;
+#endif /* __OPTION_EGLIBC_CRYPT_UFC */
 }
 weak_alias (__crypt_r, crypt_r)
 
@@ -158,7 +167,12 @@ crypt (key, salt)
     return __sha512_crypt (key, salt);
 #endif
 
+#if __OPTION_EGLIBC_CRYPT_UFC
   return __crypt_r (key, salt, &_ufc_foobar);
+#else /* __OPTION_EGLIBC_CRYPT_UFC */
+  __set_errno (ENOSYS);
+  return NULL;
+#endif /* __OPTION_EGLIBC_CRYPT_UFC */
 }
 
 
diff --git a/option-groups.def b/option-groups.def
index 26ef4bb..177634a 100644
--- a/option-groups.def
+++ b/option-groups.def
@@ -198,6 +198,24 @@ config OPTION_EGLIBC_CHARSETS
           WCHAR_T           - EGLIBC's internal form (target-endian,
                               32-bit ISO 10646)
 
+config OPTION_EGLIBC_CRYPT
+   bool "Encryption library"
+   help
+       This option group includes the `libcrypt' library which
+       provides functions for one-way encryption.  Supported
+       encryption algorithms include MD5, SHA-256, SHA-512 and DES.
+
+config OPTION_EGLIBC_CRYPT_UFC
+   bool "Ultra fast `crypt' implementation"
+   depends OPTION_EGLIBC_CRYPT
+   help
+       This option group provides ultra fast DES-based implementation of
+       the `crypt' function.  When this option group is disabled,
+       (a) the library will not provide the setkey[_r] and encrypt[_r]
+       functions and (b) the crypt[_r] function will return NULL and set the
+       errno to ENOSYS if /salt/ passed does not correspond to either MD5,
+       SHA-256 or SHA-512 algorithm.
+
 config OPTION_EGLIBC_DB_ALIASES
    bool "Functions for accessing the mail aliases database"
    help
diff --git a/option-groups.defaults b/option-groups.defaults
index 4653e64..5d6be82 100644
--- a/option-groups.defaults
+++ b/option-groups.defaults
@@ -15,6 +15,8 @@ OPTION_EGLIBC_BSD = y
 OPTION_EGLIBC_CXX_TESTS = y
 OPTION_EGLIBC_CATGETS = y
 OPTION_EGLIBC_CHARSETS = y
+OPTION_EGLIBC_CRYPT = y
+OPTION_EGLIBC_CRYPT_UFC = y
 OPTION_EGLIBC_DB_ALIASES = y
 OPTION_EGLIBC_ENVZ = y
 OPTION_EGLIBC_FCVT = y