[minor] umask 400 causes here-{doc,string} failure

[Stephane Chazelas]

Note: sent to bash, zsh and Schily Bourne shell dev mailing
lists (not mksh as my email provider apparently doesn't play
well with mirbsd.org's expensive greylisting, please feel free
to forward there if you don't use gmail).

That's from:
https://unix.stackexchange.com/questions/429285/cannot-create-temp-file-for-here-document-permission-denied

$ bash -c 'umask 400; cat <<< test'
bash: cannot create temp file for here-document: Permission denied
$ zsh -c 'umask 400; cat <<< test'
zsh:1: can't create temp file for here document: permission denied
$ bosh -c 'umask 400; cat << EOF
test
EOF'
bosh: /tmp/sh193220: cannot open
$ mksh -c 'umask 400; cat <<< test'
mksh: can't open temporary file /tmp/sh933f2z.tmp: Permission denied

Those shells use temporary files to store the content of the
here-documents as the Bourne shell initially did, and open them
in read-only mode to make it cat's stdin.

When umask contains the 0400 bit, the file is created without
read permission to the user, hence the error upon that second
open().

(note that bosh also leaves the temp file behind in that
case).

I can think of several ways to address it:

1- do nothing and blame the user as the user explicitly asked
for files to be unreadable (but then again, it's not obvious
to the user that heredocs imply a temp file)

2- do like AT&T ksh/tcsh (or yash for big heredocs that don't
fit in the pipe buffer) and open the file only once for both
writing the content and making it the command's stdin (with a
lseek() to beginning in between). That means the fd ends up
being writable though I can't see it being a huge problem. (Yash
actually gives the file 000 permissions here regardless of the
umask with open("/tmp/yash-ECCFE6268", O_RDWR|O_CREAT|O_EXCL,
0), but see below about =(...) emulation)

3. do like dash/yash/rc/es and use a pipe instead of a temp
file. That means having to fork a process to feed the data (or
like yash fall back to a temp file for big heredocs). That also
means the fd is no longer seekable

The change could break some scripts for bash, as on Linux (where
/dev/fd/n behaves differently from other *nices), we see some
doing:

cmd1 /dev/fd/3 3<<< "$(cmd2)"

to emulate zsh's cmd1 =(cmd2) (command substitution using a temp
file). (A 0400 umask also makes a =(...) file unreadable, but
definitely here it's the user's problem).

4. Reset the umask temporarily to 077 before creating the temp
file (and block trapped signals until it's restored).


2 would have my preference.

[Ilkka Virta]

Post by Stephane Chazelas
$ bash -c 'umask 400; cat <<< test'
bash: cannot create temp file for here-document: Permission denied
Those shells use temporary files to store the content of the
here-documents as the Bourne shell initially did, and open them
in read-only mode to make it cat's stdin.
When umask contains the 0400 bit, the file is created without
read permission to the user, hence the error upon that second
open().
1- do nothing and blame the user
2- open the file only once for both
writing the content and making it the command's stdin
3. use a pipe instead of a temp file
4. Reset the umask temporarily to 077

One more came to mind:

5. manually chmod() the tempfile to 0400 or 0600 if the open() for
reading fails with EACCES, and then retry. Should be doable with a
localized change to that particular error condition, without changing
the overall behaviour.

Post by Stephane Chazelas
2 would have my preference.

[Martijn Dekker]

Post by Ilkka Virta

Post by Stephane Chazelas
$ bash -c 'umask 400; cat <<< test'
bash: cannot create temp file for here-document: Permission denied
Those shells use temporary files to store the content of the
here-documents as the Bourne shell initially did, and open them
in read-only mode to make it cat's stdin.
When umask contains the 0400 bit, the file is created without
read permission to the user, hence the error upon that second
open().
1- do nothing and blame the user
2- open the file only once for both
writing the content and making it the command's stdin
3. use a pipe instead of a temp file
4. Reset the umask temporarily to 077

5. manually chmod() the tempfile to 0400 or 0600 if the open() for
reading fails with EACCES, and then retry. Should be doable with a
localized change to that particular error condition, without changing
the overall behaviour.

Unless I'm missing something, there should be no reason for an internal
temp file to have any permissions other than 0600 (user
readable/writable), so it seems to me that an fchmod call straight after
creating the file and before returning the fd is the simplest way of
fixing the bug; this makes the permissions of internal temp files
entirely independent of the umask.

diff --git a/lib/sh/tmpfile.c b/lib/sh/tmpfile.c
index e41e45b..1805cdf 100644
--- a/lib/sh/tmpfile.c
+++ b/lib/sh/tmpfile.c
@@ -203,7 +203,6 @@ sh_mktmpfd (nameroot, flags, namep)
}
if (namep)
*namep = filename;
- return fd;
#else /* !USE_MKSTEMP */
sh_seedrand ();
do
@@ -224,8 +223,9 @@ sh_mktmpfd (nameroot, flags, namep)
else
free (filename);

