Hi,
On Sun, Aug 24, 2008 at 03:34:50PM -0700, [EMAIL PROTECTED] 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 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.
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.
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 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.
[...]
> +#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?
> + }
> + 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)
[...]
> + 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.
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.
> 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.
--
<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
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general
