less(1): `!' command

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

less(1): `!' command

kshe-2
Hi,

Would a patch to bring back the `!' command to less(1) be accepted?  The
commit message for its removal explains that ^Z should be used instead,
but that obviously does not work if less(1) is run from something else
than an interactive shell, for example when reading manual pages from a
vi(1) instance spawned directly by `xterm -e vi' in a window manager or
by `neww vi' in a tmux(1) session.

If not, then at least documentation for this command should be removed
properly (I cannot provide a diff as this file contains raw backspace
characters):

        $ cd /usr/src/usr.bin/less/
        $ printf '99d\nwq\n' | ed - less.hlp

Regards,

kshe

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Jiri B-2
On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> Hi,
>
> Would a patch to bring back the `!' command to less(1) be accepted?  The
> commit message for its removal explains that ^Z should be used instead,
> but that obviously does not work if less(1) is run from something else
> than an interactive shell, for example when reading manual pages from a
> vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> by `neww vi' in a tmux(1) session.

Why should less be able to spawn another programs? This would undermine
all pledge work.

IIUC your vi scenario, you are not spawing 'vi' from less but the opposite
way. That should work.

j.

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

kshe-2
On Sat, 16 Dec 2017 18:13:16 +0000, Jiri B wrote:

> On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> > Hi,
> >
> > Would a patch to bring back the `!' command to less(1) be accepted?  The
> > commit message for its removal explains that ^Z should be used instead,
> > but that obviously does not work if less(1) is run from something else
> > than an interactive shell, for example when reading manual pages from a
> > vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> > by `neww vi' in a tmux(1) session.
>
> Why should less be able to spawn another programs? This would undermine
> all pledge work.

Because of at least `v' and `|', less(1) already is able to invoke
arbitrary programs, and accordingly needs the "proc exec" promise, so
bringing `!' back would not change anything from a security perspective
(otherwise, I would obviously not have made such a proposition).

In fact, technically, what I want to do is still currently possible:
from any less(1) instance, one may use `v' to invoke vi(1), and then use
vi(1)'s own `!' command as desired.  So the functionality of `!' is
still there; it was only made more difficult to reach for no apparent
reason.

Regards,

kshe

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Theo de Raadt-2
> On Sat, 16 Dec 2017 18:13:16 +0000, Jiri B wrote:
> > On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> > > Hi,
> > >
> > > Would a patch to bring back the `!' command to less(1) be accepted?  The
> > > commit message for its removal explains that ^Z should be used instead,
> > > but that obviously does not work if less(1) is run from something else
> > > than an interactive shell, for example when reading manual pages from a
> > > vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> > > by `neww vi' in a tmux(1) session.
> >
> > Why should less be able to spawn another programs? This would undermine
> > all pledge work.
>
> Because of at least `v' and `|', less(1) already is able to invoke
> arbitrary programs, and accordingly needs the "proc exec" promise, so
> bringing `!' back would not change anything from a security perspective
> (otherwise, I would obviously not have made such a proposition).
>
> In fact, technically, what I want to do is still currently possible:
> from any less(1) instance, one may use `v' to invoke vi(1), and then use
> vi(1)'s own `!' command as desired.  So the functionality of `!' is
> still there; it was only made more difficult to reach for no apparent
> reason.

No apparent reason?

Good you have an opinion.  I have a different opinion: We should look
for rarely used functionality and gut it.  Over the last 40 years
people have felt a desire to add all possible features and options to
all commands, and noone ever considered the impact of having all
programs above to reach all system calls, and that these features are
being installed in all program operating environents.  Then someone
adds less(1) to a script which requires security, and just like that
it has none.

The entire environment is poisoned, and people are pushed to jump to
other environments which aren't poisoned in this way, until enough
people arrive there, the feature explosion happens there also
resulting in "reach all the system calls", and we're stuck in the same
rut again.

I don't think all programs should be able to run all other programs.

As a result I support the idea of trying to find the things people
don't actually use, and removing them incrementally.  '|' should be on
the list next.

But you don't.  Luckily you have other choices.

Are you prepared to die on this hill that less must support '!'?  If
so, there's that FreeBSD hill over there..

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Theo de Raadt-2
In reply to this post by Jiri B-2
> > Would a patch to bring back the `!' command to less(1) be accepted?  The
> > commit message for its removal explains that ^Z should be used instead,
> > but that obviously does not work if less(1) is run from something else
> > than an interactive shell, for example when reading manual pages from a
> > vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> > by `neww vi' in a tmux(1) session.
>
> Why should less be able to spawn another programs? This would undermine
> all pledge work.

It does not undermine any pledge work at all.

The strategy is reduction of "many programs have ways to break out to
full system call operation, but why?"

Fixing all of these concerns won't happen in a day.  We are boiling this
frog slowly.

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

kshe-2
In reply to this post by Theo de Raadt-2
On Sat, 16 Dec 2017 19:39:27 +0000, Theo de Raadt wrote:

> > On Sat, 16 Dec 2017 18:13:16 +0000, Jiri B wrote:
> > > On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> > > > Hi,
> > > >
> > > > Would a patch to bring back the `!' command to less(1) be accepted?  The
> > > > commit message for its removal explains that ^Z should be used instead,
> > > > but that obviously does not work if less(1) is run from something else
> > > > than an interactive shell, for example when reading manual pages from a
> > > > vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> > > > by `neww vi' in a tmux(1) session.
> > >
> > > Why should less be able to spawn another programs? This would undermine
> > > all pledge work.
> >
> > Because of at least `v' and `|', less(1) already is able to invoke
> > arbitrary programs, and accordingly needs the "proc exec" promise, so
> > bringing `!' back would not change anything from a security perspective
> > (otherwise, I would obviously not have made such a proposition).
> >
> > In fact, technically, what I want to do is still currently possible:
> > from any less(1) instance, one may use `v' to invoke vi(1), and then use
> > vi(1)'s own `!' command as desired.  So the functionality of `!' is
> > still there; it was only made more difficult to reach for no apparent
> > reason.
>
> No apparent reason?
>
> Good you have an opinion.  I have a different opinion: We should look
> for rarely used functionality and gut it.

I completely agree, and I also completely agree with the rest of what
you said.  However, in this particular case, the functionality of `!' is
still fully (albeit indirectly) accessible, as shown above, and this is
why its deletion, when not immediately followed by that of `|' and `v',
made little sense for me.

Either the commands that require "proc exec" should all be removed along
with that promise, or `!' should be brought back without any pledge(2)
modifications.  But currently it really feels like a big waste (for both
parties) to request such high privileges, and then to do almost nothing
useful with them.

If the plan really was to get rid of all such commands eventually, what
exactly is preventing that from happening now?  May I go ahead and
prepare a patch to remove "proc exec" entirely?

Regards,

kshe

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Theo de Raadt-2
> On Sat, 16 Dec 2017 19:39:27 +0000, Theo de Raadt wrote:
> > > On Sat, 16 Dec 2017 18:13:16 +0000, Jiri B wrote:
> > > > On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> > > > > Hi,
> > > > >
> > > > > Would a patch to bring back the `!' command to less(1) be accepted?  The
> > > > > commit message for its removal explains that ^Z should be used instead,
> > > > > but that obviously does not work if less(1) is run from something else
> > > > > than an interactive shell, for example when reading manual pages from a
> > > > > vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> > > > > by `neww vi' in a tmux(1) session.
> > > >
> > > > Why should less be able to spawn another programs? This would undermine
> > > > all pledge work.
> > >
> > > Because of at least `v' and `|', less(1) already is able to invoke
> > > arbitrary programs, and accordingly needs the "proc exec" promise, so
> > > bringing `!' back would not change anything from a security perspective
> > > (otherwise, I would obviously not have made such a proposition).
> > >
> > > In fact, technically, what I want to do is still currently possible:
> > > from any less(1) instance, one may use `v' to invoke vi(1), and then use
> > > vi(1)'s own `!' command as desired.  So the functionality of `!' is
> > > still there; it was only made more difficult to reach for no apparent
> > > reason.
> >
> > No apparent reason?
> >
> > Good you have an opinion.  I have a different opinion: We should look
> > for rarely used functionality and gut it.
>
> I completely agree, and I also completely agree with the rest of what
> you said.  However, in this particular case, the functionality of `!' is
> still fully (albeit indirectly) accessible, as shown above, and this is
> why its deletion, when not immediately followed by that of `|' and `v',
> made little sense for me.

Oh, so you don't agree.  Or do you.  I can't tell.  You haven't made up
your mind enough to have a final position?

