NetBSD Problem Report #22271

Received: (qmail 23471 invoked by uid 605); 25 Jul 2003 22:13:21 -0000
Message-Id: <20030725221320.6A2D011152@narn.netbsd.org>
Date: Fri, 25 Jul 2003 22:13:20 +0000 (UTC)
From: cagney@mac.com
Sender: gnats-bugs-owner@NetBSD.org
Reply-To: cagney@mac.com
To: gnats-bugs@gnats.NetBSD.org
Subject: /var/db/dhclient.leases world readable
X-Send-Pr-Version: www-1.0

>Number:         22271
>Category:       security
>Synopsis:       /var/db/dhclient.leases world readable
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    mellon
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jul 25 22:14:00 +0000 2003
>Closed-Date:    Sat Sep 23 04:47:57 +0000 2006
>Last-Modified:  Sat Sep 23 04:47:57 +0000 2006
>Originator:     Andrew Cagney
>Release:        1.6.1
>Organization:
>Environment:
NetBSD localhost 1.6.1_STABLE NetBSD 1.6.1_STABLE (NETLUX) #10: Tue Jun 24 23:24:56 EDT 2003     cagney@localhost:/home/NetBSD/netbsd-1-6/src/sys/arch/macppc/compile/NETLUX macppc

>Description:
/etc/dhclient.conf can be configured to contain, and iterate through, a number of wireless access point keys vis:

interface "wi0" {
        #medium "-powersave"
        media "nwid '' -nwkey", "nwid aaa nwkey 0x1234"
}

When dhclient succeeds in binding to a base station, the relevant "media" option ends up being written to /var/db/dhclient.leases which is world readable.  Vis:

