l2ping(8) and -f switch

[Maksim Yevmenkin]

On Mon, Mar 28, 2011 at 3:52 PM, Alexander Best <arundel at freebsd.org> wrote:
> On Mon Mar 28 11, Maksim Yevmenkin wrote:
>> On Mon, Mar 28, 2011 at 2:55 PM, Alexander Best <arundel at freebsd.org> wrote:
>> > On Mon Mar 28 11, Maksim Yevmenkin wrote:
>> >> On Mon, Mar 28, 2011 at 2:37 PM, Alexander Best <arundel at freebsd.org> wrote:
>> >> > On Mon Mar 28 11, Alexander Best wrote:
>> >> >> On Mon Mar 28 11, Maksim Yevmenkin wrote:
>> >> >> > On Mon, Mar 28, 2011 at 12:59 PM, Alexander Best <arundel at freebsd.org> wrote:
>> >> >> > > On Mon Mar 28 11, Maksim Yevmenkin wrote:
>> >> >> > >> On Mon, Mar 28, 2011 at 7:04 AM, Iain Hibbert <plunky at rya-online.net> wrote:
>> >> >> > >> > On Mon, 28 Mar 2011, Alexander Best wrote:
>> >> >> > >> >
>> >> >> > >> >> On Mon Mar 28 11, Iain Hibbert wrote:
>> >> >> > >> >> > On Mon, 28 Mar 2011, Alexander Best wrote:
>> >> >> > >> >> >
>> >> >> > >> >> > > thus i believe making the -f switch only accessable to super-users (in
>> >> >> > >> >> > > accordance with ping(8)/ping6(8)) would increase security.
>> >> >> > >> >> >
>> >> >> > >> >> > what stops the user from recompiling l2ping without this restriction?
>> >> >> > >> >>
>> >> >> > >> >> nothing. but what stops him from recompiling ping(8) or ping6(8) without the
>> >> >> > >> >> restriction? still it's there.
>> >> >> > >> >
>> >> >> > >> > AFAIK you need superuser privileges to even send ICMP_ECHO packets, thats
>> >> >> > >> > why ping is traditionally a suid program and making a new binary won't
>> >> >> > >> > help normal users..  I'm guessing that l2ping doesn't have the same
>> >> >> > >> > restrictions?
>> >> >> > >>
>> >> >> > >> Guys,
>> >> >> > >>
>> >> >> > >> first of all thanks for the patch.
>> >> >> > >>
>> >> >> > >> i think one really needs to understand what "flood" really means in
>> >> >> > >> l2ping(8). "flood" ping(8) basically floods the link with icmp echo
>> >> >> > >> requests without waiting for remote system to reply. yes, this is
>> >> >> > >> potentially dangerous and thus its reasonable to require super-user
>> >> >> > >> privileges. "flood" l2ping(8) is NOT the same. all l2ping(8) does is
>> >> >> > >> "flood" mode
>> >> >> > >>
>> >> >> > >> 1) sends l2cap echo request
>> >> >> > >> 2) waits for l2cap echo response (or timeout)
>> >> >> > >> 3) repeats
>> >> >> > >>
>> >> >> > >> in other words, there is no delay between each l2cap echo
>> >> >> > >> request-response transaction. its not really "flood". i'm not sure if
>> >> >> > >> it really worth to go all the way to restricting this. however, if
>> >> >> > >> people think that it should be restricted, i will not object.
>> >> >> > >
>> >> >> > > how about removing the term "flood" from the l2ping(2) man page, if the -f
>> >> >> > > semantics can't actually be called that way?
>> >> >> >
>> >> >> > that would be fine. l2ping(8) -h calls it
>> >> >> >
>> >> >> > -f         No delay (sort of flood)
>> >> >> >
>> >> >> > and l2ping(8) man page calls it
>> >> >> >
>> >> >> > -f      ``Flood'' ping, i.e., no delay between packets.
>> >> >> >
>> >> >> > it would be nice to make those consistent :) i'm not sure what the
>> >> >> > best name would be though.
>> >> >>
>> >> >> another possibility would be to allow l2ping -i 0 and say that the -f flag is
>> >> >> an alias for that.
>> >>
>> >> the existing code provides exactly this behavior. perhaps just a man
>> >> page and usage() change?
>> >
>> > hmmm...no actually. l2ping -i 0 is not a valid parameter, since -i has to be
>> > greater than 0. so it's not possible to simply say "-f is an alias for -i 0",
>> > because that implies that -i 0 should work (which it doesn't).
>>
>> well, don't call it an "alias" then :) call it "effectively -i 0", "no
>> delay" or something like that :)
>>
>> >> > the following patch will implement this behavior.
>> >>
>> >> if we are going to go this route then why not just get rid of the
>> >> "flood" variable all together? just set wait to 0 (zero) if -f was
>> >> specified. also, we should probably use strtol(3) instead of atoi(3).
>> >
>> > i've thought of that. however that would mean l2ping -f -i 3 would set the
>> > delay to 3 seconds and usually an -f switch implies "force". so i think the
>> > current behavior of -f having a higher priority than any -i X option should be
>> > kept.
>>
>> i think that this is not worthy of long discussion :) i agree that
>> word 'flood' is not appropriate and/or confusing. all the patches
>> provided were fine, imo. please make a decision and commit the best
>> (in your opinion) fix.
>
> sorry, but i don't own a src commit bit. however i think the following patch
> should be fine. i've also eliminated a few var = NULL, since they're all being
> initialised properly and unconditionally at some point. also the defenitions
> didn't apply to style(9). plus i've converted all the atoi() calls to strtol()
> calls.

committed, thanks!

max