<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style></head><body lang=EN-US link=blue vlink="#954F72"><div class=WordSection1><p class=MsoNormal>Does anyone have an example of something that is secure without caching that is insecure with caching? All of the examples posted as problematic are still insecure.</p><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Billy3</p><p class=MsoNormal><o:p> </o:p></p><div style='mso-element:para-border-div;border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal style='border:none;padding:0in'><b>From: </b><a href="mailto:aaron@aaronballman.com">Aaron Ballman</a><br><b>Sent: </b>Tuesday, July 17, 2018 4:46 AM<br><b>To: </b><a href="mailto:eric@efcs.ca">Eric Fiselier</a><br><b>Cc: </b><a href="mailto:cfe-dev@lists.llvm.org">clang developer list</a>; <a href="mailto:mclow.lists@gmail.com">Marshall Clow</a>; <a href="mailto:titus@google.com">Titus Winters</a>; <a href="mailto:billy.oneal@gmail.com">Billy O'Neal</a>; <a href="mailto:gromer@google.com">Geoffrey Romer</a><br><b>Subject: </b>Re: [cfe-dev] [libc++][RFC] Implementing Directory Entry Caching forFilesystem</p></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Thank you for putting this together, I greatly appreciate it!</p><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Out of curiosity, is the caching behavior something you expect you'll</p><p class=MsoNormal>make configurable for users of libc++ (for instance, a macro to enable</p><p class=MsoNormal>or disable caching), or are there issues preventing such an</p><p class=MsoNormal>implementation? My feeling is that we're going to have the usual push</p><p class=MsoNormal>and pull between security and performance where different users will</p><p class=MsoNormal>have different (and equally valid) use cases in mind justifying the</p><p class=MsoNormal>need for the performance provided by caching or the security red flags</p><p class=MsoNormal>enhanced by not caching. We should obviously pick a good default</p><p class=MsoNormal>behavior (and can argue over the definition of "good" in that</p><p class=MsoNormal>context), but I think no matter what option we pick, users will want</p><p class=MsoNormal>the other option to be available.</p><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>~Aaron</p><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>On Mon, Jul 16, 2018 at 10:39 PM, Eric Fiselier via cfe-dev</p><p class=MsoNormal><cfe-dev@lists.llvm.org> wrote:</p><p class=MsoNormal>> Hi All,</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> I have a couple of questions and concerns regarding implementations of</p><p class=MsoNormal>> P0317R1 [1], which I would like help from the community answering. The paper</p><p class=MsoNormal>> changes `directory_entry` to cache a number of attributes, including file</p><p class=MsoNormal>> status, permissions, file size, number of hard links, last write time, and</p><p class=MsoNormal>> more. For reference, this is the interface of `directory_entry` before this</p><p class=MsoNormal>> paper [2], and this is the interface after [3].</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The implementation has a lot of latitude over which attributes it caches, if</p><p class=MsoNormal>> any, and how it caches them. As I'll explain below, each different approach</p><p class=MsoNormal>> can cause non-obvious behavior, possibly leading to TOCTOU bugs and security</p><p class=MsoNormal>> vulnerabilities!</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> My question for the community is this:</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Given the considerations below, what should libc++ choose to do?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> 1. Cache nothing?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> 2. Cache as much as possible, when possible?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> 3. Something in between?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> I would like to land std::filesystem before the 7.0 branch, and doing so</p><p class=MsoNormal>> requires deciding on which implementation to provide now.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Note that this paper only considers POSIX based implementations of</p><p class=MsoNormal>> filesystem.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> TOCTOU Violations</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> ============================</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Time of check to time of use, or TOCTOU, is a class of software bugs caused</p><p class=MsoNormal>> by changes in a system between the time a condition is checked, and the time</p><p class=MsoNormal>> the result is used [4]. If libc++ chooses to do any caching in</p><p class=MsoNormal>> directory_entry, it will be vulnerable to this. If libc++ chooses to do no</p><p class=MsoNormal>> caching at all, it will also be vulnerable to this but in different</p><p class=MsoNormal>> scenarios.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Most importantly, whichever approach is taken, libc++ will bare some</p><p class=MsoNormal>> responsibility for the TOCTOU bugs it allows users to write accidently. So</p><p class=MsoNormal>> it’s imperative we consider all approaches carefully.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Let’s take a simple example:</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> void remove_symlinks_in_dir(path p) {</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> for (directory_entry ent : directory_iterator(p)) {</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> if (ent.is_symlink())</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> remove(ent);</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> }</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> }</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Above, a TOCTOU violation may occur if another process acts on the file</p><p class=MsoNormal>> referenced by `ent`, changing the attributes between the point the</p><p class=MsoNormal>> directory_entry was created and the point at which the `is_symlink`</p><p class=MsoNormal>> condition is checked.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> At this point it's important to note that <filesystem> interface is, in</p><p class=MsoNormal>> general, vulnerable to TOCTOU --- even without directory_entry caching. For</p><p class=MsoNormal>> example:</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> void remove_symlinks_in_dir(path p) {</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> for (directory_entry ent : directory_iterator(p)) {</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> if (is_symlink(ent))</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> remove(ent);</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> }</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> }</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> In the above case, a TOCTOU violation still occurs, since changes to `ent`</p><p class=MsoNormal>> can take place between the check and the call to `remove`. However, at least</p><p class=MsoNormal>> the above example makes it clear *when* the check is occuring.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Simply eliminating caching will not prevent TOCTOU violations, but, in this</p><p class=MsoNormal>> particular case, it would make them harder to write and make it more obvious</p><p class=MsoNormal>> where they may occur. Having a cache both extends the period of time between</p><p class=MsoNormal>> the check and the use, as well as hiding when the check actually occurs.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Perhaps the above concessions make it worthwhile to support a more efficient</p><p class=MsoNormal>> interface with caching and fewer file system accesses?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> If we do choose to cache some attributes, then, at minimum, it should be</p><p class=MsoNormal>> obvious to the user which values are cached and which are not. The following</p><p class=MsoNormal>> section will explore the feasibility of this.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Directory Iteration VS Refresh</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> ==================================</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The paper's intention is to allow attributes returned during directory</p><p class=MsoNormal>> iteration to be accessed by the user without additional calls to the</p><p class=MsoNormal>> underlying filesystem.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The cache is populated two different ways:</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> During directory iteration, the cache is populated with only the attributes</p><p class=MsoNormal>> returned by directory iteration function. With POSIX `readdir`, only the</p><p class=MsoNormal>> file type as returned by `lstat` is available.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Using the `refresh` function, either called directly by the user, or</p><p class=MsoNormal>> implicitly by the constructors or modifying methods (Ex. `assign`).</p><p class=MsoNormal>> `refresh` has the ability to fully populate the cache (but not atomically in</p><p class=MsoNormal>> some cases, see the symlink considerations below).</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Note that const accessors may not update the cache.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Therefore the state of the cache depends on whether the `directory_entry`</p><p class=MsoNormal>> was created during directory iteration or by the `directory_entry(path&)`</p><p class=MsoNormal>> constructor, and whether there have been any intervening calls to `refresh`</p><p class=MsoNormal>> or any other modifying member functions.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The following case demonstrates a surprising TOCTOU violation, only when the</p><p class=MsoNormal>> `directory_entry` cache was populated during directory iteration, and not by</p><p class=MsoNormal>> a call to refresh:</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> // Assume the filesystem supports permissions on symlinks.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> bool is_readable_symlink(directory_entry ent) {</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> if (!ent.is_symlink())</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> return false;</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> // We should have the file_status for the symlink cached, right?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> file_status st = ent.symlink_status();</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> // Nope! Only when `refresh` has been called are the perms cached.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> assert(is_symlink(st)); // May fail!</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> return (st.permissions() & perms::others_read) != perms::none;</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> }</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The above example is intended to demonstrate that the different accessors</p><p class=MsoNormal>> may return a mix of cached and uncached values. Here `is_symlink()` is</p><p class=MsoNormal>> cached, but `symlink_status()` may not be.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> If the symlink has been replaced with a regular file, then the result of</p><p class=MsoNormal>> this function is potentially dangerous nonsense.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> So what happens when the user calls `refresh()` before accessing the</p><p class=MsoNormal>> attributes? Does this make it safer?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The answer depends on the caching behavior of the implementation. and</p><p class=MsoNormal>> whether that behavior provides "atomic" or "non-atomic" attribute access (as</p><p class=MsoNormal>> defined in the next section).</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Atomic VS Non-Atomic Attribute Access</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> =============================================</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> As previously described, TOCTOU violations require there to be intervening</p><p class=MsoNormal>> time between the check and the use. We'll call any filesystem function, or</p><p class=MsoNormal>> set of functions, which don’t allow for TOCTOU bugs "atomic". All other</p><p class=MsoNormal>> functions or set of functions are considered "non-atomic".</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Therefore, a `directory_entry` object can be said to provide "atomic</p><p class=MsoNormal>> attribute access" across all accessors if and only if (A) all of the</p><p class=MsoNormal>> attributes are cached, and (B) the cache was populated atomically. A</p><p class=MsoNormal>> non-caching implementation of `directory_entry` never provides atomic</p><p class=MsoNormal>> attribute access.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Let’s consider the consequences using the following example:</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> bool is_empty_regular_file(path p) {</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> directory_entry ent(p); // Calls refresh()</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> return ent.is_regular_file() && ent.file_size() == 0;</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> }</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> In a non-caching implementation `refresh()` is a nop, and therefore opens</p><p class=MsoNormal>> the user up to TOCTOU issues occuring between the call to `is_symlink()` and</p><p class=MsoNormal>> `is_regular_file()`.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> In a fully-caching implementation, which provides atomic attribute access,</p><p class=MsoNormal>> no such problem occurs.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> In this case it seems preferable for libc++ to provide a fully-caching</p><p class=MsoNormal>> implementation with "atomic attribute access". But is this possible? The</p><p class=MsoNormal>> answer is "sometimes, but not always" as we'll see below.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Problems with Symlinks And Refresh()</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> ===========================================</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Above we established that if refresh caches at all, then it should do so</p><p class=MsoNormal>> "atomically", retrieving all attributes from the filesystem in a atomic</p><p class=MsoNormal>> manner. Note that for symlinks, some of the cached attributes refer to the</p><p class=MsoNormal>> symlink (is_symlink() and `symlink_status()), while the rest refer to the</p><p class=MsoNormal>> linked entity (is_regular_file(), file_size(), etc).</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> When the directory entry does not refer to a symlink, a single call to</p><p class=MsoNormal>> `lstat` provides enough information to fully populate the cache atomically.</p><p class=MsoNormal>> However, when the entity is a symlink, we would need a second call to `stat`</p><p class=MsoNormal>> to determine the properties of the linked file.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Therefore, "atomic attribute access" is not possible to guarantee.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> An implementation may choose to partially populate the caches, omitting</p><p class=MsoNormal>> attributes about the linked entity; or it may choose to fully populate the</p><p class=MsoNormal>> cache non-atomically using a non-atomic series of calls to `lstat` and</p><p class=MsoNormal>> `stat`.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> The former case obfuscates from the user which attributes are cached and</p><p class=MsoNormal>> which are not, opening them up to TOCTOU violations.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Worse yet, the latter case, which fully populates the cache non-atomically,</p><p class=MsoNormal>> potentially commits a TOCTOU violation itself, and this violation is hidden</p><p class=MsoNormal>> from the user, preventing them from knowing it exists or being able to do</p><p class=MsoNormal>> anything about it.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Both solutions seem fraught with problems.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Conclusion</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> ===============</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> I hope to have shown the pros and cons of different directory entry caching</p><p class=MsoNormal>> implementations. Now I need your help deciding what's best for Libc++!</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Personally, I think the best solution is to revert these changes to the</p><p class=MsoNormal>> standard, since each possible implementation seems broken in one way or</p><p class=MsoNormal>> another. Directory entries should not cache, and directory iterators should</p><p class=MsoNormal>> not attempt to populate the cache incompletely. Afterwards, a better and</p><p class=MsoNormal>> safer proposal can be put forward which to provides efficient and atomic</p><p class=MsoNormal>> access to multiple attributes for the same entity can be proposed.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Unfortunately, it might be to late at this point to revert the paper, even</p><p class=MsoNormal>> if the committee generally agrees with these problems.</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> Thoughts?</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0317r1.html</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> [2] https://en.cppreference.com/w/cpp/experimental/fs/directory_entry</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> [3] https://en.cppreference.com/w/cpp/filesystem/directory_entry</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> [4] https://en.wikipedia.org/wiki/Time_of_check_to_time_of_use</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal>> _______________________________________________</p><p class=MsoNormal>> cfe-dev mailing list</p><p class=MsoNormal>> cfe-dev@lists.llvm.org</p><p class=MsoNormal>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</p><p class=MsoNormal>><o:p> </o:p></p><p class=MsoNormal><o:p> </o:p></p></div></body></html>