Re: OpenSSH and -current out-of-tree patched for ~C?
Thus said Theo de Raadt on Wed, 30 Nov 2022 19:44:09 -0700: > It makes ssh safer for people who don't use the fancy features, > because the ssh client cannot perform a vast number of system calls if > it gets fooled. Got it, makes sense now; and as you say my understanding was backwards. pledge() is being used to eliminate a bunch of risky system calls for those who are not using ~C and are still at risk even if they are NOT using ~C (especially where ~C users are in the minority as you point out). Also, as Stuart explained, there is at least an alternative mechanism for opening up dynamic tunnels which means that the need to enable ~C is even less compelling (as long as one is using ControlMaster which is arguably another one of those "power user" features). And while I've used ControlMaster for years, I was unaware of this alternative as I didn't realize that a shared session could cause the master to open up new tunnels that would remain in place even after the slave exits (nor indeed had I even thought to try it). In my testing it seems that they do in fact remain. Thanks, Andy
Re: OpenSSH and -current out-of-tree patched for ~C?
Thus said Stuart Henderson on Wed, 30 Nov 2022 16:13:36 +: > It allows a much tighter pledge in the client, so less attack surface > against a bad server. So it's to prevent a malicious SSH server from exploiting a client who choses to use ~C to open up the ssh> prompt and create or destroy tunnels? > Alternatively you can use connection multiplexing (which didn't > support ~C anyway) and run a separate ssh -L / -R, which will > establish an extra channel using the existing connection. I also already use connection multiplexing, so this might be an option, however, does this mean that I'll have to maintain an extra terminal just so I can open a new -L/-R or can I just login and logout and that will effectively open the channel on the master channel and then exit? I guess I'll have to experiment some. But I'll probably just turn on the new option whenever I encounter the lack of functionality. Thanks for the suggestion. Andy
Re: OpenSSH and -current out-of-tree patched for ~C?
Thus said "Theo de Raadt" on Wed, 23 Nov 2022 18:56:21 -0700: > A new "enablecommandline" configuration option re-enables those > particular features, and the diff later on will show why we feel these > features should be optional. Glad that the option is being retained as optional but I also look forward to seeing the rationale for this change. I for one use the ~C, not daily, but often enough that I would have noticed over a longer period of time because I don't run -current. In my particular scenario, I often open up long running SSH sessions that have tunnels to other TCP/IP sessions (some of them interactive), and the ability to add tunnels dynamically means that I don't have to restart a bunch of interactive sessions or interrupt other long-running things over the tunnels. If I decide that the tunnel is something I want to use long-term, I'll add it to ~/.ssh/config for future use. Thanks, Andy
Move auth_approval in su.c before fork is lost due to pledge?
Hello,
I noticed that my locate.database wasn't being updated:
Rebuilding locate database:
Abort trap
Not installing locate database; zero size
>From the following:
echo "${UPDATEDB} --fcodes=- --tmpdir=${TMPDIR:-/tmp}" | \
nice -5 su -m nobody 2>/dev/null 1>$TMP
As it turns out, it is because I have an approve entry in
/etc/login.conf and this requires the ability to fork the approval
program. When su tries to run approve it fails and I find the following
in dmesg:
su(77960): syscall 2 "proc"
ktrace also shows that pledge shut it down. So is the following patch
correct? I don't see any downsides, but perhaps there reasons for why
auth_approval happens last?
Index: su.c
===
RCS file: /home/cvs/src/usr.bin/su/su.c,v
retrieving revision 1.70
diff -u -p -r1.70 su.c
--- su.c30 Oct 2015 19:45:03 - 1.70
+++ su.c8 Jan 2017 04:07:14 -
@@ -215,6 +215,9 @@ main(int argc, char **argv)
fprintf(stderr, "Login incorrect\n");
}
+ if (pwd->pw_uid && auth_approval(as, lc, pwd->pw_name, "su") <= 0)
+ auth_err(as, 1, "approval failure");
+
if (pledge("stdio rpath getpw exec id", NULL) == -1)
err(1, "pledge");
@@ -332,9 +335,6 @@ main(int argc, char **argv)
if (pledge("stdio rpath exec", NULL) == -1)
err(1, "pledge");
-
- if (pwd->pw_uid && auth_approval(as, lc, pwd->pw_name, "su") <= 0)
- auth_err(as, 1, "approval failure");
auth_close(as);
execv(shell, np);
Thanks,
Andy
--
TAI64 timestamp: 40005871bcbe
Re: regression tests and patch for calendar(1)
Thus said "Todd C. Miller" on Wed, 14 Sep 2016 10:04:58 -0600: > I've committed the fix as well as the calendar regress. Excellent. I've actually been working on a program that can be put into regress instead of the large number of files that currently exist for the expected output; it will generate the expected output files based on a given set of input and a date, then run calendar to test. But, the current regress will suffice until that is complete. Thanks, Andy -- TAI64 timestamp: 400057da0b2f
regression tests and patch for calendar(1)
Hello,
While writing a set of regression tests for calendar(1) I discovered a
bug introduced by my last patch. The following patch fixes that and all
regression tests in the attachment of tests passes.
Thanks,
Andy
Index: day.c
===
RCS file: /home/cvs/src/usr.bin/calendar/day.c,v
retrieving revision 1.33
diff -u -p -r1.33 day.c
--- day.c 13 Jul 2016 21:32:01 - 1.33
+++ day.c 31 Aug 2016 13:40:01 -
@@ -543,7 +543,9 @@ isnow(char *endp, int bodun)
tdiff = difftime(ttmp, f_time)/ SECSPERDAY;
if (tdiff <= offset + f_dayAfter ||
(bodun && tdiff == -1)) {
- if ((tmtmp.tm_mon == month) &&
+ if (((tmtmp.tm_mon == month) ||
+(flags & F_SPECIAL) ||
+(interval == WEEKLY)) &&
(tdiff >= 0 ||
(bodun && tdiff == -1))) {
if ((tmp = malloc(sizeof(struct
match))) == NULL)
calendar-regress.tar.gz
Description: calendar-regress.tar.gz
Re: [PATCH] shutdown: dot not write non-printable characters to wall(1)
Thus said Consus on Mon, 01 Aug 2016 21:58:16 +0300: > This patch fixes the issue by removing '\007' from the shutdown > notification. Do ttys no longer understand bel? Thanks, Andy -- TAI64 timestamp: 4000579f9de9
Re: Regression tests for calendar(1)
Thus said "Andy Bradford" on 28 Jul 2016 02:56:59 -0600: > Not enough? Different style required? Does the patch need to be broken > up? Or a different approach? Specifically, I originally had in mind, a single monolithic input file (generated with as many possible combinations of rules as I did in the current wdout.in) with 366+ test outputs (one for each day of the year, with maybe a few more from a non-leap year), but wasn't certain if that was necessary. Thanks, Andy -- TAI64 timestamp: 4000579a1a60
Followup patch for regression to calendar(1)
Hello,
Here's the followup patch that accounts for the missed events.
Specifically, some kinds of events actually can have a differing month
(specifically Easter and Weekly events like Every Monday).
This allows all tests to pass (from previous email):
Index: day.c
===
RCS file: /home/cvs/src/usr.bin/calendar/day.c,v
retrieving revision 1.33
diff -u -p -r1.33 day.c
--- day.c 13 Jul 2016 21:32:01 - 1.33
+++ day.c 28 Jul 2016 08:46:54 -
@@ -543,7 +543,9 @@ isnow(char *endp, int bodun)
tdiff = difftime(ttmp, f_time)/ SECSPERDAY;
if (tdiff <= offset + f_dayAfter ||
(bodun && tdiff == -1)) {
- if ((tmtmp.tm_mon == month) &&
+ if (((tmtmp.tm_mon == month) ||
+(flags & F_SPECIAL) ||
+(interval == WEEKLY)) &&
(tdiff >= 0 ||
(bodun && tdiff == -1))) {
if ((tmp = malloc(sizeof(struct
match))) == NULL)
--
TAI64 timestamp: 40005799c9d4
Regression tests for calendar(1)
Hello, A few weeks ago I submitted a patch to calendar(1) that helped it handle fifth weekday events and 31st events, and afterward, I noticed that there were no regression tests. In writing them, I discovered that while the patch fixed these two particular cases, it broke others. I'll send a potential fix which passes all the tests in a following email. But here is a potential set of regressions and comments/suggestions are appreciated. Are there too many tests? Not enough? Different style required? Does the patch need to be broken up? Thanks. Index: Makefile === RCS file: /home/cvs/src/regress/usr.bin/Makefile,v retrieving revision 1.32 diff -u -p -r1.32 Makefile --- Makefile26 Jul 2015 17:29:41 - 1.32 +++ Makefile28 Jul 2016 08:25:26 - @@ -1,7 +1,7 @@ # $OpenBSD: Makefile,v 1.32 2015/07/26 17:29:41 zhuk Exp $ # $NetBSD: Makefile,v 1.1 1997/12/30 23:27:11 cgd Exp $ -SUBDIR+= basename bc dc diff diff3 dirname doas file grep gzip +SUBDIR+= basename bc calendar dc diff diff3 dirname doas file grep gzip SUBDIR+= m4 mandoc openssl sdiff sed signify sort tsort SUBDIR+= xargs Index: calendar/20160101.wdout === RCS file: calendar/20160101.wdout diff -N calendar/20160101.wdout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160101.wdout 28 Jul 2016 08:25:26 - @@ -0,0 +1,8 @@ +Jan 01*First Friday +Jan 01*First Friday in Jan +Jan 02*First Saturday +Jan 02*First Saturday in Jan +Jan 03*First Sunday +Jan 03*First Sunday in Jan +Jan 04*First Monday +Jan 04*First Monday in Jan Index: calendar/20160114.manout === RCS file: calendar/20160114.manout diff -N calendar/20160114.manout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160114.manout28 Jul 2016 08:25:26 - @@ -0,0 +1,2 @@ +Jan 14*Every Thursday +Jan 15*15th of every month Index: calendar/20160115.wdout === RCS file: calendar/20160115.wdout diff -N calendar/20160115.wdout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160115.wdout 28 Jul 2016 08:25:26 - @@ -0,0 +1,8 @@ +Jan 15*Third Friday +Jan 15*Third Friday in Jan +Jan 16*Third Saturday +Jan 16*Third Saturday in Jan +Jan 17*Third Sunday +Jan 17*Third Sunday in Jan +Jan 18*Third Monday +Jan 18*Third Monday in Jan Index: calendar/20160129.wdout === RCS file: calendar/20160129.wdout diff -N calendar/20160129.wdout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160129.wdout 28 Jul 2016 08:25:26 - @@ -0,0 +1,9 @@ +Jan 29*Fifth Friday +Jan 29*Fifth Friday in Jan +Jan 30*Fifth Saturday +Jan 30*Fifth Saturday in Jan +Jan 31*Fifth Sunday +Jan 31*Fifth Sunday in Jan +Feb 01*First Monday +Feb 01*First Monday in Feb +Feb 01*Fifth Monday in Jan Index: calendar/20160131.monout === RCS file: calendar/20160131.monout diff -N calendar/20160131.monout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160131.monout28 Jul 2016 08:25:26 - @@ -0,0 +1,3 @@ +Jan 31*Every Sunday +Feb 01*First of Month +Feb 01*Every Monday Index: calendar/20160201.wdout === RCS file: calendar/20160201.wdout diff -N calendar/20160201.wdout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160201.wdout 28 Jul 2016 08:25:26 - @@ -0,0 +1,6 @@ +Feb 01*First Monday +Feb 01*First Monday in Feb +Feb 01*Fifth Monday in Jan +Feb 02*First Tuesday +Feb 02*First Tuesday in Feb +Feb 02*Fifth Tuesday in Jan Index: calendar/20160203.wdout === RCS file: calendar/20160203.wdout diff -N calendar/20160203.wdout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160203.wdout 28 Jul 2016 08:25:26 - @@ -0,0 +1,6 @@ +Feb 03*First Wednesday +Feb 03*First Wednesday in Feb +Feb 03*Fifth Wednesday in Jan +Feb 04*First Thursday +Feb 04*First Thursday in Feb +Feb 04*Fifth Thursday in Jan Index: calendar/20160229.wdout === RCS file: calendar/20160229.wdout diff -N calendar/20160229.wdout --- /dev/null 1 Jan 1970 00:00:00 - +++ calendar/20160229.wdout 28 Jul 2016 08:25:26 - @@ -0,0 +1,5 @@ +Feb 29*Fifth Monday +Feb 29*Fifth Monday in Feb +Mar 01*First Tuesday +Mar 01*
calendar(1) handling of Weekday+5 and 31st events
Hello,
The following patch seems to allow calendar to better handle the fifth
weekday and 31st events. Currently, both events match also at the
beginning of certain months which doesn't seem to make sense. For
example, March 02 is considered a 31st this year and July 03 is
considered a Fifth Sunday:
$ sh /tmp/calendar.sh
calendar -f /tmp/calendar.We1Ga8 -t 20160101
Jan 03* Fifth Sunday.
calendar -f /tmp/calendar.We1Ga8 -t 20160130
Jan 31* Fifth Sunday.
Jan 31* 31st.
calendar -f /tmp/calendar.We1Ga8 -t 20160301
Mar 02* 31st.
calendar -f /tmp/calendar.We1Ga8 -t 20160701
Jul 01* 31st.
Jul 03* Fifth Sunday.
calendar -f /tmp/calendar.We1Ga8 -t 20160729
Jul 31* Fifth Sunday.
Jul 31* 31st.
Here is how it behaves with the patch:
$ sh /tmp/calendar.sh /usr/src/usr.bin/calendar/obj/calendar
/usr/src/usr.bin/calendar/obj/calendar -f /tmp/calendar.83O1qH -t 20160101
/usr/src/usr.bin/calendar/obj/calendar -f /tmp/calendar.83O1qH -t 20160130
Jan 31* Fifth Sunday.
Jan 31* 31st.
/usr/src/usr.bin/calendar/obj/calendar -f /tmp/calendar.83O1qH -t 20160301
/usr/src/usr.bin/calendar/obj/calendar -f /tmp/calendar.83O1qH -t 20160701
/usr/src/usr.bin/calendar/obj/calendar -f /tmp/calendar.83O1qH -t 20160729
Jul 31* Fifth Sunday.
Jul 31* 31st.
Here is calendar.sh:
$ cat /tmp/calendar.sh
PROG=${1:-calendar}
CALENDAR=$(mktemp -t calendar.XX)
printf '
Sunday+5\tFifth Sunday.
31 *\t31st.
' >"$CALENDAR"
for date in 20160101 20160130 20160301 20160701 20160729
do
set -- "$PROG" -f "$CALENDAR" -t "$date"
echo "$@"
eval "$@"
done
rm "$CALENDAR"
And here is the patch:
Index: calendar/day.c
===
RCS file: /home/cvs/src/usr.bin/calendar/day.c,v
retrieving revision 1.32
diff -u -p -r1.32 day.c
--- calendar/day.c 8 Dec 2015 19:04:50 - 1.32
+++ calendar/day.c 9 Jul 2016 20:53:13 -
@@ -543,8 +543,9 @@ isnow(char *endp, int bodun)
tdiff = difftime(ttmp, f_time)/ SECSPERDAY;
if (tdiff <= offset + f_dayAfter ||
(bodun && tdiff == -1)) {
- if (tdiff >= 0 ||
- (bodun && tdiff == -1)) {
+ if ((tmtmp.tm_mon == month) &&
+ (tdiff >= 0 ||
+ (bodun && tdiff == -1))) {
if ((tmp = malloc(sizeof(struct
match))) == NULL)
err(1, NULL);
tmp->when = ttmp;
This works because when variable_weekday returns it places a value in v1
that is larger than a month's worth of days. For example, for July 1,
2016, Sunday+5 gets a value of 33 in tmtmp.tm_mday for which mktime
advances advances the month to account for the extra days when creating
the timestamp. Now the months are different and should probably not
count as a match.
Are there any bad side-effects in this approach?
Thanks,
Andy
--
TAI64 timestamp: 40005781686f