- return fd;
#endif /* !USE_MKSTEMP */
+ fchmod(fd, S_IRUSR | S_IWUSR);
+ return fd;
}

FILE *

[Chet Ramey]

Post by Martijn Dekker
Unless I'm missing something, there should be no reason for an internal
temp file to have any permissions other than 0600 (user readable/writable),
so it seems to me that an fchmod call straight after creating the file and
before returning the fd is the simplest way of fixing the bug; this makes
the permissions of internal temp files entirely independent of the umask.

That doesn't work for the same reason as discussed in
http://lists.gnu.org/archive/html/bug-bash/2018-03/msg00074.html.
It's unlikely that someone will set his umask to 400 and expect no ill
effects, but I suppose it's better not to fail in the face of that kind
of behavior.

[Greg Wooledge]

Post by Chet Ramey
That doesn't work for the same reason as discussed in
http://lists.gnu.org/archive/html/bug-bash/2018-03/msg00074.html.
It's unlikely that someone will set his umask to 400 and expect no ill
effects, but I suppose it's better not to fail in the face of that kind
of behavior.

I still maintain that the "umask 400" is most likely a user error.
The user probably wanted a umask that would cause all of the files to
have 0400 permissions. Such a umask would be 0377, not 0400.

A umask that denies read permission to the owner of the file but leaves
the file world writable is simply not rational.

Bash's behavior seems acceptable to me:

wooledg:~$ bash -c 'umask 0777; cat <<< hello'
bash: cannot create temp file for here-document: Permission denied
wooledg:~$ bash-5.0-beta -c 'umask 0777; cat <<< hello'
bash-5.0-beta: cannot create temp file for here-document: Permission denied

But if you want to follow ksh's lead, I would also find that acceptable:

wooledg:~$ ksh -c 'umask 0777; cat <<< hello'
hello

[Greg Wooledge]

Post by Stephane Chazelas
1- do nothing and blame the user as the user explicitly asked
for files to be unreadable

This one gets my vote. That umask is just entirely wrong. Did the
user really mean 0377 by chance? (Even then, what was the objective?
Any umask that includes owner bits is just not rational.)

[L A Walsh]

Post by Stephane Chazelas
$ bash -c 'umask 400; cat <<< test'
bash: cannot create temp file for here-document: Permission denied
...

---

Another way to address it (with env-based limits for instance + sum):
If using less than env-mem limits for here-docs, then
create the here-doc in memory.

Have '<' read from a file-position pointer in the memory buffer.
- Would allow "seeking" if that's an issue.
- for _many_ here-docs, size is relatively small -- so would preclude many
small file IO ops for small here-docs. This may be more important for
here-docs below the media's sector size (w/4K sectors: more common),
or on a
RAID, less than strip size.
- a basic file-w/sequential read facility is not difficult to write;
- no security issues or race-windows of a file temporarily on disk
- no name collisions or lookups

For small systems, even allowing 8-64K in memory could help many usages
w/small
H-docs, but on systems w/many GigaBytes of memory -- all H-doc usage could
come from memory.

In cases where the heredoc had to be accessible from multiple processes,
could use shared-memory if available.

In all cases, fall-back could be to current solution(s).

As a comparison, 'vim' would use temp files to handle large-file editing.
With shift to systems having more memory -- most files can be fully
edited in
memory.

Right now, my linux system has 5.3G available in tmp, compared to 149G
available in free memory.

What would be the downside(s) of such an implementation?

[Eduardo Bustamante]

On Mon, Mar 12, 2018 at 2:05 PM, L A Walsh <***@tlinx.org> wrote:
[...]

Post by L A Walsh
What would be the downside(s) of such an implementation?

There's code out there that relies on several properties of regular
files, one of them for example, that you can seek on them. I recommend
against any change to here documents and here strings that breaks this
expectation.

[L A Walsh]

Post by Eduardo Bustamante
[...]

Post by L A Walsh
What would be the downside(s) of such an implementation?

There's code out there that relies on several properties of regular
files, one of them for example, that you can seek on them. I recommend
against any change to here documents and here strings that breaks this
expectation.

----
Is relying on HERE-doc implementation something that is portable? Is it
required by POSIX? Still a few things to remember...

Complete file-system emulations can be done in tmpfs (memory).
The fact that it is in memory and not a on-disk file system wouldn't be at
issue -- as the emulation of backing by files could be a separate issue from
whether or not here-docs could use memory by default.

At the very least, I said that the use of memory would be subject to
ENV-var limits. Couldn't those applications needing intermediate files set
those limits to '1-byte', so they'd always fall back to the (or some)
current
legacy implementation?

[Greg Wooledge]

Post by L A Walsh
Is relying on HERE-doc implementation something that is portable? Is it
required by POSIX? Still a few things to remember...

POSIX says "no":

It is unspecified whether the file descriptor is opened as a regular
file, a special file, or a pipe. Portable applications cannot rely on
the file descriptor being seekable (see XSH lseek).

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_04

That said, I think a lot of scripts *do* bend this rule and rely on
the here-document creating a seekable temp file, because this has been
the common practice across many different shells for a long time.

[Robert Elz]

Date: Thu, 15 Mar 2018 08:37:39 -0400
From: Greg Wooledge <***@eeg.ccf.org>
Message-ID: <***@eeg.ccf.org>

| That said, I think a lot of scripts *do* bend this rule and rely on
| the here-document creating a seekable temp file, because this has been
| the common practice across many different shells for a long time.

The script itself is hardly going to care, it is more the application that
is given the here-doc on stdin..

If a here doc was passed to some other fd, and the application run
with /dev/fd/3 (or whichever) as an arg, it might be a bigger issue.
But I doubt that there are many scripts that do things like that.

As a general rule, applications that believe they can seek stdin
(where here docs usually are presented) are poorly designed,
if not outright broken, as it means that it isn't possible to run them as

... | application

and in general, I don't think there are very many such applications.

Further, there are shells (I believe all ash derived shells, at least) which
do not use files for here docs (which is why it is unspecified in POSIX).
I don't recall ever seeing a bug report, or anything (that matters, obviously
anyone can deliberately write code that would fail) that suffers because of
this.

kre

[Joerg Schilling]

Post by Stephane Chazelas
Note: sent to bash, zsh and Schily Bourne shell dev mailing
lists (not mksh as my email provider apparently doesn't play
well with mirbsd.org's expensive greylisting, please feel free
to forward there if you don't use gmail).
https://unix.stackexchange.com/questions/429285/cannot-create-temp-file-for-here-document-permission-denied

This is a really interesting problem.

Post by Stephane Chazelas
$ bash -c 'umask 400; cat <<< test'
bash: cannot create temp file for here-document: Permission denied
$ zsh -c 'umask 400; cat <<< test'
zsh:1: can't create temp file for here document: permission denied
$ bosh -c 'umask 400; cat << EOF
test
EOF'
bosh: /tmp/sh193220: cannot open
$ mksh -c 'umask 400; cat <<< test'
mksh: can't open temporary file /tmp/sh933f2z.tmp: Permission denied
Those shells use temporary files to store the content of the
here-documents as the Bourne shell initially did, and open them
in read-only mode to make it cat's stdin.

It is a bit more complex:

They first create a temp file for the here document and this is done
before umask(0400) gets executed.

They then create another file that gets the expanded version for the here
document temp input file. Since this is done after chmod(0400) was executed,
the second file is not readable.

Post by Stephane Chazelas
When umask contains the 0400 bit, the file is created without
read permission to the user, hence the error upon that second
open().
(note that bosh also leaves the temp file behind in that
case).

This is what bosh inherited from the temp file management from the
SVR4 Bourne Shell that creates hard links to temp files in case there is a
sub-shell. This in the past created plenty of left over temp files for various
reasons. At the same time, the original SVR4 Bourne Shell did unlink many of
the temp files too early. Bosh fixed the latter problem by introducing an io
barrier in the list of temp files.

The main problem here is that the second (expanded) temp file is not entered
into the temp file list and thus did not get removed.
With my fixes that keep the file open (see below) the file is now removed before
closing it. I hope this is the only case where temp files still have not been
removed correctly.

Post by Stephane Chazelas
1- do nothing and blame the user as the user explicitly asked
for files to be unreadable (but then again, it's not obvious
to the user that heredocs imply a temp file)
2- do like AT&T ksh/tcsh (or yash for big heredocs that don't
fit in the pipe buffer) and open the file only once for both
writing the content and making it the command's stdin (with a
lseek() to beginning in between). That means the fd ends up
being writable though I can't see it being a huge problem. (Yash
actually gives the file 000 permissions here regardless of the
umask with open("/tmp/yash-ECCFE6268", O_RDWR|O_CREAT|O_EXCL,
0), but see below about =(...) emulation)

This is not sufficient in case that the original (unexpanded) temp file
was created with 0400 already.

...

Post by Stephane Chazelas
4. Reset the umask temporarily to 077 before creating the temp
file (and block trapped signals until it's restored).
2 would have my preference.

See above, this is most likely not sufficient as I am not sure whether it
works to keep the primary temp file open. Ksh93 may do this since it needs to
implement a more complex housekeeping algorithm as a result of the "virtual
subshell" feature anyway.

I changed bosh to keep the second temp file open since the original code
creates the file, writes the expanded input to it, closes it and immediately
reopens it readonly and then removes it.

To fix the write only problem with the primary temp file, I called

fchmod(fd, 0600)

in the function tmpfil() immediately after the creation of the temp file
succeeded.

The modified bosh still passes the bosh unit test suite.

Jörg