[PATCH 0/2] Fix issue with newer glibc crypt() function

[PATCH 0/2] Fix issue with newer glibc crypt() function

[Saul Wold]

The 2.17 version of glibc's crypt() was changed to returning NULL if
it got an invalid seed, this broke tinylogin and shadow in different
ways.

Thanks to the Slackware team for "just in time patches", they found it 
first in shadow around March 28th!

I do not believe that these will introduce any security concerns, please 
review with a careful eye.

Sau!


Saul Wold (2):
  tinylogin: fix segfault from crypt()
  shadow: add patch to fix crypt: Invalid Argument

 .../tinylogin/tinylogin-1.4/glibc_crypt_fix.patch  | 23 ++++++++
 meta/recipes-core/tinylogin/tinylogin_1.4.bb       |  4 +-
 .../files/slackware_fix_for_glib-2.17_crypt.patch  | 63 ++++++++++++++++++++++
 meta/recipes-extended/shadow/shadow_4.1.4.3.bb     |  1 +
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch
 create mode 100644 meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch

-- 
1.8.0.2

[PATCH 1/2] tinylogin: fix segfault from crypt()

[Saul Wold]

In glibc 2.17, crypt() now expects 2 valid chars for the seed or
it will error out and return a NULL. The tinylogin code took the
result from crypt directly into a strcmp() which caused a segfault

Tinylogin has been deperacted, busybox now has login support, I will
investigate using busybox login support for 1.5.

[YOCTO #4097]

Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 .../tinylogin/tinylogin-1.4/glibc_crypt_fix.patch  | 23 ++++++++++++++++++++++
 meta/recipes-core/tinylogin/tinylogin_1.4.bb       |  4 +++-
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch

diff --git a/meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch b/meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch
new file mode 100644
index 0000000..0a24656
--- /dev/null
+++ b/meta/recipes-core/tinylogin/tinylogin-1.4/glibc_crypt_fix.patch
@@ -0,0 +1,23 @@
+
+staring from glibc 2.17 the crypt() function will error out and return
+NULL if the seed or "correct" is invalid. The failure case for this is
+an unknown user which tinylogin assigns '!' for the password. crypt() 
+now expects a minimum of 2 valid characters. If we get a NULL return
+value from the crypt, assume we fail and return 0.
+
+Upstream-Status: Inappropriate [tinylogin depercated]
+Signed-off-by: Saul Wold <sgw@linux.intel.com>
+
+Index: tinylogin-1.4/libbb/correct_password.c
+===================================================================
+--- tinylogin-1.4.orig/libbb/correct_password.c
++++ tinylogin-1.4/libbb/correct_password.c
+@@ -74,5 +74,8 @@ int correct_password ( const struct pass
+ 	}
+ 	encrypted = crypt ( unencrypted, correct );
+ 	memset ( unencrypted, 0, xstrlen ( unencrypted ));
++        if ( !encrypted )
++		return 0;
++
+ 	return ( strcmp ( encrypted, correct ) == 0 ) ? 1 : 0;
+ }
diff --git a/meta/recipes-core/tinylogin/tinylogin_1.4.bb b/meta/recipes-core/tinylogin/tinylogin_1.4.bb
index aaed92a..6e08011 100644
--- a/meta/recipes-core/tinylogin/tinylogin_1.4.bb
+++ b/meta/recipes-core/tinylogin/tinylogin_1.4.bb
@@ -18,7 +18,9 @@ SRC_URI = "http://www.angstrom-distribution.org/unstable/sources/tinylogin-${PV}
 	file://remove-index.patch \
 	file://use_O2_option.patch \
 	file://passwd_rotate_check.patch \
-	file://avoid_static.patch"
+	file://avoid_static.patch \
+        file://glibc_crypt_fix.patch \
+        "
 
 SRC_URI[md5sum] = "44da0ff2b727455669890b24305e351d"
 SRC_URI[sha256sum] = "5e542e4b7825305a3678bf73136c392feb0d44b8bbf926e8eda5453eea7ddd6b"
-- 
1.8.0.2

[PATCH 2/2] shadow: add patch to fix crypt: Invalid Argument

[Saul Wold]

