Salt string compatibility with generic implementations.

All other existing implementations of the md5crypt, sha256crypt,
and sha512crypt hashing methods allow any ASCII character to be
present in the salt string.

We changed our implementation of these hash methods to be compliant
with the general behaviour, except for the colon ':' character,
which serves a special purpose in the Unix shadow file.

Also adapt test/badsalt to expect this difference in behaviour from
the other hashing methods available in libxcrypt.

Fixes #105

lib/crypt-md5.c

@@ -2,6 +2,7 @@
    Compatible with the behavior of MD5 crypt introduced in FreeBSD 2.0.
 
    Copyright (C) 1996-2017 Free Software Foundation, Inc.
+   Modified by Björn Esser <besser82 at fedoraproject.org> in 2020.
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public License
@@ -73,6 +74,7 @@ crypt_md5crypt_rn (const char *phrase, size_t phr_size,
   uint8_t *result = buf->result;
   char *cp = (char *)output;
   const char *salt = setting;
+  const char *badsalt;
 
   size_t salt_size;
   size_t cnt;
@@ -83,12 +85,18 @@ crypt_md5crypt_rn (const char *phrase, size_t phr_size,
     /* Skip salt prefix.  */
     salt += sizeof (md5_salt_prefix) - 1;
 
-  salt_size = strspn (salt, b64t);
-  if (salt[salt_size] && salt[salt_size] != '$')
+  /* Find the next '$', or string-terminator if not present. */
+  salt_size = strcspn (salt, "$");
+
+  /* Check if there is no seperator for columns ':' in the salt. */
+  badsalt = strpbrk (setting, ":");
+  if (badsalt && badsalt < salt + salt_size)
     {
       errno = EINVAL;
       return;
     }
+
+  /* Ensure we do not use more salt than SALT_LEN_MAX. */
   if (salt_size > SALT_LEN_MAX)
     salt_size = SALT_LEN_MAX;
 

lib/crypt-sha256.c

@@ -3,6 +3,7 @@
  * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1].
  * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018.
  * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018.
+ * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020.
  * To the extent possible under law, the named authors have waived all
  * copyright and related or neighboring rights to this work.
  *
@@ -100,6 +101,7 @@ crypt_sha256crypt_rn (const char *phrase, size_t phr_size,
   uint8_t *s_bytes = buf->s_bytes;
   char *cp = (char *)output;
   const char *salt = setting;
+  const char *badsalt;
 
   size_t salt_size;
   size_t cnt;
@@ -140,12 +142,18 @@ crypt_sha256crypt_rn (const char *phrase, size_t phr_size,
       rounds_custom = true;
     }
 
-  salt_size = strspn (salt, b64t);
-  if (salt[salt_size] && salt[salt_size] != '$')
+  /* Find the next '$', or string-terminator if not present. */
+  salt_size = strcspn (salt, "$");
+
+  /* Check if there is no seperator for columns ':' in the salt. */
+  badsalt = strpbrk (setting, ":");
+  if (badsalt && badsalt < salt + salt_size)
     {
       errno = EINVAL;
       return;
     }
+
+  /* Ensure we do not use more salt than SALT_LEN_MAX. */
   if (salt_size > SALT_LEN_MAX)
     salt_size = SALT_LEN_MAX;
 

lib/crypt-sha512.c

@@ -3,6 +3,7 @@
  * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1].
  * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018.
  * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018.
+ * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020.
  * To the extent possible under law, the named authors have waived all
  * copyright and related or neighboring rights to this work.
  *
@@ -101,6 +102,7 @@ crypt_sha512crypt_rn (const char *phrase, size_t phr_size,
   uint8_t *s_bytes = buf->s_bytes;
   char *cp = (char *)output;
   const char *salt = setting;
+  const char *badsalt;
 
   size_t salt_size;
   size_t cnt;
@@ -141,15 +143,20 @@ crypt_sha512crypt_rn (const char *phrase, size_t phr_size,
       rounds_custom = true;
     }
 
-  salt_size = strspn (salt, b64t);
-  if (salt[salt_size] && salt[salt_size] != '$')
+  /* Find the next '$', or string-terminator if not present. */
+  salt_size = strcspn (salt, "$");
+
+  /* Check if there is no seperator for columns ':' in the salt. */
+  badsalt = strpbrk (setting, ":");
+  if (badsalt && badsalt < salt + salt_size)
     {
       errno = EINVAL;
       return;
     }
+
+  /* Ensure we do not use more salt than SALT_LEN_MAX. */
   if (salt_size > SALT_LEN_MAX)
     salt_size = SALT_LEN_MAX;
-  phr_size = strlen (phrase);
 
   /* Compute alternate SHA512 sum with input PHRASE, SALT, and PHRASE.  The
      final result will be added to the first context.  */

test/badsalt.c

@@ -252,7 +252,8 @@ check_results (const char *label, const char *fn,
     {
       if (retval[0] == '*' ||
           strncmp (retval, setting,
-                   (setting[l_setting - 1] == '*') ? l_setting - 1 : l_setting))
+                   (setting[l_setting - 1] == '*' ||
+                    setting[l_setting - 1] == ':') ? l_setting - 1 : l_setting))
         {
           printf ("FAIL: %s/%s/%s: expected success, got non-matching %s\n",
                   label, setting, fn, retval);
@@ -433,6 +434,21 @@ test_one_case (const struct testcase *t,
       p = page + pagesize - (t->plen + 2);
       memcpy (p, goodhash, t->plen);
       p[t->plen] = '*';
+
+      const char *md5_label           = "MD5 (FreeBSD)";
+      const char *sha256_plain_label  = "SHA-256 (plain)";
+      const char *sha512_plain_label  = "SHA-512 (plain)";
+      const char *sha256_rounds_label = "SHA-256 (rounds)";
+      const char *sha512_rounds_label = "SHA-512 (rounds)";
+
+      /* The asterisk character is a valid salt character for these hashes.  */
+      if (!strncmp (t->label, md5_label, strlen (md5_label))                      ||
+          !strncmp (t->label, sha256_plain_label, strlen (sha256_plain_label))    ||
+          !strncmp (t->label, sha512_plain_label, strlen (sha512_plain_label))    ||
+          !strncmp (t->label, sha256_rounds_label, strlen (sha256_rounds_label))  ||
+          !strncmp (t->label, sha512_rounds_label, strlen (sha512_rounds_label)))
+        p[t->plen] = ':';
+
       p[t->plen+1] = '\0';
       if (!test_one_setting (t->label, p, cd, true))
         return false;