>Number:         12045
>Category:       lib
>Synopsis:       skey generates incorrect passwords when using SHA-1
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 25 21:12:00 PST 2001
>Closed-Date:
>Last-Modified:
>Originator:     John Valdes
>Release:        NetBSD-1.5
>Organization:
	University of Chicago
>Environment:
	Mac PowerBook G3, NetBSD-1.5, macppc, libskey.so.1 & libc.so.12
System: NetBSD scallop 1.5 NetBSD 1.5 (SCALLOP) #1: Sun Jan 21 18:25:09 CST 2001 root@scallop:/usr/src/sys/arch/macppc/compile/SCALLOP macppc


>Description:
	One-time passwords generated with /usr/bin/skey when using sha1
	as the hash function are incorrect.  They do not match the test
	cases given in Appendix C of RFC2289.
>How-To-Repeat:
	The following illustrates the problem:

		prompt> skey -x -t sha1 0 TeSt
		Enter secret password: This is a test.
		E16A 9EBB F48F 9D97

	The correct response should be "BB9E 6AE1 979D 8FF4".  From this
	example, that the 2 four-byte ints making up this response are
	byte-swapped.

	Note that the PowerPC is big-endian; I believe the same problem
	exists on little-endian systems.  I haven't tested this directly
	(I don't have NetBSD installed on any little-endian systems), but
	on an i386 system running OpenBSD 2.6, which has almost identical
	source for the skey routines, I get the same, incorrect results
	with skey when using sha1.  However, please test this on NetBSD-1.5
	running on an i386 or other little-endian system.

>Fix:
	The fix is to swap the bytes returned by the SHA1 hash functions
	before returning results to skey.  This can be fixed in
	/usr/src/lib/libskey/skeysubr.c.  Here's a patch.  Note that if
	this bug isn't present in little-endian systems, then in the patch
	below, replace the calls to "bswap32()" with "htole32()":

*** skeysubr.c.dist	Mon Jul 17 14:55:53 2000
--- skeysubr.c	Thu Jan 25 16:40:48 2001
***************
*** 160,166 ****
  	SHA1_CTX sha;
  	u_int32_t results[5];
  	size_t buflen;
- 	int i, j;
  
  	if ((buf = mkSeedPassword(seed, passwd, &buflen)) == NULL)
  		return -1;
--- 160,165 ----
***************
*** 177,195 ****
  	results[0] ^= results[4];
  
  	/*
! 	 * Even though my readings of rfc2289 indicate that this is
! 	 * wrong (it converts stuff to big endian), it is needed to
! 	 * make the output match up the regression tests in said rfc.
! 	 * Something is wrong here? --mjl
  	 */
! 		
! 	for(i=j=0; j<8; i++, j+=4)
! 		{
! 		result[j+3] = (unsigned char)(results[i] & 0xff);
! 		result[j+2] = (unsigned char)((results[i] >> 8) & 0xff);
! 		result[j+1] = (unsigned char)((results[i] >> 16) & 0xff);
! 		result[j+0] = (unsigned char)((results[i] >> 24) & 0xff);
! 		}
  
  	return(0);
  }
--- 176,188 ----
  	results[0] ^= results[4];
  
  	/*
! 	 * RFC2289 indicates SHA1 results should be stored in little-endian
! 	 * order; we seem to need to swap byte regardless of host byte order
! 	 * (else we would use htole32()). -jpv
  	 */
! 	results[0] = bswap32(results[0]);
! 	results[1] = bswap32(results[1]);
! 	(void)memcpy((void *)result, (void *)results, SKEY_BINKEY_SIZE);
  
  	return(0);
  }
***************
*** 266,272 ****
  {
  	SHA1_CTX sha;
  	u_int32_t results[5];
- 	int i, j;
  	
  	SHA1Init(&sha);
  	SHA1Update(&sha, (unsigned char *)x, SKEY_BINKEY_SIZE);
--- 259,264 ----
***************
*** 277,289 ****
  	results[1] ^= results[3];
  	results[0] ^= results[4];
  
! 	for(i=j=0; j<8; i++, j+=4)
! 		{
! 		x[j+3] = (unsigned char)(results[i] & 0xff);
! 		x[j+2] = (unsigned char)((results[i] >> 8) & 0xff);
! 		x[j+1] = (unsigned char)((results[i] >> 16) & 0xff);
! 		x[j+0] = (unsigned char)((results[i] >> 24) & 0xff);
! 		}
  }
  
  #if 0
--- 269,278 ----
  	results[1] ^= results[3];
  	results[0] ^= results[4];
  
! 	/* Swap bytes */
! 	results[0] = bswap32(results[0]);
! 	results[1] = bswap32(results[1]);
! 	(void)memcpy((void *)x, (void *)results, SKEY_BINKEY_SIZE);
  }
  
  #if 0
>Release-Note:
>Audit-Trail:
>Unformatted: