<div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 18, 2018 at 11:03 AM Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Jul 18, 2018 at 1:46 PM, Geoffrey Romer <<a href="mailto:gromer@google.com" target="_blank">gromer@google.com</a>> wrote:<br>
><br>
> On Wed, Jul 18, 2018 at 9:52 AM Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> On Tue, Jul 17, 2018 at 5:53 PM, Geoffrey Romer <<a href="mailto:gromer@google.com" target="_blank">gromer@google.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> > On Tue, Jul 17, 2018 at 10:40 AM Billy O'Neal <<a href="mailto:billy.oneal@gmail.com" target="_blank">billy.oneal@gmail.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> Does anyone have an example of something that is secure without caching<br>
>> >> that is insecure with caching? All of the examples posted as<br>
>> >> problematic are<br>
>> >> still insecure.<br>
>> ><br>
>> ><br>
>> > +1. By my reading, different caching decisions can only make TOCTOU<br>
>> > problems<br>
>> > more or less acute; caching can't introduce TOCTOU problems that don't<br>
>> > already exist.<br>
>><br>
>> That is my belief as well. However, I don't find the argument to be<br>
>> persuasive, so it's a -1 from me.<br>
>><br>
>> > From that point of view, caching could actually be helpful,<br>
>> > by making the TOCTOU problems more obvious.<br>
>><br>
>> Caching is hidden from the user's view, and I fail to see how<br>
>> something hidden from the user's view will will make anything more<br>
>> obvious. What caching does do is extend the length of time over which<br>
>> TOCTOU bugs can occur, which is worse behavior from a security<br>
>> perspective. Users will definitely introduce exciting TOCTOU bugs into<br>
>> their code with or without the caching mechanism, but I'm worried<br>
>> about worsening the effects in practice.<br>
><br>
><br>
> Generally speaking, I would expect "worsening the effects" of the bugs to<br>
> make those bugs more obvious.<br>
<br>
I don't think that applies to cases that are timing based. You have to<br>
know to be actively testing that scenario to notice the worsening<br>
effects, at which point you'd likely have already fixed the TOCTOU.<br></blockquote><div><br></div><div>I agree that "more obvious" may still not be obvious enough.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
>> > The only change we could<br>
>> > meaningfully make here is to give users some conditions under which<br>
>> > non-modifying operations on the same file are guaranteed to reflect a<br>
>> > single<br>
>> > self-consistent state of the file (which may or may not be the present<br>
>> > state), but the benefit of that seems minimal.<br>
>> ><br>
>> > I think this ship sailed when we decided to standardize a TOCTOU-prone<br>
>> > API;<br>
>> > all we can do now is try to educate users about what not to use<br>
>> > std::filesystem for (e.g. anything remotely related to security).<br>
>><br>
>> "This doesn't introduce new security issues, it just makes existing<br>
>> ones worse" is not an argument that leaves me with warm, fuzzy<br>
>> feelings. The goal is obviously to prevent the TOCTOU bugs in the<br>
>> first place, but if we cannot achieve that, we shouldn't exacerbate<br>
>> the TOCTOU problems only because we can't achieve the ideal.<br>
><br>
><br>
> That's not so obvious to me. If exacerbating the TOCTOU problems makes<br>
> people more aware of those problems, that could well be a net win.<br>
<br>
How is it making more people aware of the problem, though? Silently<br>
exacerbating the issue doesn't mean people are going to become aware<br>
of it when they previously were not, unless you consider "there's a<br>
new CVE out today" being the kind of education that raises awareness,<br>
but I'd hardly call that a net win.<br></blockquote><div><br></div><div>If the problems are exacerbated to the point that they're visible even in non-attack scenarios (which Eric seemed to be concerned about), that could make people more aware of the problem.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Conversely, to the extent that the caching design makes it easier to use<br>
> std::filesystem in security-sensitive contexts, that is likely to be a net<br>
> loss, because std::filesystem simply should not be used in<br>
> security-sensitive contexts.<br>
<br>
Last time I looked into it, Clang and LLVM had quite a few TOCTOU<br>
bugs. No one was compelled to fix them because hey, we're just a<br>
compiler, so who cares -- it's not a security-sensitive context. Well,<br>
until people started running it on web servers... (It's been a number<br>
of years since I looked, and we may have improved this since clangd<br>
came along, but the example still holds.)</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The trouble is that "security-sensitive context" is a moving target<br>
for a whole lot of code.<br></blockquote><div><br></div><div><div style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">Personally, I agree, but WG21's decision to standardize std::filesystem reflects a judgement that programmers can be expected to distinguish contexts where TOCTOU is an issue from contexts where it is not.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Keep in mind, security exploits are generally not statistical in nature;<br>
> halving the time window for a TOCTOU attack does not halve your<br>
> vulnerability, because the components of the attack are not occurring at<br>
> random; they're being induced by an attacker. Shortening the time window may<br>
> have some security benefit on the margins in terms of increasing the effort<br>
> required for a successful exploit, but that's highly contingent and<br>
> uncertain. I expect the security benefits of eliminating a TOCTOU<br>
> vulnerability (e.g. by using a safer filesystem API) to dwarf the benefits<br>
> of almost any reduction in the vulnerability time window.<br>
<br>
We're in agreement here that eliminating the TOCTOU is the only way to<br>
eliminate the vulnerability. However, much of security is about taking<br>
many small steps to mitigate security issues and reduce attack vectors<br>
as part of defense-in-depth.</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I guess my point is: let's not be quick to say "well, we can't solve<br>
this to the ideal, so we can do whatever we want in the name of<br>
performance." </blockquote><div><br></div><div>I'm not saying we can't "solve this to the ideal", I'm saying we can't "solve" it at all; at best we can apply mitigations that may reduce the vulnerability by some degree that's unknown, but probably small. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">As history has unkindly demonstrated, that way lies many<br>
years of CVEs.</blockquote><div><br></div><div>What history do you have in mind? My sense is that "let's patch up this fundamentally vulnerable API to try to make it a little harder to attack" has also been a path to many years of CVEs.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I think a lit review of TOCTOU papers might be<br>
interesting to see if anyone has explored the ramifications of a<br>
widened time window. I'll try to do that when I have a spare moment,<br>
but if anyone gets to it before me, I'd love to hear the results.<br></blockquote><div><br></div><div>That makes sense, although I'd be mildly surprised if there was enough literature to settle the question. Ideally we'd consult with some security experts about whether this is worth addressing.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
~Aaron<br>
<br>
><br>
>><br>
>><br>
>> ~Aaron<br>
>><br>
>> ><br>
>> ><br>
>> >><br>
>> >><br>
>> >><br>
>> >> Billy3<br>
>> >><br>
>> >><br>
>> >><br>
>> >> From: Aaron Ballman<br>
>> >> Sent: Tuesday, July 17, 2018 4:46 AM<br>
>> >> To: Eric Fiselier<br>
>> >> Cc: clang developer list; Marshall Clow; Titus Winters; Billy O'Neal;<br>
>> >> Geoffrey Romer<br>
>> >> Subject: Re: [cfe-dev] [libc++][RFC] Implementing Directory Entry<br>
>> >> Caching<br>
>> >> forFilesystem<br>
>> >><br>
>> >><br>
>> >><br>
>> >> Thank you for putting this together, I greatly appreciate it!<br>
>> >><br>
>> >><br>
>> >><br>
>> >> Out of curiosity, is the caching behavior something you expect you'll<br>
>> >><br>
>> >> make configurable for users of libc++ (for instance, a macro to enable<br>
>> >><br>
>> >> or disable caching), or are there issues preventing such an<br>
>> >><br>
>> >> implementation? My feeling is that we're going to have the usual push<br>
>> >><br>
>> >> and pull between security and performance where different users will<br>
>> >><br>
>> >> have different (and equally valid) use cases in mind justifying the<br>
>> >><br>
>> >> need for the performance provided by caching or the security red flags<br>
>> >><br>
>> >> enhanced by not caching. We should obviously pick a good default<br>
>> >><br>
>> >> behavior (and can argue over the definition of "good" in that<br>
>> >><br>
>> >> context), but I think no matter what option we pick, users will want<br>
>> >><br>
>> >> the other option to be available.<br>
>> >><br>
>> >><br>
>> >><br>
>> >> ~Aaron<br>
>> >><br>
>> >><br>
>> >><br>
>> >> On Mon, Jul 16, 2018 at 10:39 PM, Eric Fiselier via cfe-dev<br>
>> >><br>
>> >> <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
>> >><br>
>> >> > Hi All,<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > I have a couple of questions and concerns regarding implementations<br>
>> >> > of<br>
>> >><br>
>> >> > P0317R1 [1], which I would like help from the community answering.<br>
>> >> > The<br>
>> >> > paper<br>
>> >><br>
>> >> > changes `directory_entry` to cache a number of attributes, including<br>
>> >> > file<br>
>> >><br>
>> >> > status,  permissions, file size, number of hard links, last write<br>
>> >> > time,<br>
>> >> > and<br>
>> >><br>
>> >> > more. For reference, this is the interface of `directory_entry`<br>
>> >> > before<br>
>> >> > this<br>
>> >><br>
>> >> > paper [2], and this is the interface after [3].<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The implementation has a lot of latitude over which attributes it<br>
>> >> > caches, if<br>
>> >><br>
>> >> > any, and how it caches them. As I'll explain below, each different<br>
>> >> > approach<br>
>> >><br>
>> >> > can cause non-obvious behavior, possibly leading to TOCTOU bugs and<br>
>> >> > security<br>
>> >><br>
>> >> > vulnerabilities!<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > My question for the community is this:<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Given the considerations below, what should libc++ choose to do?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > 1. Cache nothing?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > 2. Cache as much as possible, when possible?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > 3. Something in between?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > I would like to land std::filesystem before the 7.0 branch, and doing<br>
>> >> > so<br>
>> >><br>
>> >> > requires deciding on which implementation to provide now.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Note that this paper only considers POSIX based implementations of<br>
>> >><br>
>> >> > filesystem.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > TOCTOU Violations<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > ============================<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Time of check to time of use, or TOCTOU, is a class of software bugs<br>
>> >> > caused<br>
>> >><br>
>> >> > by changes in a system between the time a condition is checked, and<br>
>> >> > the<br>
>> >> > time<br>
>> >><br>
>> >> > the result is used [4]. If libc++ chooses to do any caching in<br>
>> >><br>
>> >> > directory_entry, it will be vulnerable to this. If libc++ chooses to<br>
>> >> > do<br>
>> >> > no<br>
>> >><br>
>> >> > caching at all, it will also be vulnerable to this but in different<br>
>> >><br>
>> >> > scenarios.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Most importantly, whichever approach is taken, libc++ will bare some<br>
>> >><br>
>> >> > responsibility for the TOCTOU bugs it allows users to write<br>
>> >> > accidently.<br>
>> >> > So<br>
>> >><br>
>> >> > it’s imperative we consider all approaches carefully.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Let’s take a simple example:<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > void remove_symlinks_in_dir(path p) {<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  for (directory_entry ent : directory_iterator(p)) {<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >    if (ent.is_symlink())<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >      remove(ent);<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  }<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > }<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Above, a TOCTOU violation may occur if another process acts on the<br>
>> >> > file<br>
>> >><br>
>> >> > referenced by `ent`, changing the attributes between the point the<br>
>> >><br>
>> >> > directory_entry was created and the point at which the `is_symlink`<br>
>> >><br>
>> >> > condition is checked.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > At this point it's important to note that <filesystem> interface is,<br>
>> >> > in<br>
>> >><br>
>> >> > general, vulnerable to TOCTOU --- even without directory_entry<br>
>> >> > caching.<br>
>> >> > For<br>
>> >><br>
>> >> > example:<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > void remove_symlinks_in_dir(path p) {<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  for (directory_entry ent : directory_iterator(p)) {<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >    if (is_symlink(ent))<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >      remove(ent);<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  }<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > }<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > In the above case, a TOCTOU violation still occurs, since changes to<br>
>> >> > `ent`<br>
>> >><br>
>> >> > can take place between the check and the call to `remove`. However,<br>
>> >> > at<br>
>> >> > least<br>
>> >><br>
>> >> > the above example makes it clear *when* the check is occuring.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Simply eliminating caching will not prevent TOCTOU violations, but,<br>
>> >> > in<br>
>> >> > this<br>
>> >><br>
>> >> > particular case, it would make them harder to write and make it more<br>
>> >> > obvious<br>
>> >><br>
>> >> > where they may occur. Having a cache both extends the period of time<br>
>> >> > between<br>
>> >><br>
>> >> > the check and the use, as well as hiding when the check actually<br>
>> >> > occurs.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Perhaps the above concessions make it worthwhile to support a more<br>
>> >> > efficient<br>
>> >><br>
>> >> > interface with caching and fewer file system accesses?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > If we do choose to cache some attributes, then, at minimum, it should<br>
>> >> > be<br>
>> >><br>
>> >> > obvious to the user which values are cached and which are not. The<br>
>> >> > following<br>
>> >><br>
>> >> > section will explore the feasibility of this.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Directory Iteration VS Refresh<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > ==================================<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The paper's intention is to allow attributes returned during<br>
>> >> > directory<br>
>> >><br>
>> >> > iteration to be accessed by the user without additional calls to the<br>
>> >><br>
>> >> > underlying filesystem.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The cache is populated two different ways:<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > During directory iteration, the cache is populated with only the<br>
>> >> > attributes<br>
>> >><br>
>> >> > returned by directory iteration function. With POSIX `readdir`, only<br>
>> >> > the<br>
>> >><br>
>> >> > file type as returned by `lstat` is available.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Using the `refresh` function, either called directly by the user, or<br>
>> >><br>
>> >> > implicitly by the constructors or modifying methods (Ex. `assign`).<br>
>> >><br>
>> >> > `refresh` has the ability to fully populate the cache (but not<br>
>> >> > atomically in<br>
>> >><br>
>> >> > some cases, see the symlink considerations below).<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Note that const accessors may not update the cache.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Therefore the state of the cache depends on whether the<br>
>> >> > `directory_entry`<br>
>> >><br>
>> >> > was created during directory iteration or by the<br>
>> >> > `directory_entry(path&)`<br>
>> >><br>
>> >> > constructor, and whether there have been any intervening calls to<br>
>> >> > `refresh`<br>
>> >><br>
>> >> > or any other modifying member functions.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The following case demonstrates a surprising TOCTOU violation, only<br>
>> >> > when<br>
>> >> > the<br>
>> >><br>
>> >> > `directory_entry` cache was populated during directory iteration, and<br>
>> >> > not by<br>
>> >><br>
>> >> > a call to refresh:<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > // Assume the filesystem supports permissions on symlinks.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > bool is_readable_symlink(directory_entry ent) {<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  if (!ent.is_symlink())<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >    return false;<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  // We should have the file_status for the symlink cached, right?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  file_status st = ent.symlink_status();<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  // Nope! Only when `refresh` has been called are the perms cached.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  assert(is_symlink(st)); // May fail!<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  return (st.permissions() & perms::others_read) != perms::none;<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > }<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The above example is intended to demonstrate that the different<br>
>> >> > accessors<br>
>> >><br>
>> >> > may return a mix of cached and uncached values. Here `is_symlink()`<br>
>> >> > is<br>
>> >><br>
>> >> > cached, but `symlink_status()` may not be.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > If the symlink has been replaced with a regular file, then the result<br>
>> >> > of<br>
>> >><br>
>> >> > this function is potentially dangerous nonsense.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > So what happens when the user calls `refresh()` before accessing the<br>
>> >><br>
>> >> > attributes? Does this make it safer?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The answer depends on the caching behavior of the implementation. and<br>
>> >><br>
>> >> > whether that behavior provides "atomic" or "non-atomic" attribute<br>
>> >> > access<br>
>> >> > (as<br>
>> >><br>
>> >> > defined in the next section).<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Atomic VS Non-Atomic Attribute Access<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > =============================================<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > As previously described, TOCTOU violations require there to be<br>
>> >> > intervening<br>
>> >><br>
>> >> > time between the check and the use. We'll call any filesystem<br>
>> >> > function,<br>
>> >> > or<br>
>> >><br>
>> >> > set of functions, which don’t allow for TOCTOU bugs "atomic". All<br>
>> >> > other<br>
>> >><br>
>> >> > functions or set of functions are considered "non-atomic".<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Therefore, a `directory_entry` object can be said to provide "atomic<br>
>> >><br>
>> >> > attribute access" across all accessors if and only if (A) all of the<br>
>> >><br>
>> >> > attributes are cached, and (B) the cache was populated atomically. A<br>
>> >><br>
>> >> > non-caching implementation of `directory_entry` never provides atomic<br>
>> >><br>
>> >> > attribute access.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Let’s consider the consequences using the following example:<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > bool  is_empty_regular_file(path p) {<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  directory_entry ent(p);   // Calls refresh()<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> >  return ent.is_regular_file() && ent.file_size() == 0;<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > }<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > In a non-caching implementation `refresh()` is a nop, and therefore<br>
>> >> > opens<br>
>> >><br>
>> >> > the user up to TOCTOU issues occuring between the call to<br>
>> >> > `is_symlink()`<br>
>> >> > and<br>
>> >><br>
>> >> > `is_regular_file()`.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > In a fully-caching implementation, which provides atomic attribute<br>
>> >> > access,<br>
>> >><br>
>> >> > no such problem occurs.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > In this case it seems preferable for libc++ to provide a<br>
>> >> > fully-caching<br>
>> >><br>
>> >> > implementation with "atomic attribute access". But is this possible?<br>
>> >> > The<br>
>> >><br>
>> >> > answer is "sometimes, but not always" as we'll see below.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Problems with Symlinks And Refresh()<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > ===========================================<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Above we established that if refresh caches at all, then it should do<br>
>> >> > so<br>
>> >><br>
>> >> > "atomically", retrieving all attributes from the filesystem in a<br>
>> >> > atomic<br>
>> >><br>
>> >> > manner. Note that for symlinks, some of the cached attributes refer<br>
>> >> > to<br>
>> >> > the<br>
>> >><br>
>> >> > symlink (is_symlink() and `symlink_status()), while the rest refer to<br>
>> >> > the<br>
>> >><br>
>> >> > linked entity (is_regular_file(), file_size(), etc).<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > When the directory entry does not refer to a symlink, a single call<br>
>> >> > to<br>
>> >><br>
>> >> > `lstat` provides enough information to fully populate the cache<br>
>> >> > atomically.<br>
>> >><br>
>> >> > However, when the entity is a symlink, we would need a second call to<br>
>> >> > `stat`<br>
>> >><br>
>> >> > to determine the properties of the linked file.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Therefore, "atomic attribute access" is not possible to guarantee.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > An implementation may choose to partially populate the caches,<br>
>> >> > omitting<br>
>> >><br>
>> >> > attributes about the linked entity; or it may choose to fully<br>
>> >> > populate<br>
>> >> > the<br>
>> >><br>
>> >> > cache non-atomically using a non-atomic series of calls to `lstat`<br>
>> >> > and<br>
>> >><br>
>> >> > `stat`.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > The former case obfuscates from the user which attributes are cached<br>
>> >> > and<br>
>> >><br>
>> >> > which are not, opening them up to TOCTOU violations.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Worse yet, the latter case, which fully populates the cache<br>
>> >> > non-atomically,<br>
>> >><br>
>> >> > potentially commits a TOCTOU violation itself, and this violation is<br>
>> >> > hidden<br>
>> >><br>
>> >> > from the user, preventing them from knowing it exists or being able<br>
>> >> > to<br>
>> >> > do<br>
>> >><br>
>> >> > anything about it.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Both solutions seem fraught with problems.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Conclusion<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > ===============<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > I hope to have shown the pros and cons of different directory entry<br>
>> >> > caching<br>
>> >><br>
>> >> > implementations. Now I need your help deciding what's best for<br>
>> >> > Libc++!<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Personally, I think the best solution is to revert these changes to<br>
>> >> > the<br>
>> >><br>
>> >> > standard, since each possible implementation seems broken in one way<br>
>> >> > or<br>
>> >><br>
>> >> > another. Directory entries should not cache, and directory iterators<br>
>> >> > should<br>
>> >><br>
>> >> > not attempt to populate the cache incompletely. Afterwards, a better<br>
>> >> > and<br>
>> >><br>
>> >> > safer proposal can be put forward  which to provides efficient and<br>
>> >> > atomic<br>
>> >><br>
>> >> > access to multiple attributes for the same entity can be proposed.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Unfortunately, it might be to late at this point to revert the paper,<br>
>> >> > even<br>
>> >><br>
>> >> > if the committee generally agrees with these problems.<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > Thoughts?<br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > [1]<br>
>> >> > <a href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0317r1.html" rel="noreferrer" target="_blank">http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0317r1.html</a><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > [2] <a href="https://en.cppreference.com/w/cpp/experimental/fs/directory_entry" rel="noreferrer" target="_blank">https://en.cppreference.com/w/cpp/experimental/fs/directory_entry</a><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > [3] <a href="https://en.cppreference.com/w/cpp/filesystem/directory_entry" rel="noreferrer" target="_blank">https://en.cppreference.com/w/cpp/filesystem/directory_entry</a><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > [4] <a href="https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use" rel="noreferrer" target="_blank">https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use</a><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >> > _______________________________________________<br>
>> >><br>
>> >> > cfe-dev mailing list<br>
>> >><br>
>> >> > <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
>> >><br>
>> >> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
>> >><br>
>> >> ><br>
>> >><br>
>> >><br>
</blockquote></div></div>