Re: Picky, but much more efficient arc4random_uniform!
Yeah. It most likely won't go in. From past experience and advice, not necessarily just from a perceived lack of merit. However, many, if not all of the arguments are based upon non-facts and misconceptions from earlier submissions or just not understanding what the software is doing. The only real thing that makes it not quite as good as the standard is mine isn't threadsafe. If it could be accepted as a higher performance, non-threadsafe call, it would perform better in many typical cases and perhaps even give a safer return value, especially in large upper_bound edge cases, I suspect. There is the specifically non-threadsafe call getchar_unlocked() on OpenBSD which is presumably available for performance reasons alone, when getchar() is a perfectly viable option and is even an ISO conforming function. What I submitted could be such a higher performance non-threadsafe function. so, how about arc4random_uniform_unlocked() ?! ...other than making upper_bound a uint32_t instead of the submitted uint64_t. That'd be somewhat of a problem. -Luke > You've had several developers tell you this is not going to go in. I'd > suggest > "read the room". > > If you want this for your own use, just keep it as a local diff. Nobody > will > know (or likely care). > > -ml >
Re: cwm: remove menu-ssh
Ok to remove. -- Gabriel Okan Demirmen writes: > Hi, > > I think we've (or at least I have) mused about this for a while; a > recent mail reminded me that this feature should go - a window manager > doesn't need to parse the ssh known_hosts file for a menu; there are > better tools for this. > > Remove menu-ssh. > > okay? > > Index: calmwm.h > === > RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v > retrieving revision 1.372 > diff -u -p -r1.372 calmwm.h > --- calmwm.h 22 Jan 2020 19:58:35 - 1.372 > +++ calmwm.h 22 Jan 2020 20:09:13 - > @@ -304,7 +304,6 @@ struct conf { > int xrandr; > int xrandr_event_base; > char*conf_file; > - char*known_hosts; > char*wm_argv; > int debug; > }; > @@ -517,7 +516,6 @@ void kbfunc_menu_cmd(void *, struct > c > void kbfunc_menu_group(void *, struct cargs *); > void kbfunc_menu_wm(void *, struct cargs *); > void kbfunc_menu_exec(void *, struct cargs *); > -void kbfunc_menu_ssh(void *, struct cargs *); > void kbfunc_client_menu_label(void *, struct cargs *); > void kbfunc_exec_cmd(void *, struct cargs *); > void kbfunc_exec_lock(void *, struct cargs *); > Index: conf.c > === > RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v > retrieving revision 1.249 > diff -u -p -r1.249 conf.c > --- conf.c7 Mar 2019 12:54:21 - 1.249 > +++ conf.c22 Jan 2020 20:09:24 - > @@ -179,7 +179,6 @@ static const struct { > > { FUNC_SC(menu-cmd, menu_cmd, 0) }, > { FUNC_SC(menu-group, menu_group, 0) }, > - { FUNC_SC(menu-ssh, menu_ssh, 0) }, > { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) }, > { FUNC_SC(menu-window-hidden, menu_client, CWM_MENU_WINDOW_HIDDEN) }, > { FUNC_SC(menu-exec, menu_exec, 0) }, > @@ -210,7 +209,6 @@ static const struct { > { "CM-Delete", "lock" }, > { "M-question", "menu-exec" }, > { "CM-w", "menu-exec-wm" }, > - { "M-period", "menu-ssh" }, > { "M-Return", "window-hide" }, > { "M-Down", "window-lower" }, > { "M-Up", "window-raise" }, > @@ -316,7 +314,6 @@ conf_init(struct conf *c) > home = "/"; > } > xasprintf(&c->conf_file, "%s/%s", home, ".cwmrc"); > - xasprintf(&c->known_hosts, "%s/%s", home, ".ssh/known_hosts"); > } > > void > @@ -363,7 +360,6 @@ conf_clear(struct conf *c) > free(c->color[i]); > > free(c->conf_file); > - free(c->known_hosts); > free(c->font); > free(c->wmname); > } > Index: cwm.1 > === > RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v > retrieving revision 1.65 > diff -u -p -r1.65 cwm.1 > --- cwm.1 9 Jul 2019 21:38:44 - 1.65 > +++ cwm.1 22 Jan 2020 20:08:19 - > @@ -140,15 +140,6 @@ Resize window by a large amount; see > Spawn > .Dq exec program > dialog. > -.It Ic M-period > -Spawn > -.Dq ssh to > -dialog. > -This parses > -.Pa $HOME/.ssh/known_hosts > -to provide host auto-completion. > -.Xr ssh 1 > -will be executed via the configured terminal emulator. > .It Ic CM-w > Spawn > .Dq exec WindowManager > Index: cwmrc.5 > === > RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v > retrieving revision 1.73 > diff -u -p -r1.73 cwmrc.5 > --- cwmrc.5 2 Jul 2019 23:37:47 - 1.73 > +++ cwmrc.5 22 Jan 2020 20:07:57 - > @@ -280,10 +280,6 @@ menu. > Launch > .Dq exec WindowManager > menu. > -.It menu-ssh > -Launch > -.Dq ssh > -menu. > .It group-toggle-[n] > Toggle visibility of group n, where n is 1-9. > .It group-only-[n] > Index: kbfunc.c > === > RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v > retrieving revision 1.167 > diff -u -p -r1.167 kbfunc.c > --- kbfunc.c 21 Jan 2020 15:50:03 - 1.167 > +++ kbfunc.c 22 Jan 2020 20:09:03 - > @@ -647,72 +647,6 @@ out: > } > > void > -kbfunc_menu_ssh(void *ctx, struct cargs *cargs) > -{ > - struct screen_ctx *sc = ctx; > - struct cmd_ctx *cmd; > - struct menu *mi; > - struct menu_qmenuq; > - FILE*fp; > - char*buf, *lbuf, *p; > - char hostbuf[HOST_NAME_MAX+1]; > - char path[PATH_MAX]; > - int l; > - size_t len; > - ssize_t slen; > - int mflags = (CWM_MENU_DUMMY); >
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
i...@b3zdna.net wrote: > > Eventually there will protocol or text side bug in xterm, and someone > > will figure out ways to escalate beyond our other mitigations. Rather > > than forcing them to operate inside an uncomfortable less-than-POSIX > > pledge+unveil environment, they can reuse a simple 1996-era shell-egg to > > execve straight to a fresh shell for their pleasure. Because pledge > > "proc exec" lets them do so. > > But should it not be up to us to make sure that these exploits are > mitigated? Is it not our responsibility to properly go through all of > the changes made to applications in the source tree to make sure they're > secure? You'll turn into a smurf before you finish auditing xterm perfectly. And in the meantime, a new version from upstream will arrive. My point of view is for critical pieces of code, the best way to curtail risk isn't just to review against bugs, but to minimize what the software can do if such bugs are triggered. So I argue we should try to force xterm into a somewhat privsep/privdrop mindset -- using pledge and unveil to constrain action when it goes off the rails, and hopefully collect crashes that alert us ahead of time about problems. And then, I think we can disable features that aren't being used, when we shrink the unveil and pledge. I wonder how "localeFilter" / "luit" is supposed to work, because we don't allow "exec", or is that started earlier, and if so has someone reviewed that code, and added pledge/unveil to restrict the failure modes? xterm is already full of #ifdef, why not add a few more? If people don't like the xterm variation we ship, there are 20+ xterm variations with all sorts of fluff they will use instead (and most of them probably have these features ... and far worse perhaps) That's how I see it. Others might not agree, probably because they are using features I don't.
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
> Eventually there will protocol or text side bug in xterm, and someone > will figure out ways to escalate beyond our other mitigations. Rather > than forcing them to operate inside an uncomfortable less-than-POSIX > pledge+unveil environment, they can reuse a simple 1996-era shell-egg to > execve straight to a fresh shell for their pleasure. Because pledge > "proc exec" lets them do so. But should it not be up to us to make sure that these exploits are mitigated? Is it not our responsibility to properly go through all of the changes made to applications in the source tree to make sure they're secure?
Re: Picky, but much more efficient arc4random_uniform!
I’m not trying to be rude, but you don’t realize what’s going on here: uuu is a bitmask: ‘uuu’ (or (1 << bits)-1 ) in “ret = rand_holder & uuu;“ , only puts the lower ‘bit’ quantity of bits of rand_holder into ret, then it right shifts rand_holder afterward to trash them every time in the loop when it’s done. So if bits is 8, uuu is going to be 0xff No, you aren't: > > > for (;;) { > > if (rand_bits < bits) { > > rand_holder |= ((uint64_t)arc4random()) << > > rand_bits; > > > > /* > > * rand_bits will be a number between 0 and 31 > here > > * so the 0x20 bit will be empty > > * rand_bits += 32; > > */ > > rand_bits |= 32; > > } > > > > ret = rand_holder & uuu; > > rand_holder >>= bits; > > rand_bits -= bits; > > > > if (ret < upper_bound) > > return ret; > > } > > This isn't rejection sampling. This is reusing part of the rejected > sample. -- -Luke
Re: [patch] added cwm config property for hiding resize hints
Just tested this patch and confirm it works as expected. Is there any plan to add it to cwm? -- Gabriel
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
i...@b3zdna.net wrote: > For one me; I use it to send highlighted text in the terminal to an > external program. This highlighted text could be a relative path to be > opened by the external program and thus requires a cwd. But your xterm is less secure. Eventually there will protocol or text side bug in xterm, and someone will figure out ways to escalate beyond our other mitigations. Rather than forcing them to operate inside an uncomfortable less-than-POSIX pledge+unveil environment, they can reuse a simple 1996-era shell-egg to execve straight to a fresh shell for their pleasure. Because pledge "proc exec" lets them do so. "proc exec" can only be safely used by (small) shells, or during startup of processes setting up a fork+exec privsep, or in one process of a very careful privsep design (probably using strict unveil) That doesn't describe xterm. I think this feature is very misguided.
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
> Having looked into this, I think "exec-formatted" and "exec-selectable". > are the kind of mis-designed anti-security features I don't want in xterm Fair enough. If this feature is already capable of circumventing the system security then I see why adding them in might not be a good idea. ig0r
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
> There is so much regret in this feature. xterm is supposed to be as > secure as possible so why does it need to inspect all the processes > in the system (that is what this feature gives xterm, as soon as you > pledge "ps"). I don't see any other way to get xterm's cwd. If there's a way to do so without having to inpsect all processes, I'm happy to make the change to the patch to do just that. > How about if xterm wasn't capable of doing that? Is it much of a loss? > Who is using this? For one me; I use it to send highlighted text in the terminal to an external program. This highlighted text could be a relative path to be opened by the external program and thus requires a cwd. I'm sure there are other people who use exec-formatted or other xterm functions that rely on cwd who could benefit from this change. ig0r
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
Having looked into this, I think "exec-formatted" and "exec-selectable". are the kind of mis-designed anti-security features I don't want in xterm The idea of this is very much against the idea of priv-sep or priv-drop. If a command is supposed to be capable of starting another fresh command at any time in the future, then it obviously cannot drop all it's capabilities, therefore it is not privdrop. It must retain the ability to spawn new commands, but that is precisely what we don't want xterm to be able to do. This is handled optionally, by users configuring it, but the users are not warned that this makes their xterm less self-protecting against the excalation of bugs (which will always exist in a program as large as xterm) We deleted the ! command from less(1), and similar spawning-features from other commands, with the same justification, and thus far our users have adapted and there have been no complaints. I would be super-happy if this chunk was deleted from xterm: { String data = NULL; getKeymapResources(SHELL_OF(term), "vt100", "VT100", XtRString, &data, sizeof(dat\ a)); if (data && (strstr(data, "exec-formatted") || strstr(data, "exec-selectable"))) { if (pledge("stdio rpath wpath id proc exec tty", NULL) == -1) { xtermWarning("pledge\n"); exit(1); } of course, also manual page-updates to inform users that these 3 features which use re-exec (also "spawn-new-terminal") are unsupported, plus why we did so. Because any user using that feature is running a less-secure xterm than the alternative which has a bunch of unveil() plus if (pledge("stdio rpath wpath id proc tty", NULL) == -1) { I don't know why xterm lacks third call to pledge to drop "proc". You can wait4(2) or killpg(2) from "stdio". Is there a reason?
Re: Picky, but much more efficient arc4random_uniform!
On Sun, May 15, 2022 at 08:40:19PM -0500, Luke Small wrote: > https://marc.info/?l=openbsd-tech&m=165259528425835&w=2 > > This one (which is strongly based upon my first of two versions) which I > submitted after Guenther correctly trashed version 2, doesn’t reuse any > part of the sample. It picks up a clean new bitfield upon failure. > > I think Guenther didn’t, perhaps like yourself, realize I submitted this > later program. That’s why he said it wasn’t correct. It didn’t occur to me > at the time of responding to him: “correct correct correct.” > You've had several developers tell you this is not going to go in. I'd suggest "read the room". If you want this for your own use, just keep it as a local diff. Nobody will know (or likely care). -ml > On Sun, May 15, 2022 at 7:47 PM Damien Miller wrote: > > > On Sat, 14 May 2022, Luke Small wrote: > > > > > Look at my code. I don’t even use a modulus operator. I perform hit and > > > miss with a random bitstream. > > > > > > How can I have a bias of something I don’t do? I return a bitstream which > > > meets the parameters of being a value less than the upper bound. Much > > like > > > arc4random_buf(). > > > > > > If I use arc4random_uniform() repeatedly to create a random distribution > > of > > > say numbers less than 0x1000 or even something weird like 0x1300 will the > > > random distribution be better with arc4random_uniform() or with mine? For > > > 0x1000 mine will simply pluck 12 bits of random data straight from the > > > arc4random() (and preserve the remaining 20 bits for later) on the first > > > try, just like it’s arc4random_buf(). > > > > > > arc4random_uniform() will perform a modulus of a 32 bit number which adds > > > data to the bitstream. Does it make it better? Perhaps it makes it harder > > > to guess the source bits. > > > > > > I don’t know; and I’m not going to pretend to be a cryptologist. But I’m > > > looking at modulo bias. > > > > > > I didn’t know what it was, before, but I basically “rejection sample”: > > > > > > > > https://research.kudelskisecurity.com/2020/07/28/the-definitive-guide-to-modulo-bias-and-how-to-avoid-it/ > > > > No, you aren't: > > > > > for (;;) { > > > if (rand_bits < bits) { > > > rand_holder |= ((uint64_t)arc4random()) << > > > rand_bits; > > > > > > /* > > > * rand_bits will be a number between 0 and 31 > > here > > > * so the 0x20 bit will be empty > > > * rand_bits += 32; > > > */ > > > rand_bits |= 32; > > > } > > > > > > ret = rand_holder & uuu; > > > rand_holder >>= bits; > > > rand_bits -= bits; > > > > > > if (ret < upper_bound) > > > return ret; > > > } > > > > This isn't rejection sampling. This is reusing part of the rejected > > sample. > > > > Think of it like this: you want to uniformly generate a number in the > > range [2:10] by rolling 2x 6-sided dice. What do you do when you roll > > 11 or 12? You can't just reroll one of the dice because the other dice > > is constrained to be have rolled either 5 or 6, and so proceeding with > > it would force the output to be in the range [6:11] for these ~5.6% > > of initial rolls. Your output is no longer uniform. > > > > BTW the existing code already implements the prefered approach of the > > article you quoted. > > > > -d > > -- > -Luke
Re: Picky, but much more efficient arc4random_uniform!
https://marc.info/?l=openbsd-tech&m=165259528425835&w=2 This one (which is strongly based upon my first of two versions) which I submitted after Guenther correctly trashed version 2, doesn’t reuse any part of the sample. It picks up a clean new bitfield upon failure. I think Guenther didn’t, perhaps like yourself, realize I submitted this later program. That’s why he said it wasn’t correct. It didn’t occur to me at the time of responding to him: “correct correct correct.” On Sun, May 15, 2022 at 7:47 PM Damien Miller wrote: > On Sat, 14 May 2022, Luke Small wrote: > > > Look at my code. I don’t even use a modulus operator. I perform hit and > > miss with a random bitstream. > > > > How can I have a bias of something I don’t do? I return a bitstream which > > meets the parameters of being a value less than the upper bound. Much > like > > arc4random_buf(). > > > > If I use arc4random_uniform() repeatedly to create a random distribution > of > > say numbers less than 0x1000 or even something weird like 0x1300 will the > > random distribution be better with arc4random_uniform() or with mine? For > > 0x1000 mine will simply pluck 12 bits of random data straight from the > > arc4random() (and preserve the remaining 20 bits for later) on the first > > try, just like it’s arc4random_buf(). > > > > arc4random_uniform() will perform a modulus of a 32 bit number which adds > > data to the bitstream. Does it make it better? Perhaps it makes it harder > > to guess the source bits. > > > > I don’t know; and I’m not going to pretend to be a cryptologist. But I’m > > looking at modulo bias. > > > > I didn’t know what it was, before, but I basically “rejection sample”: > > > > > https://research.kudelskisecurity.com/2020/07/28/the-definitive-guide-to-modulo-bias-and-how-to-avoid-it/ > > No, you aren't: > > > for (;;) { > > if (rand_bits < bits) { > > rand_holder |= ((uint64_t)arc4random()) << > > rand_bits; > > > > /* > > * rand_bits will be a number between 0 and 31 > here > > * so the 0x20 bit will be empty > > * rand_bits += 32; > > */ > > rand_bits |= 32; > > } > > > > ret = rand_holder & uuu; > > rand_holder >>= bits; > > rand_bits -= bits; > > > > if (ret < upper_bound) > > return ret; > > } > > This isn't rejection sampling. This is reusing part of the rejected > sample. > > Think of it like this: you want to uniformly generate a number in the > range [2:10] by rolling 2x 6-sided dice. What do you do when you roll > 11 or 12? You can't just reroll one of the dice because the other dice > is constrained to be have rolled either 5 or 6, and so proceeding with > it would force the output to be in the range [6:11] for these ~5.6% > of initial rolls. Your output is no longer uniform. > > BTW the existing code already implements the prefered approach of the > article you quoted. > > -d -- -Luke
Re: [PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
There is so much regret in this feature. xterm is supposed to be as secure as possible so why does it need to inspect all the processes in the system (that is what this feature gives xterm, as soon as you pledge "ps"). How about if xterm wasn't capable of doing that? Is it much of a loss? Who is using this? i...@b3zdna.net wrote: > The current implimintation of ProcGetCWD relies on procfs. The following > patch uses sysctl to instead retrieve the current working directory. > > This enables the use of exec-formatted xterm function on a path relative > to the terminal's current working directory. > > > ig0r < ig0r [at] b3zdna [dot] net > > > > diff --git app/xterm/Makefile app/xterm/Makefile > index ee3383094..d99a02cb3 100644 > --- app/xterm/Makefile > +++ app/xterm/Makefile > @@ -14,7 +14,7 @@ CPPFLAGS+= -I. -I${.CURDIR} -I${X11BASE}/include \ > -DHAVE_CONFIG_H -DUTMP \ > -DDEF_ALLOW_FONT=False -DDEF_ALLOW_TCAP=False \ > -DDEF_ALLOW_WINDOW=False -DDEF_ALLOW_MOUSE=False \ > - -DOPT_PRINT_ON_EXIT=0 > + -DOPT_PRINT_ON_EXIT=0 -DOPT_EXEC_XTERM=True > LDADD+= -L${X11BASE}/lib -lXaw -lXpm -lXt -lSM -lICE -lXmu \ > -lXft -lXrender -lXinerama -lX11 -lxcb -lXext -lXau -lXdmcp \ > -lfontconfig -lexpat -lfreetype -lutil -ltermcap -lz > diff --git app/xterm/main.c app/xterm/main.c > index b6e4e8f58..95f6946a5 100644 > --- app/xterm/main.c > +++ app/xterm/main.c > @@ -2908,7 +2908,7 @@ main(int argc, char *argv[]ENVP_ARG) > if (data && > (strstr(data, "exec-formatted") || strstr(data, > "exec-selectable"))) { > > -if (pledge("stdio rpath wpath id proc exec tty", NULL) == -1) { > +if (pledge("stdio rpath wpath id proc exec tty ps", NULL) == -1) > { > xtermWarning("pledge\n"); > exit(1); > } > @@ -2955,7 +2955,7 @@ main(int argc, char *argv[]ENVP_ARG) > unveil(etc_utmp, "w"); > unveil(etc_wtmp, "w"); > > -if (pledge("stdio rpath wpath id proc tty", NULL) == -1) { > +if (pledge("stdio rpath wpath id proc tty ps", NULL) == -1) { > xtermWarning("pledge\n"); > exit(1); > } > diff --git app/xterm/misc.c app/xterm/misc.c > index 89eee4880..33ada0f98 100644 > --- app/xterm/misc.c > +++ app/xterm/misc.c > @@ -99,6 +99,8 @@ > > #include > > +#include > + > #ifdef VMS > #define XTERM_VMS_LOGFILE "SYS$SCRATCH:XTERM_LOG.TXT" > #ifdef ALLOWLOGFILEEXEC > @@ -1108,14 +1110,12 @@ HandleStringEvent(Widget w GCC_UNUSED, > char * > ProcGetCWD(pid_t pid) > { > -char *child_cwd = NULL; > + char child_cwd_link[BUFSIZ]; > > -if (pid) { > - char child_cwd_link[sizeof(PROCFS_ROOT) + 80]; > - sprintf(child_cwd_link, PROCFS_ROOT "/%lu/cwd", (unsigned long) pid); > - child_cwd = Readlink(child_cwd_link); > -} > -return child_cwd; > + return pid && > + !sysctl((int[]){CTL_KERN,KERN_PROC_CWD,pid}, 3, child_cwd_link, > (size_t[]){BUFSIZ}, 0, 0) > + ? strdup(child_cwd_link) > + : NULL; > } > > /* ARGSUSED */ >
[PATCH] xenocara: change xterm ProcGetCWD to get cwd via sysctl instead of procfs
The current implimintation of ProcGetCWD relies on procfs. The following patch uses sysctl to instead retrieve the current working directory. This enables the use of exec-formatted xterm function on a path relative to the terminal's current working directory. ig0r < ig0r [at] b3zdna [dot] net > diff --git app/xterm/Makefile app/xterm/Makefile index ee3383094..d99a02cb3 100644 --- app/xterm/Makefile +++ app/xterm/Makefile @@ -14,7 +14,7 @@ CPPFLAGS+=-I. -I${.CURDIR} -I${X11BASE}/include \ -DHAVE_CONFIG_H -DUTMP \ -DDEF_ALLOW_FONT=False -DDEF_ALLOW_TCAP=False \ -DDEF_ALLOW_WINDOW=False -DDEF_ALLOW_MOUSE=False \ - -DOPT_PRINT_ON_EXIT=0 + -DOPT_PRINT_ON_EXIT=0 -DOPT_EXEC_XTERM=True LDADD+=-L${X11BASE}/lib -lXaw -lXpm -lXt -lSM -lICE -lXmu \ -lXft -lXrender -lXinerama -lX11 -lxcb -lXext -lXau -lXdmcp \ -lfontconfig -lexpat -lfreetype -lutil -ltermcap -lz diff --git app/xterm/main.c app/xterm/main.c index b6e4e8f58..95f6946a5 100644 --- app/xterm/main.c +++ app/xterm/main.c @@ -2908,7 +2908,7 @@ main(int argc, char *argv[]ENVP_ARG) if (data && (strstr(data, "exec-formatted") || strstr(data, "exec-selectable"))) { -if (pledge("stdio rpath wpath id proc exec tty", NULL) == -1) { +if (pledge("stdio rpath wpath id proc exec tty ps", NULL) == -1) { xtermWarning("pledge\n"); exit(1); } @@ -2955,7 +2955,7 @@ main(int argc, char *argv[]ENVP_ARG) unveil(etc_utmp, "w"); unveil(etc_wtmp, "w"); -if (pledge("stdio rpath wpath id proc tty", NULL) == -1) { +if (pledge("stdio rpath wpath id proc tty ps", NULL) == -1) { xtermWarning("pledge\n"); exit(1); } diff --git app/xterm/misc.c app/xterm/misc.c index 89eee4880..33ada0f98 100644 --- app/xterm/misc.c +++ app/xterm/misc.c @@ -99,6 +99,8 @@ #include +#include + #ifdef VMS #define XTERM_VMS_LOGFILE "SYS$SCRATCH:XTERM_LOG.TXT" #ifdef ALLOWLOGFILEEXEC @@ -1108,14 +1110,12 @@ HandleStringEvent(Widget w GCC_UNUSED, char * ProcGetCWD(pid_t pid) { -char *child_cwd = NULL; + char child_cwd_link[BUFSIZ]; -if (pid) { - char child_cwd_link[sizeof(PROCFS_ROOT) + 80]; - sprintf(child_cwd_link, PROCFS_ROOT "/%lu/cwd", (unsigned long) pid); - child_cwd = Readlink(child_cwd_link); -} -return child_cwd; + return pid && + !sysctl((int[]){CTL_KERN,KERN_PROC_CWD,pid}, 3, child_cwd_link, (size_t[]){BUFSIZ}, 0, 0) + ? strdup(child_cwd_link) + : NULL; } /* ARGSUSED */
Re: Picky, but much more efficient arc4random_uniform!
On Sat, 14 May 2022, Luke Small wrote: > Look at my code. I don’t even use a modulus operator. I perform hit and > miss with a random bitstream. > > How can I have a bias of something I don’t do? I return a bitstream which > meets the parameters of being a value less than the upper bound. Much like > arc4random_buf(). > > If I use arc4random_uniform() repeatedly to create a random distribution of > say numbers less than 0x1000 or even something weird like 0x1300 will the > random distribution be better with arc4random_uniform() or with mine? For > 0x1000 mine will simply pluck 12 bits of random data straight from the > arc4random() (and preserve the remaining 20 bits for later) on the first > try, just like it’s arc4random_buf(). > > arc4random_uniform() will perform a modulus of a 32 bit number which adds > data to the bitstream. Does it make it better? Perhaps it makes it harder > to guess the source bits. > > I don’t know; and I’m not going to pretend to be a cryptologist. But I’m > looking at modulo bias. > > I didn’t know what it was, before, but I basically “rejection sample”: > > https://research.kudelskisecurity.com/2020/07/28/the-definitive-guide-to-modulo-bias-and-how-to-avoid-it/ No, you aren't: > for (;;) { > if (rand_bits < bits) { > rand_holder |= ((uint64_t)arc4random()) << > rand_bits; > > /* > * rand_bits will be a number between 0 and 31 here > * so the 0x20 bit will be empty > * rand_bits += 32; > */ > rand_bits |= 32; > } > > ret = rand_holder & uuu; > rand_holder >>= bits; > rand_bits -= bits; > > if (ret < upper_bound) > return ret; > } This isn't rejection sampling. This is reusing part of the rejected sample. Think of it like this: you want to uniformly generate a number in the range [2:10] by rolling 2x 6-sided dice. What do you do when you roll 11 or 12? You can't just reroll one of the dice because the other dice is constrained to be have rolled either 5 or 6, and so proceeding with it would force the output to be in the range [6:11] for these ~5.6% of initial rolls. Your output is no longer uniform. BTW the existing code already implements the prefered approach of the article you quoted. -d
Re: Picky, but much more efficient arc4random_uniform!
On Sun, 15 May 2022, Luke Small wrote: > Do I really have to use specific terminology to make a point? > > I'm not educated enough on chacha20 enough to know whether, like I > pointed out, whether choosing 5 bits from the middle of (or even from > the tail end of one and the beginning of another) 32 bit pseudorandom > cipher is "correct." You don't need to understand chacha20 to understand. arc4random_uniform() I certainly didn't when I wrote it. The underlying CSPRNG is irrelevant to how arc4random_uniform() works. It it treated as an oracle that provides 32 random bit upon request. You could swap it out for 32 coin-tossing monkeys and the implementation wouldn't need to change. It requests another 32 bit random value for each attempt at satisfying the bounds check because they need to be independent - reusing parts of a previous attempt is highly likely to introduce biases. It's almost certainly possible to make this function faster, but it's also very easy to get it wrong (e.g. I made one stupid math error in its early implementation, forever immortalised by CVS). The existing code has the advantage of being very obvious in how it works and therefore has a very low risk of being wrong. If someone is proposing to move to something less obvious then it's incumbent upon them to do the work to prove that their alternative is just as correct. > ...correct correct correct. Did I use that buzzword enough? Highly experienced people are taking he time to give you detailed, critical feedback. This can be hard to receive, but if you ever want to improve then you should consider it and try to engage constructively. -d
Re: Picky, but much more efficient arc4random_uniform!
Do I really have to use specific terminology to make a point? I'm not educated enough on chacha20 enough to know whether, like I pointed out, whether choosing 5 bits from the middle of (or even from the tail end of one and the beginning of another) 32 bit pseudorandom cipher is "correct." ...correct correct correct. Did I use that buzzword enough? -Luke On Sun, May 15, 2022 at 5:26 PM Philip Guenther wrote: > On Sun, 15 May 2022, Luke Small wrote: > > The current implementation is nothing more than a naive arc4random() % > > upper_bound which trashes initial arc4random() calls it doesn’t like, > then > > transforms over a desired modulus. The whole transformation by modulus of > > perfectly decent random data seems so awkward. It’s not like it is used > as > > some majestic artistry of RSA it seems like an ugly HACK to simply meet a > > demand lacking of something better. > > You fail to mention correctness at all or address the fact that your > version isn't while the current one is. Meanwhile, you talk about getting > only just enough random data as if there's some sort of limited supply > when there isn't. > > "My version may be wrong, but at least it doesn't look naive!" > > That is utterly the wrong attitude for OpenBSD code. > > > Best wishes. > > Philip Guenther >
Re: Picky, but much more efficient arc4random_uniform!
On Sun, 15 May 2022, Luke Small wrote: > The current implementation is nothing more than a naive arc4random() % > upper_bound which trashes initial arc4random() calls it doesn’t like, then > transforms over a desired modulus. The whole transformation by modulus of > perfectly decent random data seems so awkward. It’s not like it is used as > some majestic artistry of RSA it seems like an ugly HACK to simply meet a > demand lacking of something better. You fail to mention correctness at all or address the fact that your version isn't while the current one is. Meanwhile, you talk about getting only just enough random data as if there's some sort of limited supply when there isn't. "My version may be wrong, but at least it doesn't look naive!" That is utterly the wrong attitude for OpenBSD code. Best wishes. Philip Guenther
Re: have in6_pcbselsrc copy the selected ip to the caller instead of a reference
On Sun, May 15, 2022 at 08:19:47PM +1000, David Gwynne wrote: > this is basically the same as what i did for in_pcbselsrc, and > completely mechanical. im too tired to figure out if there's a smarter > way to do it. > > lightly tested, and more eyes are welcome because of the tiredness > thing. OK bluhm@ > Index: in6_pcb.c > === > RCS file: /cvs/src/sys/netinet6/in6_pcb.c,v > retrieving revision 1.117 > diff -u -p -r1.117 in6_pcb.c > --- in6_pcb.c 14 Apr 2022 14:10:22 - 1.117 > +++ in6_pcb.c 15 May 2022 09:53:53 - > @@ -235,7 +235,7 @@ in6_pcbaddrisavail(struct inpcb *inp, st > int > in6_pcbconnect(struct inpcb *inp, struct mbuf *nam) > { > - struct in6_addr *in6a = NULL; > + struct in6_addr in6a; > struct sockaddr_in6 *sin6; > int error; > struct sockaddr_in6 tmp; > @@ -273,7 +273,8 @@ in6_pcbconnect(struct inpcb *inp, struct > inp->inp_ipv6.ip6_hlim = (u_int8_t)in6_selecthlim(inp); > > if (in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr, sin6->sin6_port, > - IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6) ? in6a : &inp->inp_laddr6, > + IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6) ? > + &in6a : &inp->inp_laddr6, > inp->inp_lport, inp->inp_rtableid) != NULL) { > return (EADDRINUSE); > } > @@ -286,13 +287,13 @@ in6_pcbconnect(struct inpcb *inp, struct > if (error) > return (error); > if (in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr, > - sin6->sin6_port, in6a, inp->inp_lport, > + sin6->sin6_port, &in6a, inp->inp_lport, > inp->inp_rtableid) != NULL) { > inp->inp_lport = 0; > return (EADDRINUSE); > } > } > - inp->inp_laddr6 = *in6a; > + inp->inp_laddr6 = in6a; > } > inp->inp_faddr6 = sin6->sin6_addr; > inp->inp_fport = sin6->sin6_port; > Index: in6_src.c > === > RCS file: /cvs/src/sys/netinet6/in6_src.c,v > retrieving revision 1.86 > diff -u -p -r1.86 in6_src.c > --- in6_src.c 22 Feb 2022 01:15:02 - 1.86 > +++ in6_src.c 15 May 2022 09:53:53 - > @@ -91,7 +91,7 @@ int in6_selectif(struct sockaddr_in6 *, > * the values set at pcb level can be overridden via cmsg. > */ > int > -in6_pcbselsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock, > +in6_pcbselsrc(struct in6_addr *in6src, struct sockaddr_in6 *dstsock, > struct inpcb *inp, struct ip6_pktopts *opts) > { > struct ip6_moptions *mopts = inp->inp_moptions6; > @@ -138,7 +138,7 @@ in6_pcbselsrc(struct in6_addr **in6src, > > pi->ipi6_addr = sa6.sin6_addr; /* XXX: this overrides pi */ > > - *in6src = &pi->ipi6_addr; > + *in6src = pi->ipi6_addr; > return (0); > } > > @@ -147,7 +147,7 @@ in6_pcbselsrc(struct in6_addr **in6src, >* is already bound, use the bound address. >*/ > if (laddr && !IN6_IS_ADDR_UNSPECIFIED(laddr)) { > - *in6src = laddr; > + *in6src = *laddr; > return (0); > } > > @@ -167,7 +167,7 @@ in6_pcbselsrc(struct in6_addr **in6src, > if (ia6 == NULL) > return (EADDRNOTAVAIL); > > - *in6src = &ia6->ia_addr.sin6_addr; > + *in6src = ia6->ia_addr.sin6_addr; > return (0); > } > > @@ -229,7 +229,7 @@ in6_pcbselsrc(struct in6_addr **in6src, > struct ifaddr *ifa; > if ((ifa = ifa_ifwithaddr(ip6_source, rtableid)) != > NULL && ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) { > - *in6src = &satosin6(ip6_source)->sin6_addr; > + *in6src = satosin6(ip6_source)->sin6_addr; > return (0); > } > } > @@ -238,7 +238,7 @@ in6_pcbselsrc(struct in6_addr **in6src, > if (ia6 == NULL) > return (EHOSTUNREACH); /* no route */ > > - *in6src = &ia6->ia_addr.sin6_addr; > + *in6src = ia6->ia_addr.sin6_addr; > return (0); > } > > @@ -249,7 +249,7 @@ in6_pcbselsrc(struct in6_addr **in6src, > * an entry to the caller for later use. > */ > int > -in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock, > +in6_selectsrc(struct in6_addr *in6src, struct sockaddr_in6 *dstsock, > struct ip6_moptions *mopts, unsigned int rtableid) > { > struct ifnet *ifp = NULL; > @@ -279,7 +279,7 @@ in6_selectsrc(struct in6_addr **in6src, > if (ia6 == NULL) > return (EADDRNOTAVAIL); > > - *in6src = &ia6->ia_addr.sin6_addr; > +
OpenBSD Errata: May 16, 2022 (kqueue asn1 pppoe)
Errata patches for kqueue in the kernel and asn1 in libcrypto have been released for OpenBSD 7.1. Errata patches for pppoe in the kernel have been released for OpenBSD 7.1 and 7.0. Binary updates for the amd64, i386 and arm64 platform are available via the syspatch utility. Source code patches can be found on the respective errata page: https://www.openbsd.org/errata71.html https://www.openbsd.org/errata70.html
Re: Picky, but much more efficient arc4random_uniform!
The current implementation is nothing more than a naive arc4random() % upper_bound which trashes initial arc4random() calls it doesn’t like, then transforms over a desired modulus. The whole transformation by modulus of perfectly decent random data seems so awkward. It’s not like it is used as some majestic artistry of RSA it seems like an ugly HACK to simply meet a demand lacking of something better. If you understand what I’ve done, it streams in a bitfield into an integer type like it’s a buffer for just enough or slightly more data to meet the demands of the upperbound and if it exceeds upperbound-1, it is trashed and reads in a completely new bitfield to check. It relies on arc4random() supplying good random data regardless of how many bits are in the bitfield. If it does so, it should and seems to supply a good distribution across the length of the bitfield which may often be something like 5 for a common 26+26+10 upper_bound in /usr/src. It seems to me that it should be pretty good if not superior method; at least in the realm of cleaner results. Perhaps it’s confusing what I’ve done with all the bitwise operators, but it isn’t some random hacky thing I’ve cobbled together. Or does arc4random() only provide decent random data 32 bits at a time; or an even 8 bits at a time as arc4random_buf() would suggest? All I would have to prove is that chacha20 provides good or superior random bitfields regardless of how many bits are needed and regardless of whether they begin at the beginning of a byte. I don’t have the education for that, but “I got a ‘puter for Christmas.” lol. I can perhaps run simulations if I have nothing better to do. > I think I can say we know here uniformity is only *one* of the > desirable properties of a secure random generator. > > But I don't think anybody else execpt Luke was talking about > "improving". The sole purpose of arc4random_uniform() is to give a > good implementation of a random number function in a specific range > using arc4random() as the source. This is needed because the naive > implementation arc4random() % upper_bound is not uniform. > > -Otto > > > -- -Luke
Re: use cpu sensor for cpuspeed
On 2022-05-15, Mark Kettenis wrote: > > From: "Ted Unangst" > > Date: Sat, 14 May 2022 20:23:39 -0400 > > > > The cpu hz sensor is more accurate and updates faster than than the value > > currently used for hw.cpuspeed. So return that value (scaled). > > > > This doesn't set cpuspeed directly because the acpi does that and it's hard > > to create a whole system of priority overrides. I think it's simpler and > > maybe even better to track every value, and then return the best one > > available. > > At this point, I don't want this. Right now, it is actually useful > that hw.cpuspeed represents the requested P-state whereas the sensors > give us the "effective" frequency the core is actually running at. > And that is useful for evaluating what is actually happening. The man page should be updated then. HW_CPUSPEED (hw.cpuspeed) The current CPU frequency (in MHz).
Re: Picky, but much more efficient arc4random_uniform!
On Sun, May 15, 2022 at 08:57:09AM -0300, Crystal Kolipe wrote: > On Sun, May 15, 2022 at 11:44:29AM +0200, Otto Moerbeek wrote: > > On Sun, May 15, 2022 at 04:27:30AM -0500, Luke Small wrote: > > > How did someone prove the current implementation was cryptographically > > > sound? Did they run massive simulations which ran the gamut of the > > > uint32_t > > > range which demanded tight tolerances over varying length runs? > > > > > > How was rc4 cipher proven bad for pseudorandom numbers? I???d be willing > > > to > > > bet it wasn???t from a mathematical proof; it was from bad data. > > > You miss the point completely. The point is: how do you derive a > > uniformly distributed random function for a smaller range, given a > > uniformly distibuted random function over the range [0-2^32-1]. > > > > The current implementation does exactly that and has all the > > information in the comments to verify the uniformity claim. You only > > need to use basic mathematical properties of modulo arithmetic to > > do the verification. > > You do all realise that uniform distribution alone does not make a > good random number generator, don't you? > > For example: > > 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5... > > That's uniformly distributed, and also useless as a random number > stream. > > Further more, _cryptographically secure_ random number generation is > not the same as _mathematically good_ random number generation. > > There are plenty of random number generation formulas which are > considered good and useful from a mathematical basis, but which are > useless for cryptography. > > So random, (pun intended), hacks at the arc4random code are not > likely to 'improve' it from the general standpoint, (although if you > have a specific need for a specific private application, that's > different). I think Stuart has already more or less made that point. > I think I can say we know here uniformity is only *one* of the desirable properties of a secure random generator. But I don't think anybody else execpt Luke was talking about "improving". The sole purpose of arc4random_uniform() is to give a good implementation of a random number function in a specific range using arc4random() as the source. This is needed because the naive implementation arc4random() % upper_bound is not uniform. -Otto
Re: Picky, but much more efficient arc4random_uniform!
On Sun, May 15, 2022 at 11:44:29AM +0200, Otto Moerbeek wrote: > On Sun, May 15, 2022 at 04:27:30AM -0500, Luke Small wrote: > > How did someone prove the current implementation was cryptographically > > sound? Did they run massive simulations which ran the gamut of the uint32_t > > range which demanded tight tolerances over varying length runs? > > > > How was rc4 cipher proven bad for pseudorandom numbers? I???d be willing to > > bet it wasn???t from a mathematical proof; it was from bad data. > You miss the point completely. The point is: how do you derive a > uniformly distributed random function for a smaller range, given a > uniformly distibuted random function over the range [0-2^32-1]. > > The current implementation does exactly that and has all the > information in the comments to verify the uniformity claim. You only > need to use basic mathematical properties of modulo arithmetic to > do the verification. You do all realise that uniform distribution alone does not make a good random number generator, don't you? For example: 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5... That's uniformly distributed, and also useless as a random number stream. Further more, _cryptographically secure_ random number generation is not the same as _mathematically good_ random number generation. There are plenty of random number generation formulas which are considered good and useful from a mathematical basis, but which are useless for cryptography. So random, (pun intended), hacks at the arc4random code are not likely to 'improve' it from the general standpoint, (although if you have a specific need for a specific private application, that's different). I think Stuart has already more or less made that point.
Re: Picky, but much more efficient arc4random_uniform!
Random generators have been analyzed for years. Pick up "Concrete mathematics" by Don E. Knuth, read it, then come back with actual science.
have in6_pcbselsrc copy the selected ip to the caller instead of a reference
this is basically the same as what i did for in_pcbselsrc, and completely mechanical. im too tired to figure out if there's a smarter way to do it. lightly tested, and more eyes are welcome because of the tiredness thing. Index: in6_pcb.c === RCS file: /cvs/src/sys/netinet6/in6_pcb.c,v retrieving revision 1.117 diff -u -p -r1.117 in6_pcb.c --- in6_pcb.c 14 Apr 2022 14:10:22 - 1.117 +++ in6_pcb.c 15 May 2022 09:53:53 - @@ -235,7 +235,7 @@ in6_pcbaddrisavail(struct inpcb *inp, st int in6_pcbconnect(struct inpcb *inp, struct mbuf *nam) { - struct in6_addr *in6a = NULL; + struct in6_addr in6a; struct sockaddr_in6 *sin6; int error; struct sockaddr_in6 tmp; @@ -273,7 +273,8 @@ in6_pcbconnect(struct inpcb *inp, struct inp->inp_ipv6.ip6_hlim = (u_int8_t)in6_selecthlim(inp); if (in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr, sin6->sin6_port, - IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6) ? in6a : &inp->inp_laddr6, + IN6_IS_ADDR_UNSPECIFIED(&inp->inp_laddr6) ? +&in6a : &inp->inp_laddr6, inp->inp_lport, inp->inp_rtableid) != NULL) { return (EADDRINUSE); } @@ -286,13 +287,13 @@ in6_pcbconnect(struct inpcb *inp, struct if (error) return (error); if (in6_pcbhashlookup(inp->inp_table, &sin6->sin6_addr, - sin6->sin6_port, in6a, inp->inp_lport, + sin6->sin6_port, &in6a, inp->inp_lport, inp->inp_rtableid) != NULL) { inp->inp_lport = 0; return (EADDRINUSE); } } - inp->inp_laddr6 = *in6a; + inp->inp_laddr6 = in6a; } inp->inp_faddr6 = sin6->sin6_addr; inp->inp_fport = sin6->sin6_port; Index: in6_src.c === RCS file: /cvs/src/sys/netinet6/in6_src.c,v retrieving revision 1.86 diff -u -p -r1.86 in6_src.c --- in6_src.c 22 Feb 2022 01:15:02 - 1.86 +++ in6_src.c 15 May 2022 09:53:53 - @@ -91,7 +91,7 @@ int in6_selectif(struct sockaddr_in6 *, * the values set at pcb level can be overridden via cmsg. */ int -in6_pcbselsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock, +in6_pcbselsrc(struct in6_addr *in6src, struct sockaddr_in6 *dstsock, struct inpcb *inp, struct ip6_pktopts *opts) { struct ip6_moptions *mopts = inp->inp_moptions6; @@ -138,7 +138,7 @@ in6_pcbselsrc(struct in6_addr **in6src, pi->ipi6_addr = sa6.sin6_addr; /* XXX: this overrides pi */ - *in6src = &pi->ipi6_addr; + *in6src = pi->ipi6_addr; return (0); } @@ -147,7 +147,7 @@ in6_pcbselsrc(struct in6_addr **in6src, * is already bound, use the bound address. */ if (laddr && !IN6_IS_ADDR_UNSPECIFIED(laddr)) { - *in6src = laddr; + *in6src = *laddr; return (0); } @@ -167,7 +167,7 @@ in6_pcbselsrc(struct in6_addr **in6src, if (ia6 == NULL) return (EADDRNOTAVAIL); - *in6src = &ia6->ia_addr.sin6_addr; + *in6src = ia6->ia_addr.sin6_addr; return (0); } @@ -229,7 +229,7 @@ in6_pcbselsrc(struct in6_addr **in6src, struct ifaddr *ifa; if ((ifa = ifa_ifwithaddr(ip6_source, rtableid)) != NULL && ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) { - *in6src = &satosin6(ip6_source)->sin6_addr; + *in6src = satosin6(ip6_source)->sin6_addr; return (0); } } @@ -238,7 +238,7 @@ in6_pcbselsrc(struct in6_addr **in6src, if (ia6 == NULL) return (EHOSTUNREACH); /* no route */ - *in6src = &ia6->ia_addr.sin6_addr; + *in6src = ia6->ia_addr.sin6_addr; return (0); } @@ -249,7 +249,7 @@ in6_pcbselsrc(struct in6_addr **in6src, * an entry to the caller for later use. */ int -in6_selectsrc(struct in6_addr **in6src, struct sockaddr_in6 *dstsock, +in6_selectsrc(struct in6_addr *in6src, struct sockaddr_in6 *dstsock, struct ip6_moptions *mopts, unsigned int rtableid) { struct ifnet *ifp = NULL; @@ -279,7 +279,7 @@ in6_selectsrc(struct in6_addr **in6src, if (ia6 == NULL) return (EADDRNOTAVAIL); - *in6src = &ia6->ia_addr.sin6_addr; + *in6src = ia6->ia_addr.sin6_addr; return (0); } @@ -303,7 +303,7 @@ in6_selectsrc(struct in6_addr **in6src, if (ia6 == NULL)
Re: Picky, but much more efficient arc4random_uniform!
On Sun, May 15, 2022 at 04:27:30AM -0500, Luke Small wrote: > I have a feeling that making it threadsafe under any quantity of threads > and still perform is likely not feasible, but if I could; or if making a > nonthreadsafe variant was possible: > > Creating a mathematical proof that somehow is “simple and probably correct” > enough, would be an impossible task even for a PhD mathematician. > > How did someone prove the current implementation was cryptographically > sound? Did they run massive simulations which ran the gamut of the uint32_t > range which demanded tight tolerances over varying length runs? > > How was rc4 cipher proven bad for pseudorandom numbers? I’d be willing to > bet it wasn’t from a mathematical proof; it was from bad data. > > I’m guessing that large upperbounds approaching 2**32 don’t feed very > soundly in the current implementation using a modulus; although I suspect > that there isn’t much of a call for such things, I’m pretty sure I saw a > 3,000,000,000 upperbound in the src partition tonight. You miss the point completely. The point is: how do you derive a uniformly distributed random function for a smaller range, given a uniformly distibuted random function over the range [0-2^32-1]. The current implementation does exactly that and has all the information in the comments to verify the uniformity claim. You only need to use basic mathematical properties of modulo arithmetic to do the verification. -Otto > > On Sun, May 15, 2022 at 3:15 AM Otto Moerbeek wrote: > > > On Sun, May 15, 2022 at 01:12:28AM -0500, Luke Small wrote: > > > > > This is version 1, which I was pretty sure was secure. > > > > > > I revamped it with a few features and implanted the binary search for > > 'bit' > > > > > > in most cases, which aren't intentionally worst-case, it's pretty darned > > > fast! > > > > > > This is a sample run of my program with your piece of code included: > > > > > > > > > 1 99319 100239 > > > 2 100353 99584 > > > 3 100032 99879 > > > 4 99704 100229 > > > 5 99530 99796 > > > 6 99617 100355 > > > 7 100490 100319 > > > 8 99793 100114 > > > 9 100426 99744 > > > 10 100315 100558 > > > 11 99279 99766 > > > 12 99572 99750 > > > 13 99955 100017 > > > 14 100413 15 > > > 15 100190 100052 > > > 16 101071 100195 > > > 17 100322 100224 > > > 18 99637 99540 > > > 19 100323 99251 > > > 20 99841 100177 > > > 21 99948 99504 > > > 22 100065 100031 > > > 23 100026 99827 > > > 24 99836 99818 > > > 25 100245 99822 > > > 26 100088 99678 > > > 27 99957 3 > > > 28 100065 99961 > > > 29 100701 100679 > > > 30 99756 99587 > > > 31 100220 100076 > > > 32 100067 15 > > > 33 99547 99984 > > > 34 100124 100031 > > > 35 99547 100661 > > > 36 99801 99963 > > > 37 100189 100230 > > > 38 99878 99579 > > > 39 99864 99442 > > > 40 99683 14 > > > 41 99907 100094 > > > 42 100291 99817 > > > 43 99908 99984 > > > 44 100044 100606 > > > 45 100065 100120 > > > 46 99358 100141 > > > 47 100152 100442 > > > 48 10 100279 > > > 49 100486 99848 > > > > Sadly a sample run cannot be used to proof your implementation is > > correct. It can only be used to show it is not correct, like Philip > > did. To show your implementation produces uniform results in all > > cases, you need to provide a solid argumentation that is easy to > > follow. So far you failed to do that and I do not see it coming, given > > the complexituy of your implementation. The current implementation > > has a straightforward argumentation as it uses relatively simple > > mathematical properties of modulo arithmetic. > > > > It is also clear your code (as it uses statics) is not thread safe. > > > > So to answer you original question clearly: no, we will not accept this. > > > > -Otto > > > -- > -Luke
Re: Picky, but much more efficient arc4random_uniform!
I have a feeling that making it threadsafe under any quantity of threads and still perform is likely not feasible, but if I could; or if making a nonthreadsafe variant was possible: Creating a mathematical proof that somehow is “simple and probably correct” enough, would be an impossible task even for a PhD mathematician. How did someone prove the current implementation was cryptographically sound? Did they run massive simulations which ran the gamut of the uint32_t range which demanded tight tolerances over varying length runs? How was rc4 cipher proven bad for pseudorandom numbers? I’d be willing to bet it wasn’t from a mathematical proof; it was from bad data. I’m guessing that large upperbounds approaching 2**32 don’t feed very soundly in the current implementation using a modulus; although I suspect that there isn’t much of a call for such things, I’m pretty sure I saw a 3,000,000,000 upperbound in the src partition tonight. On Sun, May 15, 2022 at 3:15 AM Otto Moerbeek wrote: > On Sun, May 15, 2022 at 01:12:28AM -0500, Luke Small wrote: > > > This is version 1, which I was pretty sure was secure. > > > > I revamped it with a few features and implanted the binary search for > 'bit' > > > > in most cases, which aren't intentionally worst-case, it's pretty darned > > fast! > > > > This is a sample run of my program with your piece of code included: > > > > > > 1 99319 100239 > > 2 100353 99584 > > 3 100032 99879 > > 4 99704 100229 > > 5 99530 99796 > > 6 99617 100355 > > 7 100490 100319 > > 8 99793 100114 > > 9 100426 99744 > > 10 100315 100558 > > 11 99279 99766 > > 12 99572 99750 > > 13 99955 100017 > > 14 100413 15 > > 15 100190 100052 > > 16 101071 100195 > > 17 100322 100224 > > 18 99637 99540 > > 19 100323 99251 > > 20 99841 100177 > > 21 99948 99504 > > 22 100065 100031 > > 23 100026 99827 > > 24 99836 99818 > > 25 100245 99822 > > 26 100088 99678 > > 27 99957 3 > > 28 100065 99961 > > 29 100701 100679 > > 30 99756 99587 > > 31 100220 100076 > > 32 100067 15 > > 33 99547 99984 > > 34 100124 100031 > > 35 99547 100661 > > 36 99801 99963 > > 37 100189 100230 > > 38 99878 99579 > > 39 99864 99442 > > 40 99683 14 > > 41 99907 100094 > > 42 100291 99817 > > 43 99908 99984 > > 44 100044 100606 > > 45 100065 100120 > > 46 99358 100141 > > 47 100152 100442 > > 48 10 100279 > > 49 100486 99848 > > Sadly a sample run cannot be used to proof your implementation is > correct. It can only be used to show it is not correct, like Philip > did. To show your implementation produces uniform results in all > cases, you need to provide a solid argumentation that is easy to > follow. So far you failed to do that and I do not see it coming, given > the complexituy of your implementation. The current implementation > has a straightforward argumentation as it uses relatively simple > mathematical properties of modulo arithmetic. > > It is also clear your code (as it uses statics) is not thread safe. > > So to answer you original question clearly: no, we will not accept this. > > -Otto > -- -Luke
Re: use cpu sensor for cpuspeed
On Sun, May 15, 2022 at 10:35:43AM +0200, Mark Kettenis wrote: > > From: "Ted Unangst" > > Date: Sat, 14 May 2022 20:23:39 -0400 > > > > The cpu hz sensor is more accurate and updates faster than than the value > > currently used for hw.cpuspeed. So return that value (scaled). > > > > This doesn't set cpuspeed directly because the acpi does that and it's hard > > to create a whole system of priority overrides. I think it's simpler and > > maybe even better to track every value, and then return the best one > > available. > > At this point, I don't want this. Right now, it is actually useful > that hw.cpuspeed represents the requested P-state whereas the sensors > give us the "effective" frequency the core is actually running at. > And that is useful for evaluating what is actually happening. I agree, I think both values are needed for scheduler and power management changes. The next bit on the todo list is RAPL support so the energy consumtion can be measured and shown. > > Index: identcpu.c > > === > > RCS file: /home/cvs/src/sys/arch/amd64/amd64/identcpu.c,v > > retrieving revision 1.124 > > diff -u -p -r1.124 identcpu.c > > --- identcpu.c 26 Apr 2022 10:48:20 - 1.124 > > +++ identcpu.c 15 May 2022 00:15:39 - > > @@ -64,6 +64,7 @@ void cpu_check_vmm_cap(struct cpu_info * > > /* sysctl wants this. */ > > char cpu_model[48]; > > int cpuspeed; > > +uint64_t sensorspeed; > > > > int amd64_has_xcrypt; > > #ifdef CRYPTO > > @@ -244,6 +245,8 @@ int > > cpu_amd64speed(int *freq) > > { > > *freq = cpuspeed; > > + if (sensorspeed) > > + *freq = sensorspeed / 1ULL; > > return (0); > > } > > > > @@ -337,6 +340,8 @@ cpu_hz_update_sensor(void *args) > > val = (adelta * 100) / mdelta * tsc_frequency; > > val = ((val + FREQ_50MHZ / 2) / FREQ_50MHZ) * FREQ_50MHZ; > > ci->ci_hz_sensor.value = val; > > + if (CPU_IS_PRIMARY(ci)) > > + sensorspeed = val; > > } > > > > atomic_clearbits_int(&curproc->p_flag, P_CPUPEG); > > > > > -- :wq Claudio
Re: use cpu sensor for cpuspeed
> From: "Ted Unangst" > Date: Sat, 14 May 2022 20:23:39 -0400 > > The cpu hz sensor is more accurate and updates faster than than the value > currently used for hw.cpuspeed. So return that value (scaled). > > This doesn't set cpuspeed directly because the acpi does that and it's hard > to create a whole system of priority overrides. I think it's simpler and > maybe even better to track every value, and then return the best one > available. At this point, I don't want this. Right now, it is actually useful that hw.cpuspeed represents the requested P-state whereas the sensors give us the "effective" frequency the core is actually running at. And that is useful for evaluating what is actually happening. > Index: identcpu.c > === > RCS file: /home/cvs/src/sys/arch/amd64/amd64/identcpu.c,v > retrieving revision 1.124 > diff -u -p -r1.124 identcpu.c > --- identcpu.c26 Apr 2022 10:48:20 - 1.124 > +++ identcpu.c15 May 2022 00:15:39 - > @@ -64,6 +64,7 @@ voidcpu_check_vmm_cap(struct cpu_info * > /* sysctl wants this. */ > char cpu_model[48]; > int cpuspeed; > +uint64_t sensorspeed; > > int amd64_has_xcrypt; > #ifdef CRYPTO > @@ -244,6 +245,8 @@ int > cpu_amd64speed(int *freq) > { > *freq = cpuspeed; > + if (sensorspeed) > + *freq = sensorspeed / 1ULL; > return (0); > } > > @@ -337,6 +340,8 @@ cpu_hz_update_sensor(void *args) > val = (adelta * 100) / mdelta * tsc_frequency; > val = ((val + FREQ_50MHZ / 2) / FREQ_50MHZ) * FREQ_50MHZ; > ci->ci_hz_sensor.value = val; > + if (CPU_IS_PRIMARY(ci)) > + sensorspeed = val; > } > > atomic_clearbits_int(&curproc->p_flag, P_CPUPEG); > >
Re: Picky, but much more efficient arc4random_uniform!
On Sun, May 15, 2022 at 01:12:28AM -0500, Luke Small wrote: > This is version 1, which I was pretty sure was secure. > > I revamped it with a few features and implanted the binary search for 'bit' > > in most cases, which aren't intentionally worst-case, it's pretty darned > fast! > > This is a sample run of my program with your piece of code included: > > > 1 99319 100239 > 2 100353 99584 > 3 100032 99879 > 4 99704 100229 > 5 99530 99796 > 6 99617 100355 > 7 100490 100319 > 8 99793 100114 > 9 100426 99744 > 10 100315 100558 > 11 99279 99766 > 12 99572 99750 > 13 99955 100017 > 14 100413 15 > 15 100190 100052 > 16 101071 100195 > 17 100322 100224 > 18 99637 99540 > 19 100323 99251 > 20 99841 100177 > 21 99948 99504 > 22 100065 100031 > 23 100026 99827 > 24 99836 99818 > 25 100245 99822 > 26 100088 99678 > 27 99957 3 > 28 100065 99961 > 29 100701 100679 > 30 99756 99587 > 31 100220 100076 > 32 100067 15 > 33 99547 99984 > 34 100124 100031 > 35 99547 100661 > 36 99801 99963 > 37 100189 100230 > 38 99878 99579 > 39 99864 99442 > 40 99683 14 > 41 99907 100094 > 42 100291 99817 > 43 99908 99984 > 44 100044 100606 > 45 100065 100120 > 46 99358 100141 > 47 100152 100442 > 48 10 100279 > 49 100486 99848 Sadly a sample run cannot be used to proof your implementation is correct. It can only be used to show it is not correct, like Philip did. To show your implementation produces uniform results in all cases, you need to provide a solid argumentation that is easy to follow. So far you failed to do that and I do not see it coming, given the complexituy of your implementation. The current implementation has a straightforward argumentation as it uses relatively simple mathematical properties of modulo arithmetic. It is also clear your code (as it uses statics) is not thread safe. So to answer you original question clearly: no, we will not accept this. -Otto