On Fri, Aug 15, 2014 at 06:55:49PM +0200, Alexander Hall wrote:
| On 08/15/14 16:22, Paul de Weerd wrote:
| >On Fri, Aug 15, 2014 at 04:07:21PM +0200, Alexander Hall wrote:
| >| On August 15, 2014 2:04:56 PM CEST, Theo de Raadt 
<dera...@cvs.openbsd.org> wrote:
| >| >> Is it safe to generate some randomness in /tftpboot/etc/random.seed
| >| >for
| >| >> clients that PXE boot?
| >| >
| >| >I do not even know if that file will be read... is it?
| >|
| >| IIRC, it is tried but deemed unsafe (0555) and therefore isn't used, but 
causes a warning. Maybe it had changed since.
| >
| >What do you mean?  You don't get permissions with a TFTP transfer.  If
| >the tftpd can read the file, it'll send it to you.  If it can't, it
| >won't (surprise, surprise ;).

Ah, I see what you mean.  All files transferred via tftp are
considered to have 0444 permissions by the requesting process (quite a
sane approach) and therefore not accepted (not so sure about that part
in this case).

I'm guessing the reasoning not to trust world writable files is to
prevent attackers from putting with known contents into the mix.  I'm
not sure if that's any worse than not having anything at all (and the
contents of that buffer thereby being predictable, perhaps?  not even
sure whether that's true...)

At any rate, this changes that to allow world readable files (still
not taking world writable files).  We can't check S_IWOTH over tftp,
we should probably assume 0777 for files transferred that way.  But,
if you're trusting the kernel you're getting over tftp, then why the
hell are you not trusting random.seed?  That attacker that could maybe
influence your randomness would NEVER touch your kernel to ensure it
only produces well known (to them) randomness.  That would be way too
easy...

Note that this diff doesn't fix the repeated attempts at loading the
seed file.  Still don't grok what's going on there.

Index: boot.c
===================================================================
RCS file: /cvs/src/sys/stand/boot/boot.c,v
retrieving revision 1.43
diff -u -p -r1.43 boot.c
--- boot.c      19 Feb 2014 22:02:15 -0000      1.43
+++ boot.c      15 Aug 2014 21:41:01 -0000
@@ -153,7 +153,7 @@ loadrandom(char *name, char *buf, size_t
        }
        if (fstat(fd, &sb) == -1 ||
            sb.st_uid != 0 ||
-           (sb.st_mode & (S_IWOTH|S_IROTH)))
+           (sb.st_mode & (S_IWOTH)))
                goto fail;
        (void) read(fd, buf, buflen);
 fail:

| From sys/lib/libsa/tftp.c:
| 
| ...
| int
| tftp_stat(struct open_file *f, struct stat *sb)
| {
|       struct tftp_handle *tftpfile;
|       tftpfile = (struct tftp_handle *) f->f_fsdata;
| 
|       sb->st_mode = 0444;
| ...
| 
| Ok, it wasn't 0555, but 0444. Close enough.
| 
| Then, in sys/stand/boot/boot.c:
| 
| ...
| void
| loadrandom(char *name, char *buf, size_t buflen)
| {
| ...
|       if (fstat(fd, &sb) == -1 ||
|           sb.st_uid != 0 ||
|           (sb.st_mode & (S_IWOTH|S_IROTH)))
|               goto fail;
| ...
| 
| So, actually, if the file exists, it will not emit a warning (like it does
| if it's non-existent), but silently ignore it. :-)
| 
| /Alexander
| 

-- 
>++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
+++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
                 http://www.weirdnet.nl/                 

Reply via email to