> Either the commands that require "proc exec" should all be removed along
> with that promise, or `!' should be brought back without any pledge(2)
> modifications.

That is pretty absolutist.

The universe is not always consistant, and neither is OpenBSD.

The final decisions haven't been made yet, because we haven't gauged
the usage patterns.

> But currently it really feels like a big waste (for both
> parties) to request such high privileges, and then to do almost nothing
> useful with them.

Request?  pledge isn't a "request" system.  It is a 2nd specification
of the program about maximum it believes it will use, and therefore it
is a hard brake.  At the moment the featureset still needs "proc exec".
So the specification isn't a waste, it is accurate.

> If the plan really was to get rid of all such commands eventually, what
> exactly is preventing that from happening now?  

The plan was to get rid of ! in a few commands, then later get rid of
a few more of them, and see where we end up.  With such plans, we
don't always act all on one step, because then it is too easy to get
embroiled in just that one battle and forget about the other things
which also need doing.  Also it is impossible to ask the community
because petty fights result and provide innaccurate usage assessments.

There are many other things to do.  As a result, our universe is not
always consistant.  This is an example.

> May I go ahead and prepare a patch to remove "proc exec" entirely?

Sure you could try, and see who freaks out.  Exactly what the plan was
all along.

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

kshe-2
On Sat, 16 Dec 2017 21:52:44 +0000, Theo de Raadt wrote:

> > On Sat, 16 Dec 2017 19:39:27 +0000, Theo de Raadt wrote:
> > > > On Sat, 16 Dec 2017 18:13:16 +0000, Jiri B wrote:
> > > > > On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Would a patch to bring back the `!' command to less(1) be accepted?  The
> > > > > > commit message for its removal explains that ^Z should be used instead,
> > > > > > but that obviously does not work if less(1) is run from something else
> > > > > > than an interactive shell, for example when reading manual pages from a
> > > > > > vi(1) instance spawned directly by `xterm -e vi' in a window manager or
> > > > > > by `neww vi' in a tmux(1) session.
> > > > >
> > > > > Why should less be able to spawn another programs? This would undermine
> > > > > all pledge work.
> > > >
> > > > Because of at least `v' and `|', less(1) already is able to invoke
> > > > arbitrary programs, and accordingly needs the "proc exec" promise, so
> > > > bringing `!' back would not change anything from a security perspective
> > > > (otherwise, I would obviously not have made such a proposition).
> > > >
> > > > In fact, technically, what I want to do is still currently possible:
> > > > from any less(1) instance, one may use `v' to invoke vi(1), and then use
> > > > vi(1)'s own `!' command as desired.  So the functionality of `!' is
> > > > still there; it was only made more difficult to reach for no apparent
> > > > reason.
> > >
> > > No apparent reason?
> > >
> > > Good you have an opinion.  I have a different opinion: We should look
> > > for rarely used functionality and gut it.
> >
> > I completely agree, and I also completely agree with the rest of what
> > you said.  However, in this particular case, the functionality of `!' is
> > still fully (albeit indirectly) accessible, as shown above, and this is
> > why its deletion, when not immediately followed by that of `|' and `v',
> > made little sense for me.
>
> Oh, so you don't agree.  Or do you.  I can't tell.  You haven't made up
> your mind enough to have a final position?

In the case of less(1), the underlying functionality of `!' (invoking
arbitrary programs) has not been removed at all, as `!' itself was only
one way amongst others of doing that.  Therefore, I would have prefered
that such an endeavour be conducted in steps at least as large as a
pledge(2) category.  You may say this is absolutist, but, in the end,
users might actually be more inclined to accept such removals if they
come with, and thus are justified by, a real and immediate security
benefit, like stricter pledge(2) promises, rather than some vague
theoretical explanation about the global state of their software
environment.

> [...]
>
> > May I go ahead and prepare a patch to remove "proc exec" entirely?
>
> Sure you could try, and see who freaks out.  Exactly what the plan was
> all along.

The minimal diff below does that.  If it is accepted, further cleanups
would need to follow (in particular, removing a few unused variables and
functions), and of course the manual would also need some adjustments.

Index: cmd.h
===================================================================
RCS file: /cvs/src/usr.bin/less/cmd.h,v
retrieving revision 1.10
diff -u -p -r1.10 cmd.h
--- cmd.h 6 Nov 2015 15:58:01 -0000 1.10
+++ cmd.h 17 Dec 2017 12:23:00 -0000
@@ -42,12 +42,12 @@
 #define A_FF_LINE 29
 #define A_BF_LINE 30
 #define A_VERSION 31
-#define A_VISUAL 32
+/* 32 unused */
 #define A_F_WINDOW 33
 #define A_B_WINDOW 34
 #define A_F_BRACKET 35
 #define A_B_BRACKET 36
-#define A_PIPE 37
+/* 37 unused */
 #define A_INDEX_FILE 38
 #define A_UNDO_SEARCH 39
 #define A_FF_SCREEN 40
Index: command.c
===================================================================
RCS file: /cvs/src/usr.bin/less/command.c,v
retrieving revision 1.31
diff -u -p -r1.31 command.c
--- command.c 12 Jan 2017 20:32:01 -0000 1.31
+++ command.c 17 Dec 2017 12:23:00 -0000
@@ -241,12 +241,6 @@ exec_mca(void)
  /* If tag structure is loaded then clean it up. */
  cleantags();
  break;