lease {
  interface "wi0";
  fixed-address 192.168.1.100;
  medium "nwid aaa nwkey 0x1234";
  ...

Since the file is world readable, normal users can see the nwkey that ifconfig carefully refused to display :-(
>How-To-Repeat:
Bind to a wireless basestation.  Cat /var/db/dhclient.leases.

>Fix:

>Release-Note:
>Audit-Trail:

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@netbsd.org
Cc: cagney@mac.com, mellon@netbsd.org
Subject: re: security/22271
Date: Fri, 02 Apr 2004 21:27:34 +1000

 this patch makes the db file mode 600 safely.  unfortunately has to
 use open(2) directly for this...

 ted - this OK to commit?


 .mrg.


 Index: dhclient.c
 ===================================================================
 RCS file: /cvsroot/src/dist/dhcp/client/dhclient.c,v
 retrieving revision 1.10
 diff -p -r1.10 dhclient.c
 *** dhclient.c	30 Mar 2004 19:40:58 -0000	1.10
 --- dhclient.c	2 Apr 2004 11:25:11 -0000
 *************** void rewrite_client_leases ()
 *** 2129,2139 ****
   	struct interface_info *ip;
   	struct client_state *client;
   	struct client_lease *lp;

   	if (leaseFile)
   		fclose (leaseFile);
 ! 	leaseFile = fopen (path_dhclient_db, "w");
 ! 	if (!leaseFile) {
   		log_error ("can't create %s: %m", path_dhclient_db);
   		return;
   	}
 --- 2129,2142 ----
   	struct interface_info *ip;
   	struct client_state *client;
   	struct client_lease *lp;
 + 	int fd;

   	if (leaseFile)
   		fclose (leaseFile);
 ! 	fd = open (path_dhclient_db, O_WRONLY|O_CREAT, 0600);
 ! 	if (fd != -1)
 ! 		leaseFile = fdopen (fd, "w");
 ! 	if (fd == -1 || !leaseFile) {
   		log_error ("can't create %s: %m", path_dhclient_db);
   		return;
   	}
 *************** int write_client_lease (client, lease, r
 *** 2223,2230 ****
   		return 1;

   	if (!leaseFile) {	/* XXX */
 ! 		leaseFile = fopen (path_dhclient_db, "w");
 ! 		if (!leaseFile) {
   			log_error ("can't create %s: %m", path_dhclient_db);
   			return 0;
   		}
 --- 2226,2237 ----
   		return 1;

   	if (!leaseFile) {	/* XXX */
 ! 		int fd;
 ! 
 ! 		fd = open (path_dhclient_db, O_WRONLY|O_CREAT, 0600);
 ! 		if (fd != -1)
 ! 			leaseFile = fdopen (fd, "w");
 ! 		if (fd == -1 || !leaseFile) {
   			log_error ("can't create %s: %m", path_dhclient_db);
   			return 0;
   		}

From: Ted Lemon <mellon@fugue.com>
To: matthew green <mrg@eterna.com.au>
Cc: mellon@netbsd.org, gnats-bugs@netbsd.org, cagney@mac.com
Subject: Re: security/22271
Date: Fri, 2 Apr 2004 10:55:05 -0600

 Yeah, I guess it's okay to commit, but this is not a pretty solution to 
 the problem.   You should probably commit this now so that the problem 
 is bandaged, and then assign the PR to me so that I can think about a 
 better solution to the problem.    I don't like making the lease file 
 private, since virtually everything in there is useful information that 
 went over the wire in the clear.

 :'}


From: matthew green <mrg@eterna.com.au>
To: Ted Lemon <mellon@fugue.com>
Cc: mellon@netbsd.org, gnats-bugs@netbsd.org, cagney@mac.com
Subject: re: security/22271 
Date: Sat, 03 Apr 2004 03:01:14 +1000

    Yeah, I guess it's okay to commit, but this is not a pretty solution to 
    the problem.   You should probably commit this now so that the problem 
    is bandaged, and then assign the PR to me so that I can think about a 
    better solution to the problem.    I don't like making the lease file 
    private, since virtually everything in there is useful information that 
    went over the wire in the clear.


 don't write out "media" options to the config file?  that looks like
 a simple change...will it break anything?

From: matthew green <mrg@eterna.com.au>
To: Ted Lemon <mellon@fugue.com>
Cc: mellon@netbsd.org, gnats-bugs@netbsd.org, cagney@mac.com
Subject: re: security/22271 
Date: Sat, 03 Apr 2004 03:05:02 +1000

 eg, this.


 Index: dhclient.c
 ===================================================================
 RCS file: /cvsroot/src/dist/dhcp/client/dhclient.c,v
 retrieving revision 1.10
 diff -p -r1.10 dhclient.c
 *** dhclient.c	30 Mar 2004 19:40:58 -0000	1.10
 --- dhclient.c	2 Apr 2004 17:01:59 -0000
 *************** int write_client_lease (client, lease, r
 *** 2283,2288 ****
 --- 2290,2296 ----
   		} else
   			++errors;
   	}
 + #if 0
   	if (lease -> medium) {
   		s = quotify_string (lease -> medium -> string, MDL);
   		if (s) {
 *************** int write_client_lease (client, lease, r
 *** 2295,2300 ****
 --- 2303,2309 ----
   		} else
   			errors++;
   	}
 + #endif
   	if (errno != 0) {
   		errors++;
   		errno = 0;

From: Ted Lemon <mellon@fugue.com>
To: matthew green <mrg@eterna.com.au>
Cc: mellon@netbsd.org, gnats-bugs@netbsd.org, cagney@mac.com
Subject: Re: security/22271 
Date: Fri, 2 Apr 2004 11:28:24 -0600

 On Apr 2, 2004, at 11:01 AM, matthew green wrote:
 > don't write out "media" options to the config file?  that looks like
 > a simple change...will it break anything?

 As it stands, I think not writing out media options will in fact break 
 things.   What will happen is that if it picks the lease, it just won't 
 set any media options, and then the network won't work.   So let's just 
 go with your original fix for now.   The real fix will probably involve 
 a symbolic reference to a media option definition or something like 
 that, so that the mediaopt statement in the lease file contains a 
 reference to the information in the config file, rather than containing 
 the information itself.

Responsible-Changed-From-To: security-officer->mellon 
Responsible-Changed-By: mrg 
Responsible-Changed-When: Sun Apr 4 02:35:26 UTC 2004 
Responsible-Changed-Why:  
i commited my first patch, over to you ted... 

From: matthew green <mrg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:  
Subject: pr/22271 CVS commit: src/dist/dhcp/client
Date: Sun,  4 Apr 2004 02:35:21 +0000 (UTC)

 Module Name:	src
 Committed By:	mrg
 Date:		Sun Apr  4 02:35:21 UTC 2004

 Modified Files:
 	src/dist/dhcp/client: dhclient.c

 Log Message:
 create the dhclient.leases file mode 600 for now as it may contain
 "media" lines that contain wireless network keys, that eg, ifconfig(8)
 carefully hides.

 this is not a complete solution, but it's OK until one arrives.

 approved by mellon@.  fixes the problem described in PR#22271.


 To generate a diff of this commit:
 cvs rdiff -r1.11 -r1.12 src/dist/dhcp/client/dhclient.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: jmcneill@netbsd.org
State-Changed-When: Sat, 23 Sep 2006 04:47:57 +0000
State-Changed-Why:
/var/db/dhclient.leases is no longer world readable


>Unformatted:

Home	PR Database Search