lix-project/lix
20
55
Fork 31
Code Issues 343 Code Review (Gerrit) Projects 3 Releases Wiki Activity

Merge changes Ia1481da4,Ifca1d74d into main

Browse source 
* changes:
  archive: refactor bad mutable-state API in the NAR parse listener
  archive: rename ParseSink to NARParseVisitor
This commit is contained in:
jade 2024-09-14 19:26:08 +00:00 committed by Gerrit Code Review
parent b2fc007811 ca1dc3f70b
commit 8f88590d13
6 changed files with 199 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/libstore/local-store.cc
View file

@ -1216,7 +1216,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
bool narRead = false;
Finally cleanup = [&]() {
if (!narRead) {
ParseSink sink;
NARParseVisitor sink;
try {
parseDump(sink, source);
} catch (...) {

33
src/libstore/nar-accessor.cc
View file

@ -2,6 +2,7 @@
#include "archive.hh"
#include <map>
#include <memory>
#include <stack>
#include <algorithm>
@ -33,7 +34,7 @@ struct NarAccessor : public FSAccessor
NarMember root;
struct NarIndexer : ParseSink, Source
struct NarIndexer : NARParseVisitor, Source
{
NarAccessor & acc;
Source & source;
@ -44,11 +45,12 @@ struct NarAccessor : public FSAccessor
uint64_t pos = 0;
public:
NarIndexer(NarAccessor & acc, Source & source)
: acc(acc), source(source)
{ }
void createMember(const Path & path, NarMember member)
NarMember & createMember(const Path & path, NarMember member)
{
size_t level = std::count(path.begin(), path.end(), '/');
while (parents.size() > level) parents.pop();
@ -62,6 +64,8 @@ struct NarAccessor : public FSAccessor
auto result = parents.top()->children.emplace(baseNameOf(path), std::move(member));
parents.push(&result.first->second);
}
return *parents.top();
}
void createDirectory(const Path & path) override
@ -69,28 +73,17 @@ struct NarAccessor : public FSAccessor
createMember(path, {FSAccessor::Type::tDirectory, false, 0, 0});
}
void createRegularFile(const Path & path) override
std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
{
createMember(path, {FSAccessor::Type::tRegular, false, 0, 0});
}
auto & memb = createMember(path, {FSAccessor::Type::tRegular, false, 0, 0});
void closeRegularFile() override
{ }
void isExecutable() override
{
parents.top()->isExecutable = true;
}
void preallocateContents(uint64_t size) override
{
assert(size <= std::numeric_limits<uint64_t>::max());
parents.top()->size = (uint64_t) size;
parents.top()->start = pos;
}
memb.size = (uint64_t) size;
memb.start = pos;
memb.isExecutable = executable;
void receiveContents(std::string_view data) override
{ }
return std::make_unique<FileHandle>();
}
void createSymlink(const Path & path, const std::string & target) override
{

46
src/libstore/store-api.cc
View file

@ -379,6 +379,48 @@ void Store::addMultipleToStore(
}
}
namespace {
/**
* If the NAR archive contains a single file at top-level, then save
* the contents of the file to `s`. Otherwise assert.
*/
struct RetrieveRegularNARVisitor : NARParseVisitor
{
struct MyFileHandle : public FileHandle
{
Sink & sink;
void receiveContents(std::string_view data) override
{
sink(data);
}
private:
MyFileHandle(Sink & sink) : sink(sink) {}
friend struct RetrieveRegularNARVisitor;
};
Sink & sink;
RetrieveRegularNARVisitor(Sink & sink) : sink(sink) { }
std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
{
return std::unique_ptr<MyFileHandle>(new MyFileHandle{sink});
}
void createDirectory(const Path & path) override
{
assert(false && "RetrieveRegularNARVisitor::createDirectory must not be called");
}
void createSymlink(const Path & path, const std::string & target) override
{
assert(false && "RetrieveRegularNARVisitor::createSymlink must not be called");
}
};
}
/*
The aim of this function is to compute in one pass the correct ValidPathInfo for
@ -413,7 +455,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
/* Note that fileSink and unusualHashTee must be mutually exclusive, since
they both write to caHashSink. Note that that requisite is currently true
because the former is only used in the flat case. */
RetrieveRegularNARSink fileSink { caHashSink };
RetrieveRegularNARVisitor fileSink { caHashSink };
TeeSink unusualHashTee { narHashSink, caHashSink };
auto & narSink = method == FileIngestionMethod::Recursive && hashAlgo != HashType::SHA256
@ -429,7 +471,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
information to narSink. */
TeeSource tapped { fileSource, narSink };
ParseSink blank;
NARParseVisitor blank;
auto & parseSink = method == FileIngestionMethod::Flat
? fileSink
: blank;

110
src/libutil/archive.cc
View file

@ -334,7 +334,7 @@ Generator<Entry> parse(Source & source)
}
static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar)
static WireFormatGenerator restore(NARParseVisitor & sink, Generator<nar::Entry> nar)
{
while (auto entry = nar.next()) {
co_yield std::visit(
@ -347,16 +347,13 @@ static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar)
},
[&](nar::File f) {
return [](auto f, auto & sink) -> WireFormatGenerator {
sink.createRegularFile(f.path);
sink.preallocateContents(f.size);
if (f.executable) {
sink.isExecutable();
}
auto handle = sink.createRegularFile(f.path, f.size, f.executable);
while (auto block = f.contents.next()) {
sink.receiveContents(std::string_view{block->data(), block->size()});
handle->receiveContents(std::string_view{block->data(), block->size()});
co_yield *block;
}
sink.closeRegularFile();
handle->close();
}(std::move(f), sink);
},
[&](nar::Symlink sl) {
@ -377,12 +374,12 @@ static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar)
}
}
WireFormatGenerator parseAndCopyDump(ParseSink & sink, Source & source)
WireFormatGenerator parseAndCopyDump(NARParseVisitor & sink, Source & source)
{
return restore(sink, nar::parse(source));
}
void parseDump(ParseSink & sink, Source & source)
void parseDump(NARParseVisitor & sink, Source & source)
{
auto parser = parseAndCopyDump(sink, source);
while (parser.next()) {
@ -390,32 +387,54 @@ void parseDump(ParseSink & sink, Source & source)
}
}
struct RestoreSink : ParseSink
/*
* Note [NAR restoration security]:
* It's *critical* that NAR restoration will never overwrite anything even if
* duplicate filenames are passed in. It is inevitable that not all NARs are
* fit to actually successfully restore to the target filesystem; errors may
* occur due to collisions, and this *must* cause the NAR to be rejected.
*
* Although the filenames are blocked from being *the same bytes* by a higher
* layer, filesystems have other ideas on every platform:
* - The store may be on a case-insensitive filesystem like APFS, ext4 with
* casefold directories, zfs with casesensitivity=insensitive
* - The store may be on a Unicode normalizing (or normalization-insensitive)
* filesystem like APFS (where files are looked up by
* hash(normalize(fname))), HFS+ (where file names are always normalized to
* approximately NFD), or zfs with normalization=formC, etc.
*
* It is impossible to know the version of Unicode being used by the underlying
* filesystem, thus it is *impossible* to stop these collisions.
*
* Overwriting files as a result of invalid NARs will cause a security bug like
* CppNix's CVE-2024-45593 (GHSA-h4vv-h3jq-v493)
*/
/**
* This code restores NARs from disk.
*
* See Note [NAR restoration security] for security invariants in this procedure.
*
*/
struct NARRestoreVisitor : NARParseVisitor
{
Path dstPath;
private:
class MyFileHandle : public FileHandle
{
AutoCloseFD fd;
void createDirectory(const Path & path) override
MyFileHandle(AutoCloseFD && fd, uint64_t size, bool executable) : FileHandle(), fd(std::move(fd))
{
Path p = dstPath + path;
if (mkdir(p.c_str(), 0777) == -1)
throw SysError("creating directory '%1%'", p);
};
void createRegularFile(const Path & path) override
{
Path p = dstPath + path;
fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)};
if (!fd) throw SysError("creating file '%1%'", p);
if (executable) {
makeExecutable();
}
void closeRegularFile() override
{
/* Call close explicitly to make sure the error is checked */
fd.close();
maybePreallocateContents(size);
}
void isExecutable() override
void makeExecutable()
{
struct stat st;
if (fstat(fd.get(), &st) == -1)
@ -424,7 +443,7 @@ struct RestoreSink : ParseSink
throw SysError("fchmod");
}
void preallocateContents(uint64_t len) override
void maybePreallocateContents(uint64_t len)
{
if (!archiveSettings.preallocateContents)
return;
@ -442,11 +461,41 @@ struct RestoreSink : ParseSink
#endif
}
public:
~MyFileHandle() = default;
virtual void close() override
{
/* Call close explicitly to make sure the error is checked */
fd.close();
}
void receiveContents(std::string_view data) override
{
writeFull(fd.get(), data);
}
friend struct NARRestoreVisitor;
};
public:
void createDirectory(const Path & path) override
{
Path p = dstPath + path;
if (mkdir(p.c_str(), 0777) == -1)
throw SysError("creating directory '%1%'", p);
};
std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
{
Path p = dstPath + path;
AutoCloseFD fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)};
if (!fd) throw SysError("creating file '%1%'", p);
return std::unique_ptr<MyFileHandle>(new MyFileHandle(std::move(fd), size, executable));
}
void createSymlink(const Path & path, const std::string & target) override
{
Path p = dstPath + path;
@ -457,7 +506,7 @@ struct RestoreSink : ParseSink
void restorePath(const Path & path, Source & source)
{
RestoreSink sink;
NARRestoreVisitor sink;
sink.dstPath = path;
parseDump(sink, source);
}
@ -468,10 +517,9 @@ WireFormatGenerator copyNAR(Source & source)
// FIXME: if 'source' is the output of dumpPath() followed by EOF,
// we should just forward all data directly without parsing.
static ParseSink parseSink; /* null sink; just parse the NAR */
static NARParseVisitor parseSink; /* null sink; just parse the NAR */
return parseAndCopyDump(parseSink, source);
}
}

72
src/libutil/archive.hh
View file

@ -76,45 +76,47 @@ WireFormatGenerator dumpString(std::string_view s);
/**
* \todo Fix this API, it sucks.
* A visitor for NAR parsing that performs filesystem (or virtual-filesystem)
* actions to restore a NAR.
*
* Methods of this may arbitrarily fail due to filename collisions.
*/
struct ParseSink
struct NARParseVisitor
{
virtual void createDirectory(const Path & path) { };
virtual void createRegularFile(const Path & path) { };
virtual void closeRegularFile() { };
virtual void isExecutable() { };
virtual void preallocateContents(uint64_t size) { };
virtual void receiveContents(std::string_view data) { };
virtual void createSymlink(const Path & path, const std::string & target) { };
};
/**
* If the NAR archive contains a single file at top-level, then save
* the contents of the file to `s`. Otherwise barf.
/**
* A type-erased file handle specific to this particular NARParseVisitor.
*/
struct RetrieveRegularNARSink : ParseSink
{
bool regular = true;
Sink & sink;
RetrieveRegularNARSink(Sink & sink) : sink(sink) { }
void createDirectory(const Path & path) override
struct FileHandle
{
regular = false;
FileHandle() {}
FileHandle(FileHandle const &) = delete;
FileHandle & operator=(FileHandle &) = delete;
/** Puts one block of data into the file */
virtual void receiveContents(std::string_view data) { }
/**
* Explicitly closes the file. Further operations may throw an assert.
* This exists so that closing can fail and throw an exception without doing so in a destructor.
*/
virtual void close() { }
virtual ~FileHandle() = default;
};
virtual void createDirectory(const Path & path) { }
/**
* Creates a regular file in the extraction output with the given size and executable flag.
* The size is guaranteed to be the true size of the file.
*/
[[nodiscard]]
virtual std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable)
{
return std::make_unique<FileHandle>();
}
void receiveContents(std::string_view data) override
{
sink(data);
}
void createSymlink(const Path & path, const std::string & target) override
{
regular = false;
}
virtual void createSymlink(const Path & path, const std::string & target) { }
};
namespace nar {
@ -160,8 +162,8 @@ Generator<Entry> parse(Source & source);
}
WireFormatGenerator parseAndCopyDump(ParseSink & sink, Source & source);
void parseDump(ParseSink & sink, Source & source);
WireFormatGenerator parseAndCopyDump(NARParseVisitor & sink, Source & source);
void parseDump(NARParseVisitor & sink, Source & source);
void restorePath(const Path & path, Source & source);

2
src/libutil/file-system.hh
View file

@ -210,7 +210,7 @@ inline Paths createDirs(PathView path)
}
/**
* Create a symlink.
* Create a symlink. Throws if the symlink exists.
*/
void createSymlink(const Path & target, const Path & link);