- case A_PIPE:
- if (secure)
- break;
- (void) pipe_mark(pipec, cbuf);
- error("|done", NULL);
- break;
  }
 }
 
@@ -1396,35 +1390,6 @@ again:
  c = getcc();
  goto again;
 
- case A_VISUAL:
- /*
- * Invoke an editor on the input file.
- */
- if (secure) {
- error("Command not available", NULL);
- break;
- }
- if (ch_getflags() & CH_HELPFILE)
- break;
- if (strcmp(get_filename(curr_ifile), "-") == 0) {
- error("Cannot edit standard input", NULL);
- break;
- }
- if (curr_altfilename != NULL) {
- error("WARNING: This file was viewed via "
-    "LESSOPEN", NULL);
- }
- /*
- * Expand the editor prototype string
- * and pass it to the system to execute.
- * (Make sure the screen is displayed so the
- * expansion of "+%lm" works.)
- */
- make_display();
- cmd_exec();
- lsystem(pr_expand(editproto, 0), NULL);
- break;
-
  case A_NEXT_FILE:
  /*
  * Examine next file.
@@ -1568,25 +1533,6 @@ again:
  cmd_exec();
  gomark(c);
  break;
-
- case A_PIPE:
- if (secure) {
- error("Command not available", NULL);
- break;
- }
- start_mca(A_PIPE, "|mark: ", (void*)NULL, 0);
- c = getcc();
- if (c == erase_char || c == erase2_char ||
-    c == kill_char)
- break;
- if (c == '\n' || c == '\r')
- c = '.';
- if (badmark(c))
- break;
- pipec = c;
- start_mca(A_PIPE, "!", ml_shell, 0);
- c = getcc();
- goto again;
 
  case A_B_BRACKET:
  case A_F_BRACKET:
Index: decode.c
===================================================================
RCS file: /cvs/src/usr.bin/less/decode.c,v
retrieving revision 1.18
diff -u -p -r1.18 decode.c
--- decode.c 17 Sep 2016 15:06:41 -0000 1.18
+++ decode.c 17 Dec 2017 12:23:00 -0000
@@ -148,8 +148,6 @@ static unsigned char cmdtable[] =
  ':', 'x', 0, A_INDEX_FILE,
  ':', 'd', 0, A_REMOVE_FILE,
  ':', 't', 0, A_OPT_TOGGLE|A_EXTRA, 't', 0,
- '|', 0, A_PIPE,
- 'v', 0, A_VISUAL,
  '+', 0, A_FIRSTCMD,
 
  'H', 0, A_HELP,
Index: lesskey.c
===================================================================
RCS file: /cvs/src/usr.bin/less/lesskey.c,v
retrieving revision 1.17
diff -u -p -r1.17 lesskey.c
--- lesskey.c 17 Sep 2016 15:06:41 -0000 1.17
+++ lesskey.c 17 Dec 2017 12:23:00 -0000
@@ -134,7 +134,6 @@ struct cmdname cmdnames[] = {
  { "next-tag", A_NEXT_TAG },
  { "noaction", A_NOACTION },
  { "percent", A_PERCENT },
- { "pipe", A_PIPE },
  { "prev-file", A_PREV_FILE },
  { "prev-tag", A_PREV_TAG },
  { "quit", A_QUIT },
@@ -152,7 +151,6 @@ struct cmdname cmdnames[] = {
  { "toggle-option", A_OPT_TOGGLE },
  { "undo-hilite", A_UNDO_SEARCH },
  { "version", A_VERSION },
- { "visual", A_VISUAL },
  { NULL, 0 }
 };
 
Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/less/main.c,v
retrieving revision 1.35
diff -u -p -r1.35 main.c
--- main.c 17 Sep 2016 15:06:41 -0000 1.35
+++ main.c 17 Dec 2017 12:23:00 -0000
@@ -96,7 +96,7 @@ main(int argc, char *argv[])
  exit(1);
  }
  } else {
- if (pledge("stdio rpath wpath cpath fattr proc exec tty", NULL) == -1) {
+ if (pledge("stdio rpath wpath cpath fattr tty", NULL) == -1) {
  perror("pledge");
  exit(1);
  }

Regards,

kshe

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Nicholas Marriott-2
I don't think we should bring ! back.

I wanted to remove v and | (and some other stuff) shortly afterwards, but
several people objected.

I did suggest having a lightweight less in base for most people and adding
the full upstream less to ports for the stuff we don't want to maintain
(like we do for eg libevent) but other people didn't like that idea.



On 17 December 2017 at 15:48, kshe <[hidden email]> wrote:

> On Sat, 16 Dec 2017 21:52:44 +0000, Theo de Raadt wrote:
> > > On Sat, 16 Dec 2017 19:39:27 +0000, Theo de Raadt wrote:
> > > > > On Sat, 16 Dec 2017 18:13:16 +0000, Jiri B wrote:
> > > > > > On Sat, Dec 16, 2017 at 04:55:44PM +0000, kshe wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > Would a patch to bring back the `!' command to less(1) be
> accepted?  The
> > > > > > > commit message for its removal explains that ^Z should be used
> instead,
> > > > > > > but that obviously does not work if less(1) is run from
> something else
> > > > > > > than an interactive shell, for example when reading manual
> pages from a
> > > > > > > vi(1) instance spawned directly by `xterm -e vi' in a window
> manager or
> > > > > > > by `neww vi' in a tmux(1) session.
> > > > > >
> > > > > > Why should less be able to spawn another programs? This would
> undermine
> > > > > > all pledge work.
> > > > >
> > > > > Because of at least `v' and `|', less(1) already is able to invoke
> > > > > arbitrary programs, and accordingly needs the "proc exec" promise,
> so
> > > > > bringing `!' back would not change anything from a security
> perspective
> > > > > (otherwise, I would obviously not have made such a proposition).
> > > > >
> > > > > In fact, technically, what I want to do is still currently
> possible:
> > > > > from any less(1) instance, one may use `v' to invoke vi(1), and
> then use
> > > > > vi(1)'s own `!' command as desired.  So the functionality of `!' is
> > > > > still there; it was only made more difficult to reach for no
> apparent
> > > > > reason.
> > > >
> > > > No apparent reason?
> > > >
> > > > Good you have an opinion.  I have a different opinion: We should look
> > > > for rarely used functionality and gut it.
> > >
> > > I completely agree, and I also completely agree with the rest of what
> > > you said.  However, in this particular case, the functionality of `!'
> is
> > > still fully (albeit indirectly) accessible, as shown above, and this is
> > > why its deletion, when not immediately followed by that of `|' and `v',
> > > made little sense for me.
> >
> > Oh, so you don't agree.  Or do you.  I can't tell.  You haven't made up
> > your mind enough to have a final position?
>
> In the case of less(1), the underlying functionality of `!' (invoking
> arbitrary programs) has not been removed at all, as `!' itself was only
> one way amongst others of doing that.  Therefore, I would have prefered
> that such an endeavour be conducted in steps at least as large as a
> pledge(2) category.  You may say this is absolutist, but, in the end,
> users might actually be more inclined to accept such removals if they
> come with, and thus are justified by, a real and immediate security
> benefit, like stricter pledge(2) promises, rather than some vague
> theoretical explanation about the global state of their software
> environment.
>
> > [...]
> >
> > > May I go ahead and prepare a patch to remove "proc exec" entirely?
> >
> > Sure you could try, and see who freaks out.  Exactly what the plan was
> > all along.
>
> The minimal diff below does that.  If it is accepted, further cleanups
> would need to follow (in particular, removing a few unused variables and
> functions), and of course the manual would also need some adjustments.
>
> Index: cmd.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/cmd.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 cmd.h
> --- cmd.h       6 Nov 2015 15:58:01 -0000       1.10
> +++ cmd.h       17 Dec 2017 12:23:00 -0000
> @@ -42,12 +42,12 @@
>  #define        A_FF_LINE               29
>  #define        A_BF_LINE               30
>  #define        A_VERSION               31
> -#define        A_VISUAL                32
> +/* 32 unused */
>  #define        A_F_WINDOW              33
>  #define        A_B_WINDOW              34
>  #define        A_F_BRACKET             35
>  #define        A_B_BRACKET             36
> -#define        A_PIPE                  37
> +/* 37 unused */
>  #define        A_INDEX_FILE            38
>  #define        A_UNDO_SEARCH           39
>  #define        A_FF_SCREEN             40
> Index: command.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/command.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 command.c
> --- command.c   12 Jan 2017 20:32:01 -0000      1.31
> +++ command.c   17 Dec 2017 12:23:00 -0000
> @@ -241,12 +241,6 @@ exec_mca(void)
>                 /* If tag structure is loaded then clean it up. */
>                 cleantags();
>                 break;
> -       case A_PIPE:
> -               if (secure)
> -                       break;
> -               (void) pipe_mark(pipec, cbuf);
> -               error("|done", NULL);
> -               break;
>         }
>  }
>
> @@ -1396,35 +1390,6 @@ again:
>                         c = getcc();
>                         goto again;
>
> -               case A_VISUAL:
> -                       /*
> -                        * Invoke an editor on the input file.
> -                        */
> -                       if (secure) {
> -                               error("Command not available", NULL);
> -                               break;
> -                       }
> -                       if (ch_getflags() & CH_HELPFILE)
> -                               break;
> -                       if (strcmp(get_filename(curr_ifile), "-") == 0) {
> -                               error("Cannot edit standard input", NULL);
> -                               break;
> -                       }
> -                       if (curr_altfilename != NULL) {
> -                               error("WARNING: This file was viewed via "
> -                                   "LESSOPEN", NULL);
> -                       }
> -                       /*
> -                        * Expand the editor prototype string
> -                        * and pass it to the system to execute.
> -                        * (Make sure the screen is displayed so the
> -                        * expansion of "+%lm" works.)
> -                        */
> -                       make_display();
> -                       cmd_exec();
> -                       lsystem(pr_expand(editproto, 0), NULL);
> -                       break;
> -
>                 case A_NEXT_FILE:
>                         /*
>                          * Examine next file.
> @@ -1568,25 +1533,6 @@ again:
>                         cmd_exec();
>                         gomark(c);
>                         break;
> -
> -               case A_PIPE:
> -                       if (secure) {
> -                               error("Command not available", NULL);
> -                               break;
> -                       }
> -                       start_mca(A_PIPE, "|mark: ", (void*)NULL, 0);
> -                       c = getcc();
> -                       if (c == erase_char || c == erase2_char ||
> -                           c == kill_char)
> -                               break;
> -                       if (c == '\n' || c == '\r')
> -                               c = '.';
> -                       if (badmark(c))
> -                               break;
> -                       pipec = c;
> -                       start_mca(A_PIPE, "!", ml_shell, 0);
> -                       c = getcc();
> -                       goto again;
>
>                 case A_B_BRACKET:
>                 case A_F_BRACKET:
> Index: decode.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/decode.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 decode.c
> --- decode.c    17 Sep 2016 15:06:41 -0000      1.18
> +++ decode.c    17 Dec 2017 12:23:00 -0000
> @@ -148,8 +148,6 @@ static unsigned char cmdtable[] =
>         ':', 'x', 0,                    A_INDEX_FILE,
>         ':', 'd', 0,                    A_REMOVE_FILE,
>         ':', 't', 0,                    A_OPT_TOGGLE|A_EXTRA,   't', 0,
> -       '|', 0,                         A_PIPE,
> -       'v', 0,                         A_VISUAL,
>         '+', 0,                         A_FIRSTCMD,
>
>         'H', 0,                         A_HELP,
> Index: lesskey.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/lesskey.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 lesskey.c
> --- lesskey.c   17 Sep 2016 15:06:41 -0000      1.17
> +++ lesskey.c   17 Dec 2017 12:23:00 -0000
> @@ -134,7 +134,6 @@ struct cmdname cmdnames[] = {
>         { "next-tag",                   A_NEXT_TAG },
>         { "noaction",                   A_NOACTION },
>         { "percent",                    A_PERCENT },
> -       { "pipe",                       A_PIPE },
>         { "prev-file",                  A_PREV_FILE },
>         { "prev-tag",                   A_PREV_TAG },
>         { "quit",                       A_QUIT },
> @@ -152,7 +151,6 @@ struct cmdname cmdnames[] = {
>         { "toggle-option",              A_OPT_TOGGLE },
>         { "undo-hilite",                A_UNDO_SEARCH },
>         { "version",                    A_VERSION },
> -       { "visual",                     A_VISUAL },
>         { NULL,                         0 }
>  };
>
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/main.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 main.c
> --- main.c      17 Sep 2016 15:06:41 -0000      1.35
> +++ main.c      17 Dec 2017 12:23:00 -0000
> @@ -96,7 +96,7 @@ main(int argc, char *argv[])
>                         exit(1);
>                 }
>         } else {
> -               if (pledge("stdio rpath wpath cpath fattr proc exec tty",
> NULL) == -1) {
> +               if (pledge("stdio rpath wpath cpath fattr tty", NULL) ==
> -1) {
>                         perror("pledge");
>                         exit(1);
>                 }
>
> Regards,
>
> kshe
>
>
Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Stuart Henderson
On 2017/12/22 19:47, Nicholas Marriott wrote:
> I don't think we should bring ! back.
>
> I wanted to remove v and | (and some other stuff) shortly afterwards, but
> several people objected.
>
> I did suggest having a lightweight less in base for most people and adding
> the full upstream less to ports for the stuff we don't want to maintain
> (like we do for eg libevent) but other people didn't like that idea.

less(1) can already be made more lightweight by setting LESSSECURE=1.
(I quite like this even without the reduced pledge, my biggest annoyance
with less is when I accidentally press 'v').

Any opinions on switching the default?

Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/less/main.c,v
retrieving revision 1.35
diff -u -p -u -1 -2 -r1.35 main.c
--- main.c 17 Sep 2016 15:06:41 -0000 1.35
+++ main.c 22 Dec 2017 22:19:04 -0000
@@ -87,17 +87,17 @@ main(int argc, char *argv[])
 
- secure = 0;
+ secure = 1;
  s = lgetenv("LESSSECURE");
- if (s != NULL && *s != '\0')
- secure = 1;
+ if (s != NULL && strcmp(s, "0") == 0)
+ secure = 0;
 
  if (secure) {
  if (pledge("stdio rpath wpath tty", NULL) == -1) {
  perror("pledge");
  exit(1);
  }
  } else {
  if (pledge("stdio rpath wpath cpath fattr proc exec tty", NULL) == -1) {
  perror("pledge");
  exit(1);
  }
  }
Index: less.1
===================================================================
RCS file: /cvs/src/usr.bin/less/less.1,v
retrieving revision 1.52
diff -u -p -r1.52 less.1
--- less.1 24 Oct 2016 13:46:58 -0000 1.52
+++ less.1 22 Dec 2017 22:17:28 -0000
@@ -1674,9 +1674,7 @@ differences in invocation syntax, the
 .Ev LESSEDIT
 variable can be changed to modify this default.
 .Sh SECURITY
-When the environment variable
-.Ev LESSSECURE
-is set to 1,
+Normally,
 .Nm
 runs in a "secure" mode.
 This means these features are disabled:
@@ -1698,6 +1696,10 @@ Metacharacters in filenames, such as "*"
 .It " "
 Filename completion (TAB, ^L).
 .El
+.Pp
+To enable these features, set the environment variable
+.Ev LESSSECURE
+to 0.
 .Sh COMPATIBILITY WITH MORE
 If the environment variable
 .Ev LESS_IS_MORE

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Alexander Hall


On December 22, 2017 11:21:12 PM GMT+01:00, Stuart Henderson <[hidden email]> wrote:

>On 2017/12/22 19:47, Nicholas Marriott wrote:
>> I don't think we should bring ! back.
>>
>> I wanted to remove v and | (and some other stuff) shortly afterwards,
>but
>> several people objected.
>>
>> I did suggest having a lightweight less in base for most people and
>adding
>> the full upstream less to ports for the stuff we don't want to
>maintain
>> (like we do for eg libevent) but other people didn't like that idea.
>
>less(1) can already be made more lightweight by setting LESSSECURE=1.
>(I quite like this even without the reduced pledge, my biggest
>annoyance
>with less is when I accidentally press 'v').
>
>Any opinions on switching the default?

An interesting twist on this is that if someone is currently (mistakenly) using
LESSECURE=0,
e.g. for not having their system "less secure", they would currently aquire the intended goal, while after this change, that would change.

Not sure if misconfigured systems are our main focus, but given the name "less", the aforementioned mistake doesn't strike me as totally unreasonable.

/Alexander

>Index: main.c
>===================================================================
>RCS file: /cvs/src/usr.bin/less/main.c,v
>retrieving revision 1.35
>diff -u -p -u -1 -2 -r1.35 main.c
>--- main.c 17 Sep 2016 15:06:41 -0000 1.35
>+++ main.c 22 Dec 2017 22:19:04 -0000
>@@ -87,17 +87,17 @@ main(int argc, char *argv[])
>
>- secure = 0;
>+ secure = 1;
> s = lgetenv("LESSSECURE");
>- if (s != NULL && *s != '\0')
>- secure = 1;
>+ if (s != NULL && strcmp(s, "0") == 0)
>+ secure = 0;
>
> if (secure) {
> if (pledge("stdio rpath wpath tty", NULL) == -1) {
> perror("pledge");
> exit(1);
> }
> } else {
> if (pledge("stdio rpath wpath cpath fattr proc exec tty", NULL) ==
>-1) {
> perror("pledge");
> exit(1);
> }
> }
>Index: less.1
>===================================================================
>RCS file: /cvs/src/usr.bin/less/less.1,v
>retrieving revision 1.52
>diff -u -p -r1.52 less.1
>--- less.1 24 Oct 2016 13:46:58 -0000 1.52
>+++ less.1 22 Dec 2017 22:17:28 -0000
>@@ -1674,9 +1674,7 @@ differences in invocation syntax, the
> .Ev LESSEDIT
> variable can be changed to modify this default.
> .Sh SECURITY
>-When the environment variable
>-.Ev LESSSECURE
>-is set to 1,
>+Normally,
> .Nm
> runs in a "secure" mode.
> This means these features are disabled:
>@@ -1698,6 +1696,10 @@ Metacharacters in filenames, such as "*"
> .It " "
> Filename completion (TAB, ^L).
> .El
>+.Pp
>+To enable these features, set the environment variable
>+.Ev LESSSECURE
>+to 0.
> .Sh COMPATIBILITY WITH MORE
> If the environment variable
> .Ev LESS_IS_MORE

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

kshe-2
In reply to this post by Stuart Henderson
On Fri, 22 Dec 2017 22:21:12 +0000, Stuart Henderson wrote:

> On 2017/12/22 19:47, Nicholas Marriott wrote:
> > I don't think we should bring ! back.
> >
> > I wanted to remove v and | (and some other stuff) shortly afterwards, but
> > several people objected.
> >
> > I did suggest having a lightweight less in base for most people and adding
> > the full upstream less to ports for the stuff we don't want to maintain
> > (like we do for eg libevent) but other people didn't like that idea.
>
> less(1) can already be made more lightweight by setting LESSSECURE=1.
> (I quite like this even without the reduced pledge, my biggest annoyance
> with less is when I accidentally press 'v').
>
> Any opinions on switching the default?

I thought about that possibility too, and I mostly agree with the idea
as I also run less(1) in secure mode very often, but it is nevertheless
quite irrelevant to the original concern, which is that, when one
chooses not to run less(1) in secure mode, whether that mode is the
default one or not, it is inconsistent, for multiple reasons, to have
removed the `!' command, but not `v' nor `|'.

Until some form of agreement can be reached on that issue, I have
reverted the removal of `!' in my personal tree, so I still pay the
exact same price as everybody else ("proc exec"), but at least I now get
something useful out of that.

Regards,

kshe

Reply | Threaded
Open this post in threaded view
|

Re: less(1): `!' command

Jeremie Courreges-Anglas-2
In reply to this post by Stuart Henderson
On Fri, Dec 22 2017, Stuart Henderson <[hidden email]> wrote:

> On 2017/12/22 19:47, Nicholas Marriott wrote:
>> I don't think we should bring ! back.
>>
>> I wanted to remove v and | (and some other stuff) shortly afterwards, but
>> several people objected.
>>
>> I did suggest having a lightweight less in base for most people and adding
>> the full upstream less to ports for the stuff we don't want to maintain
>> (like we do for eg libevent) but other people didn't like that idea.
>
> less(1) can already be made more lightweight by setting LESSSECURE=1.
> (I quite like this even without the reduced pledge, my biggest annoyance
> with less is when I accidentally press 'v').
>
> Any opinions on switching the default?

Makes sense to me, I can live without the 's' command.  ok jca@

> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/main.c,v
> retrieving revision 1.35
> diff -u -p -u -1 -2 -r1.35 main.c
> --- main.c 17 Sep 2016 15:06:41 -0000 1.35
> +++ main.c 22 Dec 2017 22:19:04 -0000
> @@ -87,17 +87,17 @@ main(int argc, char *argv[])
>  
> - secure = 0;
> + secure = 1;
>   s = lgetenv("LESSSECURE");
> - if (s != NULL && *s != '\0')
> - secure = 1;
> + if (s != NULL && strcmp(s, "0") == 0)
> + secure = 0;
>  
>   if (secure) {
>   if (pledge("stdio rpath wpath tty", NULL) == -1) {
>   perror("pledge");
>   exit(1);
>   }
>   } else {
>   if (pledge("stdio rpath wpath cpath fattr proc exec tty", NULL) == -1) {
>   perror("pledge");
>   exit(1);
>   }
>   }
> Index: less.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/less.1,v
> retrieving revision 1.52
> diff -u -p -r1.52 less.1
> --- less.1 24 Oct 2016 13:46:58 -0000 1.52
> +++ less.1 22 Dec 2017 22:17:28 -0000
> @@ -1674,9 +1674,7 @@ differences in invocation syntax, the
>  .Ev LESSEDIT
>  variable can be changed to modify this default.
>  .Sh SECURITY
> -When the environment variable
> -.Ev LESSSECURE
> -is set to 1,
> +Normally,
>  .Nm
>  runs in a "secure" mode.
>  This means these features are disabled:
> @@ -1698,6 +1696,10 @@ Metacharacters in filenames, such as "*"
>  .It " "
>  Filename completion (TAB, ^L).
>  .El
> +.Pp
> +To enable these features, set the environment variable
> +.Ev LESSSECURE
> +to 0.
>  .Sh COMPATIBILITY WITH MORE
>  If the environment variable
>  .Ev LESS_IS_MORE
>

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE