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

Re: [Nbd] NBD passwords (was Re: nbd-server working easily in cygwin in XP)

" Hi,
" On Sun, Aug 24, 2008 at 03:34:50PM -0700, ulmo@...205... wrote:
" > Following the maintainer's outline of what to do, I made a patch to the
" > latest SVN, attached.  You can download it applied at:
" > 
" > ftp://ftp.armory.com/pub/user/ulmo/nbd/nbd-new.tar.gz
" Whoa. Thanks! I did intend to implement this at some later point,
" but you beat me to it. By far!

I was doing a security maintenance cycle on my system, and this was in
the list.

" I still have a few questions, though:
" > Note that since it is autoconf raw, you may need to grab a cooked
" > set of autoconf files (*.in, config.*, and some other junk).
" > 
" > You have to write the client and server passwords to nbd-client's
" > standard input, one line at a time.

At this time, since it's new and a bit messy code, someone could make
the new NBD_AUTH code autoconf-selectable (through ./configure).  I
don't know how.  I did design the NBD_AUTH #defines to work that way,

" I'm thinking it'd be nice if there'd be a command-line option to
" specify a file containing the passwords; otherwise, this would make
" it damn hard to script nbd-client.

I agree.  I was in a hurry.

I appended an incomplete todo list to this email, notes basically for
myself to read.

" Also: why two passwords? I must admit that I hadn't thought of
" authenticating the server, which obviously makes sense and is the
" right thing to do; but why can't we reuse the same password for both
" auth passes?

I'm not an expert at security, I just think about it in decent
measures.  To me it is obvious that the server needs to authenticate
to the client, because we don't want someone to pretend to be the
server.  Perhaps I've had too many people tell me they were police
when they were trying to mug me (they always failed, but that's not
the point).  I don't trust the "higher up" until they show me reason
to trust them (at which point I make a decision that based upon least
problems, most quality, and usually try to go with decent systems in
place).  This goes with servers as well.

Anyway, since I am new to writing password hashing code (this was my
first, and I just followed your procedure list rather than researching
it myself), I thought about the ordering of events that happens, and
don't know techniques that will work better.  Here are the issues:
when the client sends its hash back to the server with the password in
it, the server would then have a way to .. wait, I was worried that
when you send the password to the server, it would just accept it,
then send the password it just got from the client back to the client.
I guess the theory is that that is hard to do.  I was thinking the
hash was just a way to stop evesdroppers from listening in and
fabricating the passwords, but I suppose it also could be utilized to
prevent the peer (server in this case) from knowing what password was
in the hash without foreknowledge of it.  Still, with a sufficiently
low quality password, this would allow a rather simple brute-force
attack from the server to corrupt the client if they shared the same

Ok, I've just decided.  Is it correct that in this scenereo, I could
just re-use the password, and allow them to be equal?  If so, I'll
recode it to allow either one or two passwords.  If only one password
is supplied, it is used for both.  If two passwords are supplied, then
one is for server and one is for client.  I'll have to think of an easy
way to configure that without much fuss or muss.

" > I also made a small patch to that patch that uses urandom at the
" end of > random reads, so it goes faster, but is less secure.
" Mm. Perhaps this should be made a config option: allow the user to
" choose at runtime, rather than compile time.

I agree.

" [...]
" > +#ifdef NBD_AUTH
" > +	/* Prepare data for following section. */
" > +
" > +	memset(clipass,0,NBD_AUTHSZ);
" > +	memset(srvpass,0,NBD_AUTHSZ);
" > +	fprintf(stdout,"enter client password:");	/* stty -echo */
" Looks like you forgot to actually add that stty :-)
" > +	fgets(clipass, NBD_AUTHSZ, stdin);clipass[strlen(clipass)-1]='\0';
" > +
" > +	if (getpeername(sock, &peeraddr, &peeraddrlen) < 0) {
" > +		err("Negotiation failed 2: %m");
" > +		exit(62);				/* exit required to stop bugs */
" Which bugs would that be?

I've never seen the "err" code.  That means I ought to just write my
own equivilent function that does the same thing, perhaps just reusing
the err code, something like "security_err(...)" with that exit in it.
If the err code returns and there's no exit in there, then the
system's security gets bypassed.

" > +	}
" > +	if (getsockname(sock, &sockaddr, &sockaddrlen) < 0) {
" > +		err("Negotiation failed 3: %m");
" > +		exit(63);				/* exit required to stop bugs */
" > +	}
" > +
" > +	/* Get random from server, both apply server & client ips &
" > +	ports & families & client password, both digest, send back to
" > +	server for them to compare. */
" > +
" > +	if (read(sock, auth, (size_t)NBD_AUTHSZ) < 0) {
" > +		err("Negotiation failed 4: %m");
" > +		exit(64);				/* exit required to stop bugs */
" > +	}
" > +	SHA512_Init(&hashcontext);
" > +	SHA512_Update(&hashcontext, (unsigned char*)auth, NBD_AUTHSZ);
" > +	SHA512_Update(&hashcontext, (unsigned char*)&peeraddr, sizeof(peeraddr));
" > +	SHA512_Update(&hashcontext, (unsigned char*)&sockaddr, sizeof(sockaddr));
" > +	SHA512_Update(&hashcontext, (unsigned char*)clipass, strlen(clipass));
" > +	SHA512_Final(clidigest,&hashcontext);
" > +#ifdef DODBGBYPASS
" Looks like you forgot to rename something here, too (either that, or
" remove it)

It was the last bug I had that I fixed:  since I was in a hurry, my sloppy hack
for inputting passwords in nbd-client bit me:  fgets puts a '\n' at the end of
the strings :)  I was busy diagnosing it for like 2 hours.

" [...]
" > +		err("BAD SERVER PASSWORD!!!!!!!!");
" > +		exit(71);			/* exit required for bug security */
" > +	}
" > +#ifdef DODBG
" > +	fprintf(stderr,"Server password succeeded, and before that, client, too, most likely.\n");
" > +#endif /* DODBG */
" > +#else /* NBD_AUTH */
" >  	if (read(sock, buf, 8) < 0)
" >  		err("Failed/1: %m");
" >  	if (strlen(buf)==0)
" >  		err("Server closed connection");
" > -	if (strcmp(buf, INIT_PASSWD))
" > +	if (strncmp(buf, INIT_PASSWD,strlen(INIT_PASSWD)))
" Hmm, so you're replacing the INIT_PASSWD code with the SHA512
" password code you've implemented. I don't think that's a very good
" idea, since it changes the protocol incompatibly. I was actually
" thinking of not touching the INIT_PASSWD, but rather set an extra
" flag in the 'reserved' field, signalling that the client and the
" server understand authentication; they could then do the
" authentication itself immediately thereafter. This would also allow
" one compilation of a client and a server to serve/use both
" authenticated and non-authenticated exports.

Excellent point.  I will contingent it upon whether or not the
password(s) exist.  If there are passwords, then security is invoked.
I'll keep the protocol the same.

" The name INIT_PASSWD is a bit of a misnomer, anyway; it's not a
" password, it's a protocol banner, like '220 <hostname> ESMTP <software name>'
" for ESMTP, 'SSH-<protocol version>-<software name>' for SSH, etc.

I might end up renaming it a bit, to avoid confusion.

" >  		err("INIT_PASSWD bad");
" > +#endif /* NBD_AUTH */
" >  	printf(".");
" >  	if (read(sock, &magic, sizeof(magic)) < 0)
" >  		err("Failed/2: %m");
" > diff -x'*~' -Nrup nbd/nbd-server.c nbd-new/nbd-server.c
" > --- nbd/nbd-server.c	2008-08-24 09:42:13.000000000 -0700
" > +++ nbd-new/nbd-server.c	2008-08-24 14:42:50.000000000 -0700
" > @@ -173,6 +173,9 @@ typedef struct {
" >  	off_t expected_size; /**< size of the exported file as it was told to
" >  			       us through configuration */
" >  	gchar* listenaddr;   /**< The IP address we're listening on */
" > +#ifdef NBD_AUTH
" > +	char *srvpass,*clipass;
" > +#endif /* NBD_AUTH */
" >  	unsigned int port;   /**< port we're exporting this file at */
" >  	char* authname;      /**< filename of the authorization file */
" >  	int flags;           /**< flags associated with this exported file */
" > @@ -367,7 +370,7 @@ void dump_section(SERVER* serve, gchar* 
" >  		printf("\tcopyonwrite = true\n");
" >  	}
" >  	if(serve->expected_size) {
" > -		printf("\tfilesize = %Ld\n", (long long int)serve->expected_size);
" > +		printf("\tfilesize = %lld\n", (long long int)serve->expected_size);
" Ah, good catch :)
" That's about it. Thanks again for this patch; and sorry for being a bit
" picky about it.

Not at all.  It's a work in progress, but I know it's already useful,
and it would take us all a bit less time to have it progress in an
orderly manner rather with a bit of feedback (like that INIT_PASSWD
comment you pointed out) than I go make a big can of worms and submit
it like that.

Brad Allen

" -- 
" <Lo-lan-do> Home is where you have to wash the dishes.
"   -- #debian-devel, Freenode, 2004-09-22
" -------------------------------------------------------------------------
" This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
" Build the coolest Linux based applications with Moblin SDK & win great prizes
" Grand prize is a trip for two to an Open Source event anywhere in the world
" http://moblin-contest.org/redirect.php?banner_id=100&url=/
" _______________________________________________
" Nbd-general mailing list
" Nbd-general@lists.sourceforge.net
" https://lists.sourceforge.net/lists/listinfo/nbd-general

This is a limited todo list, with a few man page and google notes, for
me, not for you to bother with.  The annotations alone took me a few
hours to produce; my first implementation didn't need to be concerned
with that, but certainly fixing it up does.

- Better diagnostic output.  Mostly that it is taking a while to
  read from /dev/random.  Use stderr.
