Re: Fwd: [PATCH v5 00/22] Some rtld-audit fixes

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 23/11/2021 18:13, Jonathon Anderson wrote:
>> On 22/11/2021 14:46, jma14 wrote:
>>> On 11/19/21 14:31, Florian Weimer       wrote:
>>>
>>>> * Adhemerval Zanella:
>>>>>> "" for the main executable is widely known.  Usually code uses it to
>>>>>> implement a fallback on argv[0] or /proc/self/exe, though.
>>> If this is widely known, it would be helpful if this was added to the man pages. Currently the man pages define l_name as the "absolute pathname where object was found." That sounds (to me at least) like dlinfo(dlopen(NULL)) is a portable alternative to the Linux-specific AT_EXECFN, contrary to reality.
>> To provide an absolute pathname it would require to use realpath() or similar
>> strategy either before issuing la_objopen() or while parsing the arguments
>> or reading AT_EXECFN.  I am not sure if this best approach, specially if the
>> executable is executed through a namespace or any kernel abstraction where
>> obtaining the full path though filesystem walk is not possible or restricted.
> I had interpreted this as l_name may not necessarily be a canonical path. To me, the wording "was found" indicates that this is the path resulting from library search, in effect .../libfoo.so.1 instead of the canonical .../libfoo.so.1.2.3. Currently l_name is not always absolute if the search involved a relative path, which I also consider an issue either in documentation or behavior (also affects dladdr).
> 
> Under this interpretation I don't see a need to use realpath() (at most getcwd()), if a user requires a canonical path they can call realpath() themselves. They already need to be robust against the possibility that the file was deleted/replaced right after it was loaded, deleted/replaced symlinks are a reasonable extension to that.

Linux at least appends '(deleted)' if the pathname has been unlinked.

> 
> On 11/23/21 10:50, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> On 23/11/2021 11:02, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> In fact I think rather than using the argv[0], since it passing the
>>>>> executable path is just a convention; I think it would be better to
>>>>> use AT_EXECFN.  On recent kernel it is always passed to userland and
>>>>> kernel should be responsible to hide any filesystem information if it
>>>>> is required.
>>>> It's still a relative path to an unknown directory, I think.  I expect
>>>> (but have not checked) that it is the pathname argument to execveat,
>>>> which may not be meaningful to the new process image.
> execveat seems to generate an AT_EXECFN under /dev/fd, it has no meaning in the new process image if O_CLOEXEC is used on the directory file descriptor. Under the interpretation above I think this is reasonable.
>>>> Yes, but it better than _dl_argv[0] and/or an empty string.  Without 
>>>> proper kernel support we can not reliable get the path, in fact the 
>>>> kernel might in fact hides it on purpose.
>> I'm worried that if we put a path-looking string there, programmers will
>> assume it is THE path to the executable.  With SUID programs, that looks
>> like a recipe for security vulnerabilities.  These users need to use
>> /proc/self/exe and give up if that's not available.
> I do not think l_name is unique in this concern, AT_EXECFN and dladdr expose the same security risk and AFAICT this is not noted in either of their man pages.
> 
> I'm not familiar with all the security constraints surrounding SUID, but as a simple but effective resolution I would consider setting the executable's l_name to the string "/proc/self/exe" in secure-execution mode. A similar change could be done in getauxval() to transparently modify AT_EXECFN. Those two changes would force all unwitting SUID users to (semi-gracefully) fail when /proc is unavailable.

I think this is fair assumption.