This patch came from Slackware and address a change in crypt()'s handling
of an invalid seed, which in the past returned an encrypted string and now
returns a NULL.

[YOCTO #4097] related to tinylogin segfault

Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 .../files/slackware_fix_for_glib-2.17_crypt.patch  | 63 ++++++++++++++++++++++
 meta/recipes-extended/shadow/shadow_4.1.4.3.bb     |  1 +
 2 files changed, 64 insertions(+)
 create mode 100644 meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch

diff --git a/meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch b/meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch
new file mode 100644
index 0000000..7cd45af
--- /dev/null
+++ b/meta/recipes-extended/shadow/files/slackware_fix_for_glib-2.17_crypt.patch
@@ -0,0 +1,63 @@
+
+This patch is from Slackware, I tried to find the actual
+author to add that attribution.  The comment below is the
+best summary, I will not repeat it here.
+
+Upstream-Status: Backport from slackware
+
+Signed-off-by: Saul Wold <sgw@linux.intel.com>
+
+Index: shadow-4.1.4.3/lib/encrypt.c
+===================================================================
+--- shadow-4.1.4.3.orig/lib/encrypt.c
++++ shadow-4.1.4.3/lib/encrypt.c
+@@ -45,15 +45,40 @@ char *pw_encrypt (const char *clear, con
+ 	static char cipher[128];
+ 	char *cp;
+ 
+-	cp = crypt (clear, salt);
+-	if (!cp) {
+-		/*
+-		 * Single Unix Spec: crypt() may return a null pointer,
+-		 * and set errno to indicate an error.  The caller doesn't
+-		 * expect us to return NULL, so...
+-		 */
+-		perror ("crypt");
+-		exit (EXIT_FAILURE);
++ 	cp = crypt (clear, salt);
++ 	if (!cp) {
++ 		/*
++		 * In glibc-2.17 and newer, crypt() will return NULL if
++		 * it was called using an invalid salt format.  Previous
++		 * versions of glibc would go ahead and compute a DES hash
++		 * using the invalid salt.  The salt value in this case was
++		 * always '!'.  We might arrive at this place if either the
++		 * user does not exist, or if the hash in /etc/shadow doesn't
++		 * have the proper magic for one of the supported hash
++		 * formats (for example, if the account was locked using
++		 * "passwd -l".  To handle this situation, we will recompute
++		 * the hash using a hardcoded salt as was previously done
++		 * by glibc.  The hash returned by the old glibc function
++		 * always began with "!!", which would ensure that it could
++		 * never match an otherwise valid hash in /etc/shadow that
++		 * was disabled with a "!" at the beginning (since the second
++		 * character would never be "!" as well), so we will also
++		 * prepend the resulting hash with "!!".  Finally, in case
++		 * crypt() failed for some other reason we will check to see
++		 * if we still get NULL from crypt even with the valid salt
++		 * and will fail if that's the case.
++ 		 */
++
++		/* Recalculate hash using a hardcoded, valid SHA512 salt: */
++		cp = crypt (clear, "$6$8IIcy/1EPOk/");
++
++		if (!cp) {
++			perror ("crypt");
++			exit (EXIT_FAILURE);
++		} else {
++			sprintf (cipher, "!!%s", cp);
++			return cipher;
++		}
+ 	}
+ 
+ 	/* The GNU crypt does not return NULL if the algorithm is not
diff --git a/meta/recipes-extended/shadow/shadow_4.1.4.3.bb b/meta/recipes-extended/shadow/shadow_4.1.4.3.bb
index 8226a6e..caf24c0 100644
--- a/meta/recipes-extended/shadow/shadow_4.1.4.3.bb
+++ b/meta/recipes-extended/shadow/shadow_4.1.4.3.bb
@@ -23,6 +23,7 @@ SRC_URI = "http://pkg-shadow.alioth.debian.org/releases/${BPN}-${PV}.tar.bz2 \
            file://shadow-4.1.4.2-su_no_sanitize_env.patch \
            file://shadow-update-pam-conf.patch \
            file://shadow_fix_for_automake-1.12.patch \
+           file://slackware_fix_for_glib-2.17_crypt.patch \
            "
 
 SRC_URI[md5sum] = "b8608d8294ac88974f27b20f991c0e79"
-- 
1.8.0.2