- Check portability of using /dev/random.  I could read other
  projects' code for this.
  Solaris 6 & 8 don't have it.
  FYI: SUN now provides /dev/random too:
    * Solaris 9 (SunOS 5.9):
      Included in the OS distribution (32Bit / 64Bit).
    * Solaris 8 (SunOS 5.8):
      Patch 112438 (32Bit / 64Bit).
    * Solaris 8_x86 (SunOS 5.8_x86):
      Patch 112439 (32Bit / 64Bit).
    * Solaris 7 / 7_x86 (SunOS 5.7 / 5.7_x86):
      Install "SUNWski" package, available at the "Solaris Easy Access CDs" in the "Sun Webserver" product.
    * Solaris 2.6 (SunOS 5.6):
      Install "SUNWski" package, available at the "Solaris Easy Access CDs" in the "Sun Webserver" product. 
  FreeBSD has /dev/random and /dev/urandom, meaning same/similar to Linux.
   Wikipedia claims no:  random is pseudo like urandom.
  NetBSD seems to have it.  see libgcrypt for a way to isolate.
  manual page <http://netbsd.gw.com/cgi-bin/man-cgi?rnd+4+NetBSD-current> says it has it.
  OpenBSD:  use /dev/srandom instead of /dev/random.  Configure??  Manual page:
  OS-X(Mac):  Beware of /dev/random on Mac OS X: msg#00079
   It's a /dev/urandom which has been labeled "/dev/random".  It claims
   Fri, 29 Aug 2003 18:51:54 +0000
   From <http://developer.apple.com/documentation/Darwin/Reference/ManPages/man4/random.4.html>
     The random device produces uniformly distributed random byte values of potentially high quality.
     To obtain random bytes, open /dev/random for reading and read from it.
     To add entropy to the random generation system, open /dev/random for writing and write data that you
     believe to be somehow random.
     /dev/urandom is a compatibility nod to Linux. On Linux, /dev/urandom will produce lower quality output
     if the entropy pool drains, while /dev/random will prefer to block and wait for additional entropy to
     be collected.  With Yarrow, this choice and distinction is not necessary, and the two devices behave
     identically. You may use either.
    Ok, so if we know a good source of random data on Apple, we could
    use that to write to it.  I have a better idea:  the run-time option
    to use "/dev/random" in OS-X will simply fail, citing that OS-X doesn't
    implement a different "/dev/random" than Linux.  In OS-X, it can default
    to using /dev/random anyway, since it's the same as /dev/urandom.
    That way if they fix it, it will be a bug in NBD but will still be secure.
  HPUX:  wikipedia mentioned it (see below).  Seems to have something:
   $ /usr/sbin/swlist -l fileset | grep -i random
   # RandomNumGen B.11.11.06 HP-UX 11.11 Strong Random Number Generator Product
     RandomNumGen.RNG-DKRN B.11.11.06 Strong Random Number Generator DLKM
     RandomNumGen.RNG-KRN B.11.11.06 Strong Random Number Generator Kernel Enablement 
   "As usual I received a great number of replies with links. It seems
   that this is an extra download from HP, I just do not remember
   downloading it for the development box. "
   (old - check wayback) <http://www.software.hp.com/cgi-bin/swdepot_parser.cgi/cgi/displayProductInfo.pl?productNumber=KRNG11I>
   (req login) <http://www2.itrc.hp.com/service/cki/docDisplay.do?docLocale=en_US&docId=200000072002648>
   <http://www.josvisser.nl/hpux11-random/hpux11-random.html> implements one for HP-UX 11.00
   Mon, 17 May 2004 12:17:09 -0400
   last link mentioned:  # time ./ssh-rand-helper
   perhaps openssh's ssh-rand-helper is of use.
  SGI IRIX:   As of February 5, 2003 IRIX 6.5.19 is releasing with all new MIPS
   systems shipping from SGI worldwide manufacturing centers.
    Pseudo Random Number Generator
     IRIX 6.5.19 provides a Pseudo Random Number Generator, accessible
     through the special files /dev/random, and /dev/urandom. These
     files support the standard file interface operations.
  Well, that sucks.  I'll make a "get random data" subroutine to handle
  all these little issues.
  Wikipedia says:
   /dev/random and /dev/urandom are also available on Solaris, Mac OS
   X, NetBSD, OpenBSD, Tru64 UNIX 5.1B, AIX 5.2, and HP-UX 11i v2.
   In Windows NT, similar functionality is delivered by ksecdd.sys,
   but reading the special file \Device\KsecDD does not work as in
   UNIX. The documented methods to generate cryptographically random
   bytes are CryptGenRandom and RtlGenRandom.
   I'll implement that for the cygwin version.  From wikipedia:
    CryptGenRandom is a crytographically secure pseudorandom number
    generator function that is included in Microsoft's Cryptographic
    Application Programming Interface. In Win32 programs, Microsoft
    recommends its use anywhere random number generation is needed. A
    2007 paper from Hebrew University suggested security problems in
    the Windows 2000 implementation of CryptGenRandom. Microsoft later
    acknowledged that the same problems exist in Windows XP, but not
    in Vista. Microsoft plans to incorporate a fix in Windows XP SP3
    in mid-2008.[1]
   I don't have time to deal with it, and will just use the existing.
   If cygwin implements its own, I'll xor them together, which is
   dumb since they're probably the same thing.
   The insecurities in the Microsoft random number generator mean
   that using two passwords for Microsoft NBD servers is far more
   important than Unix servers that can use EGD or have decent
   /dev/*random.  Perhaps an option to have server authenticate to
   client first would help for those who don't want to use two
   passwords, especially on NT and older XP (pre-SP3) systems.
   Using RtlGenRandom
    "Historically, we always told developers not to use functions such
    as rand to generate keys, nonces and passwords, rather they should
    use functions like CryptGenRandom, which creates cryptographically
    secure random numbers. The problem with CryptGenRandom is you need
    to pull in CryptoAPI (CryptAcquireContext and such) which is fine
    if you're using other crypto functions. On a default Windows XP
    and later install, CryptGenRandom calls into a function named
    ADVAPI32!RtlGenRandom, which does not require you load all the
    CryptAPI stuff. In fact, the new Whidbey CRT function, rand_s
    calls RtlGenRandom".[5]
   sigh .. ms always sucks so much.  THAT's why I went with 512 bit
   SHA keys rather than something lower.
  EGD:  for systems without good /dev/random.  I will make the
  use of egd work in the routine.  <http://egd.sourceforge.net/>
- Put in terminal echo code (off for passwords, then on again),
  when appropriate.
  linux termios ECHO
  #include <termios.h>
  #include <unistd.h>
       int tcgetattr(int fd, struct termios *termios_p);
       int tcsetattr(int fd, int optional_actions, struct termios *termios_p);
  Idea is to getattr, save that (make copy), setattr of no ECHO
  (~ECHO), then after done, setattr to original copy of original
  settings (restore to what it was).  If getting password, should do
  echo off as early as possible, so anybody pre-entering it can benefit.
  tcflag_t c_iflag;      /* input modes */tcflag_t c_oflag;      /* output modes */
  so, struct termios tios_dat tios_datmod;tcgetattr(inputfd,&tios_dat);memcpy(tios_datmod,tios_dat,sizeof(tios_datmod);
  ...read input...
  tcsetattr(inputfd,TCSANOW /* TCSADRAIN */,&tios_dat); /* Not sure TCSANOW vs. TCSADRAIN, since we already have it and it's input related too */
- Test #define NBD_AUTH on and off.
- Test NBD_AUTH in big-endian systems.  I did not put the configure
  hooks into the SHA512 code.
- Find out if you prefer non-BSD code.
- Audit the SHA512 code.  I grabbed it from the web, but it looked
  ok, and it created the same SHA512 output as "sha512sum" from Linux
  tools package, which was enough for me to do a proof of concept
  coding with it..  It also looked like really simple code, rather
  than a convoluted interface with big third party libraries.
- Test on cygwin.  I know, I ought to be the first to test it, but
  that'll be in the next few days, probably after I do some other
  work on it first, like some portability issues.
- Fix portability.  I like having FAMILY for security purposes,
  and IP and PORT are necessary, but put all that in portable format.
  Probably the *ntoa* type functions ought to make canonical output in
  strings for ip addresses with little difficulty in the code.
  Throughout NBD convert inet_ntoa to inet_ntop if there are any
  ntoa's left around:
       inet_ntop(3) extends the inet_ntoa(3) function to support
       multiple address families, inet_ntoa(3) is now considered to be
       deprecated in favor of inet_ntop(3).  The following address
       families are currently supported:
              src points to a struct in_addr (network byte order
              format) which is converted to an IPv4 network address in
              the quad format, "ddd.ddd.ddd.ddd".  The buffer dst must
              be at least INET_ADDRSTRLEN bytes long.
              src points to a struct in6_addr (network byte order
              format) which is converted to a representation of this
              address in the most appropriate IPv6 network address
              format for this address.  The buffer dst must be at
              least INET6_ADDRSTRLEN bytes long.
  Come to think of it, I want to use network byte order, and the
  information is already in network byte order in the structs.
  What I can do is just create my own family assignments that
  equal Linux's using a quick lookup table, and then for ip# and
  port# I can just use the existant values inside the structs.
  That sucks, and requires family-dependent code, but sigh it is
  nicer in output, because it avoids bugs like this:
       AF_INET6 converts mapped IPv4 addresses into an IPv6 format.IPv6
- Along with fix portability, do the same for IPv6.  Perhaps have
  to code the whole thing for IPv6 anyway.  See previous point.
- Alternate password file for config.  Perhaps use "authfile",
  the same one that has the ip#s in it.  That would require a bit
  of coding.  I'm not sure it's worth it.
- Config file for nbd-client, for obvious reasons.  A first pass
  could be REALLY simple, using a command line option to specify
  a file, and it just reads those passwords from the file.

end of message

Reply to: