Bug 132302

Summary: [patch] smbutil(1): contrib/smbfs subr.c: saved passwords >18 char fail
Product: Base System Reporter: David Horn <dhorn2000>
Component: binAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Only Me CC: emaste, gonzo, guru, jpaetzel, pi
Priority: Normal Keywords: patch
Version: Unspecified   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description David Horn 2009-03-04 09:10:01 UTC 
If a user has a saved password (encrypted using `smbutil crypt`)for smbfs that is longer than 18 characters, the smb_simplecrypt() and smb_simpledecrypt() functions will not work properly causing authentication failures.

e.g. 

/etc/nsmb.conf
[SERVER:USER]
password=$$178465324253e0c07f5f6fcc8d0a3b3bc8d8d131212


/usr/src/contrib/smbfs/lib/smb/subr.c

Fix: -Don't try to bitwise xor with >=255. 

This patch maintains backwards compatibility with people who already use nsmb.conf with shorter (<=17 char) passwords successfully.

Unified diff patch attached.  

Patch attached with submission follows:
How-To-Repeat: 1) Have a samba server
2) Have an account on samba server that has a long password (>18 characters)
3) Create your simple encrypted password using `smbutil crypt` for use in nsmb.conf
4) Save your password into ~/nsmb.conf or /etc/nsmb.conf under the appropriate [SERVER:USER] heading.
Comment 1 Matthias Apitz 2017-06-07 14:51:00 UTC 
I stumbled over the same old bug, debugged it and came up with this SVN diff:

$ svn diff src/contrib/smbfs/lib/smb/subr.c
Index: src/contrib/smbfs/lib/smb/subr.c
===================================================================
--- src/contrib/smbfs/lib/smb/subr.c	(revisión: 314251)
+++ src/contrib/smbfs/lib/smb/subr.c	(copia de trabajo)
@@ -232,6 +232,8 @@
 			  islower(ch) ? ('a' + (ch - 'a' + 13) % 26) : ch);
 		ch ^= pos;
 		pos += 13;
+		if (pos > 256)
+		    pos -= 256;
 		sprintf(dst, "%02x", ch);
 		dst += 2;
 	}
@@ -262,6 +264,8 @@
 			return EINVAL;
 		ch ^= pos;
 		pos += 13;
+		if (pos > 256)
+		    pos -= 256;
 		if (isascii(ch))
 		    ch = (isupper(ch) ? ('A' + (ch - 'A' + 13) % 26) :
 			  islower(ch) ? ('a' + (ch - 'a' + 13) % 26) : ch);

I wanted to file a PR, but found with search this older issue with nearly the same solution :-(

can someone please look into it, do a code rev and commit it; thanks
Comment 2 Kurt Jaeger freebsd_committer freebsd_triage 2017-06-07 17:38:00 UTC 
See

https://lists.freebsd.org/pipermail/freebsd-current/2017-June/066208.html

for a similar analysis, with a very similar result.
Comment 3 Josh Paetzel freebsd_committer freebsd_triage 2017-06-08 00:49:15 UTC 
Committed to HEAD.  We'll see if we can get this in to 11.1-RELEASE as well.
Comment 4 commit-hook freebsd_committer freebsd_triage 2017-06-08 00:49:27 UTC 
A commit references this bug:

Author: jpaetzel
Date: Thu Jun  8 00:48:26 UTC 2017
New revision: 319670
URL: https://svnweb.freebsd.org/changeset/base/319670

Log:
  Fix SMBFS when saved passwords are greater than 18 characters

  PR:	132302
  Submitted by:	dhorn2000@gmail.com guru@unixarea.de
  MFC after:	1 week

Changes:
  head/contrib/smbfs/lib/smb/subr.c
Comment 5 Matthias Apitz 2017-06-08 06:20:58 UTC 
(In reply to Josh Paetzel from comment #3)
I was thinking last night about the proposed patch and wanted to modify it, but now it's committed already before I could put hands on the keyboard;

the problem (bug) was that the value in 'pos' for the XOR operation:

	ch ^= pos;

at some point exceeds, when the to be crypted password is long enough, exceeds one byte, and so does the result in 'ch'; later the value from 'ch' is formated to a hex string with

       sprintf(dst, "%02x", ch);
       dst += 2;

which gives in 'dst' four new bytes, like 0136, but the pointer 'dst' is only moved two byte further; this damages the resulting string in 'dst' for the crypted pw and later decrypt goes wrong;

Said this, the test for 'pos' not exceeding one byte must be 

	if (pos >= 256) 
	    pos -=256;'

because a pw of 57 bytes lenth will hit the point where 'pos' is exactly 256 (0x100);

57++ bytes is a very unlikely password, but as it s stored into a file and not typed in from time to time it's not impossible thinking in such a beast;

sorry to come late with this comment;
Comment 6 Josh Paetzel freebsd_committer freebsd_triage 2017-06-08 16:27:05 UTC 
Please feel free to iterate over this.  The first commit stopped the bleeding.  There's no harm with fixing corner cases.
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-06-15 16:13:06 UTC 
A commit references this bug:

Author: jpaetzel
Date: Thu Jun 15 16:12:45 UTC 2017
New revision: 319980
URL: https://svnweb.freebsd.org/changeset/base/319980

Log:
  MFC 319670

  Fix SMBFS when saved passwords are greater than 18 character

  PR:	132302
  Submitted by:	dhorn2000@gmail.com guru@unixarea.de
  Approved by:	re (gjb)

Changes:
_U  stable/11/
  stable/11/contrib/smbfs/lib/smb/subr.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-06-15 23:14:39 UTC 
A commit references this bug:

Author: jpaetzel
Date: Thu Jun 15 23:14:05 UTC 2017
New revision: 319992
URL: https://svnweb.freebsd.org/changeset/base/319992

Log:
  MFC 319670

  Fix SMBFS when saved passwords are greater than 18 characters

  PR:	132302
  Submitted by:	dhorn2000@gmail.com guru@unixarea.de

Changes:
_U  stable/10/
  stable/10/contrib/smbfs/lib/smb/subr.c
Comment 9 Eitan Adler freebsd_committer freebsd_triage 2018-05-20 23:50:39 UTC 
For bugs matching the following conditions:
- Status == In Progress
- Assignee == "bugs@FreeBSD.org"
- Last Modified Year <= 2017

Do
- Set Status to "Open"
Comment 10 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-14 08:08:21 UTC 
The patch has been committed, closing as Fixed