Bug 32552

Summary: broken configuration option in rc.conf
Product: Base System Reporter: Gleb Smirnoff <glebius>
Component: confAssignee: Crist J. Clark <cjc>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Unspecified   
Hardware: Any   
OS: Any   

Description Gleb Smirnoff 2001-12-06 06:00:01 UTC
	In /etc/defaults/rc.conf exists option
	pppoed_provider="*". If it is not overriden in /etc/rc.conf,
	it is expanded to the listing of /. So pppoed is invoked
	with option -p COPYRIGHT bin boot ....

Fix: 

Comment out this line or override it.
How-To-Repeat: 	Try pppoed_enable="YES" in rc.conf. pppoed will not run on boot.
	If we insert echo in rc.network before /usr/libexec/pppoed,
	then we will see how pppoed_provider is misinterpreted.
Comment 1 Crist J. Clark freebsd_committer freebsd_triage 2001-12-06 07:14:45 UTC
On Thu, Dec 06, 2001 at 08:58:50AM +0300, Gleb Smirnoff wrote:
[snip]
> >Description:
> 	In /etc/defaults/rc.conf exists option
> 	pppoed_provider="*". If it is not overriden in /etc/rc.conf,
> 	it is expanded to the listing of /. So pppoed is invoked
> 	with option -p COPYRIGHT bin boot ....
> 
> >How-To-Repeat:
> 	Try pppoed_enable="YES" in rc.conf. pppoed will not run on boot.
> 	If we insert echo in rc.network before /usr/libexec/pppoed,
> 	then we will see how pppoed_provider is misinterpreted.
> 
> >Fix:
> 	Comment out this line or override it.

That _is_ the intended default, a '*' (see pppoed(8) why that is). The
problem is the code rc.network doesn't protect it from metacharacter
expansion.

The easiest way around this I could think of is,

Index: src/etc/rc.network
===================================================================
RCS file: /export/ncvs/src/etc/rc.network,v
retrieving revision 1.115
diff -u -r1.115 rc.network
--- src/etc/rc.network  24 Nov 2001 23:41:32 -0000      1.115
+++ src/etc/rc.network  6 Dec 2001 07:12:53 -0000
@@ -804,11 +804,13 @@
 
        case ${pppoed_enable} in
        [Yy][Ee][Ss])
+               echo -n ' pppoed';
                if [ -n "${pppoed_provider}" ]; then
-                       pppoed_flags="${pppoed_flags} -p ${pppoed_provider}"
+                       /usr/libexec/pppoed ${pppoed_flags} \
+                           -p "${pppoed_provider}" ${pppoed_interface}
+               else
+                       /usr/libexec/pppoed ${pppoed_flags} ${pppoed_interface}
                fi
-               echo -n ' pppoed';
-               /usr/libexec/pppoed ${pppoed_flags} ${pppoed_interface}
                ;;
        esac
 
Not the prettiest, but I think it works. Unless someone has a better
one, I'll commit it.
-- 
"It's always funny until someone gets hurt. Then it's hilarious."

Crist J. Clark                     |     cjclark@alum.mit.edu
                                   |     cjclark@jhu.edu
http://people.freebsd.org/~cjc/    |     cjc@freebsd.org
Comment 2 ru freebsd_committer freebsd_triage 2001-12-06 08:48:08 UTC
On Wed, Dec 05, 2001 at 11:20:02PM -0800, Crist J . Clark wrote:
> That _is_ the intended default, a '*' (see pppoed(8) why that is). The
> problem is the code rc.network doesn't protect it from metacharacter
> expansion.
> 
> The easiest way around this I could think of is,
> 
> Index: src/etc/rc.network
> ===================================================================
> RCS file: /export/ncvs/src/etc/rc.network,v
> retrieving revision 1.115
> diff -u -r1.115 rc.network
> --- src/etc/rc.network  24 Nov 2001 23:41:32 -0000      1.115
> +++ src/etc/rc.network  6 Dec 2001 07:12:53 -0000
> @@ -804,11 +804,13 @@
>  
>         case ${pppoed_enable} in
>         [Yy][Ee][Ss])
> +               echo -n ' pppoed';
>                 if [ -n "${pppoed_provider}" ]; then
> -                       pppoed_flags="${pppoed_flags} -p ${pppoed_provider}"
> +                       /usr/libexec/pppoed ${pppoed_flags} \
> +                           -p "${pppoed_provider}" ${pppoed_interface}
> +               else
> +                       /usr/libexec/pppoed ${pppoed_flags} ${pppoed_interface}
>                 fi
> -               echo -n ' pppoed';
> -               /usr/libexec/pppoed ${pppoed_flags} ${pppoed_interface}
>                 ;;
>         esac
>  
> Not the prettiest, but I think it works. Unless someone has a better
> one, I'll commit it.
> 
Index: rc.network
===================================================================
RCS file: /home/ncvs/src/etc/rc.network,v
retrieving revision 1.115
diff -u -p -r1.115 rc.network
--- rc.network	2001/11/24 23:41:32	1.115
+++ rc.network	2001/12/06 08:46:43
@@ -808,7 +808,9 @@ network_pass3() {
 			pppoed_flags="${pppoed_flags} -p ${pppoed_provider}"
 		fi
 		echo -n ' pppoed';
+		_opts=$-; set -f
 		/usr/libexec/pppoed ${pppoed_flags} ${pppoed_interface}
+		set +f; set -${_opts}
 		;;
 	esac


The ${_opts} is only needed for the case where -f was already set.
 

Cheers,
-- 
Ruslan Ermilov		Oracle Developer/DBA,
ru@sunbay.com		Sunbay Software AG,
ru@FreeBSD.org		FreeBSD committer,
+380.652.512.251	Simferopol, Ukraine

http://www.FreeBSD.org	The Power To Serve
http://www.oracle.com	Enabling The Information Age
Comment 3 Crist J. Clark freebsd_committer freebsd_triage 2001-12-06 09:42:23 UTC
State Changed
From-To: open->analyzed

ru's fix committed to CURRENT. Wait a day for MFC. 


Comment 4 Crist J. Clark freebsd_committer freebsd_triage 2001-12-06 09:42:23 UTC
Responsible Changed
From-To: freebsd-bugs->cjc

I'll handle the MFC.
Comment 5 Crist J. Clark freebsd_committer freebsd_triage 2001-12-07 08:32:57 UTC
State Changed
From-To: analyzed->closed

MFC.