campaignd: Add wrapper for atomic commits of crucial files

As the 2016-10-07~09 downtime incident shows, it is paramount to take
further steps in guaranteeing that the server can't corrupt its own data
files (especially the add-ons database) when receiving
inappropriately-timed signals.

This commit adds and deploys an ostream wrapper that requires callers to
explicitly commit the results to disk when finished writing to the
stream, so that only then the real destination file is overwritten with
the working copy (a temporary in the same dir). This way, there should
never be a situation in which the destination is left in an inconsistent
state due to a signal or exception.

The temporary receives a predictable name right now in the interest of
simplicity, since we are more or less in control of the target directory
anyway. We definitely don't want it to be an unlinked file since that
would make it impossible for admins to inspect and compare the temporary
against the original afterwards.

The code makes some assumptions about the nature of the return value of
filesystem::ostream_file() which will never be broken in this stable
branch, which is why one helper function is in campaignd land rather
than in the global filesystem API for now. This should probably be
rectified when forward-porting to master. Maybe.

Nothing of this will work reliably on Windows but we don't care. There's
only one machine in the world where we support running campaignd at this
time and it runs Linux.

src/CMakeLists.txt

@@ -1101,6 +1101,7 @@ set(campaignd_SRC
 	campaign_server/addon_utils.cpp
 	campaign_server/blacklist.cpp
 	campaign_server/campaign_server.cpp
+	campaign_server/fs_commit.cpp
 	server/input_stream.cpp
 	${network_implementation_files}
 	loadscreen_empty.cpp

src/SConscript

@@ -580,6 +580,7 @@ client_env.WesnothProgram("wesnoth", wesnoth_objects, have_client_prereqs)
 campaignd_sources = Split("""
     campaign_server/addon_utils.cpp
     campaign_server/blacklist.cpp
+    campaign_server/fs_commit.cpp
     server/input_stream.cpp
     """)
 

src/campaign_server/campaign_server.cpp

@@ -32,6 +32,7 @@
 #include "campaign_server/addon_utils.hpp"
 #include "campaign_server/blacklist.hpp"
 #include "campaign_server/control.hpp"
+#include "campaign_server/fs_commit.hpp"
 #include "version.hpp"
 #include "util.hpp"
 #include "hash.hpp"
@@ -271,8 +272,9 @@ void server::load_blacklist()
 void server::write_config()
 {
 	DBG_CS << "writing configuration and add-ons list to disk...\n";
-	filesystem::scoped_ostream out = filesystem::ostream_file(cfg_file_);
-	write(*out, cfg_);
+	filesystem::atomic_commit out(cfg_file_);
+	write(*out.ostream(), cfg_);
+	out.commit();
 	DBG_CS << "... done\n";
 }
 
@@ -794,9 +796,10 @@ void server::handle_upload(const server::request& req)
 		add_license(data);
 
 		{
-			filesystem::scoped_ostream campaign_file = filesystem::ostream_file(filename);
-			config_writer writer(*campaign_file, true, compress_level_);
+			filesystem::atomic_commit campaign_file(cfg_file_);
+			config_writer writer(*campaign_file.ostream(), true, compress_level_);
 			writer.write(data);
+			campaign_file.commit();
 		}
 
 		(*campaign)["size"] = filesystem::file_size(filename);

src/campaign_server/fs_commit.cpp

@@ -0,0 +1,123 @@
+/*
+   Copyright (C) 2016 by Ignacio R. Morelle <shadowm2006@gmail.com>
+   Part of the Battle for Wesnoth Project http://www.wesnoth.org/
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY.
+
+   See the COPYING file for more details.
+*/
+
+#include "campaign_server/fs_commit.hpp"
+
+#include "log.hpp"
+#include "serialization/parser.hpp"
+
+#include <cerrno>
+#include <cstdio>
+#include <cstring>
+
+#include <unistd.h>
+
+#ifndef _WIN32
+#include <boost/iostreams/device/file_descriptor.hpp>
+#include <boost/iostreams/stream.hpp>
+#endif
+
+static lg::log_domain log_filesystem("filesystem");
+
+#define DBG_FS LOG_STREAM(debug, log_filesystem)
+#define LOG_FS LOG_STREAM(info,  log_filesystem)
+#define WRN_FS LOG_STREAM(warn,  log_filesystem)
+#define ERR_FS LOG_STREAM(err,   log_filesystem)
+
+namespace filesystem
+{
+
+namespace
+{
+
+inline void atomic_fail(const std::string& step_description)
+{
+	const std::string errno_desc = std::strerror(errno);
+	ERR_FS << "Atomic commit failed (" << step_description << "): "
+		   << errno_desc << '\n';
+	throw filesystem::io_exception(std::string("Atomic commit failed (") + step_description + ")");
+}
+
+#ifndef _WIN32
+
+/**
+ * Returns the POSIX file descriptor associated with the stream.
+ *
+ * This only makes sense for valid streams returned by ostream_file(). Anything
+ * else will yield 0.
+ */
+int get_stream_file_descriptor(std::ostream& os)
+{
+	// NOTE: This is insider knowledge of filesystem::ostream_file(), but it will
+	//       do for 1.12 at least.
+	typedef boost::iostreams::stream<boost::iostreams::file_descriptor_sink> fd_stream_type;
+	fd_stream_type* const real = dynamic_cast<fd_stream_type*>(&os);
+	return real ? (*real)->handle() : 0;
+}
+
+#endif // ! _WIN32
+
+} // unnamed namespace
+
+atomic_commit::atomic_commit(const std::string& filename)
+	: temp_name_(filename + ".new")
+	, dest_name_(filename)
+	, out_(filesystem::ostream_file(temp_name_))
+#ifndef _WIN32
+	, outfd_(filesystem::get_stream_file_descriptor(*out_))
+#endif
+{
+	LOG_FS << "Atomic write guard created for " << dest_name_ << " using " << temp_name_ << '\n';
+}
+
+atomic_commit::~atomic_commit()
+{
+	if(!temp_name_.empty()) {
+		ERR_FS << "Temporary file for atomic write leaked: " << temp_name_ << '\n';
+	}
+}
+
+void atomic_commit::commit()
+{
+	if(temp_name_.empty()) {
+		ERR_FS << "Attempted to commit " << dest_name_ << " more than once!\n";
+		return;
+	}
+
+#ifdef _WIN32
+	// WARNING:
+	// Obviously not atomic at all. Perhaps there's an alternate way to achieve
+	// the same more securely using the Win32 API, but I don't think anyone
+	// cares about running campaignd on this platform, let alone making it
+	// resilient against environment errors. This is just here for reference.
+	if(filesystem::file_exists(dest_name_) && std::remove(dest_name_.c_str()) != 0) {
+		atomic_fail("remove");
+	}
+#else
+	if(fsync(outfd_) != 0) {
+		atomic_fail("fsync");
+	}
+#endif
+
+	if(std::rename(temp_name_.c_str(), dest_name_.c_str()) != 0) {
+		atomic_fail("rename");
+	}
+
+	LOG_FS << "Atomic commit succeeded: " << temp_name_ << " -> " << dest_name_ << '\n';
+
+	temp_name_.clear();
+}
+
+
+} // namespace filesystem

src/campaign_server/fs_commit.hpp

@@ -0,0 +1,97 @@
+/*
+   Copyright (C) 2016 by Ignacio R. Morelle <shadowm2006@gmail.com>
+   Part of the Battle for Wesnoth Project http://www.wesnoth.org/
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY.
+
+   See the COPYING file for more details.
+*/
+
+/**
+ * @file
+ * Atomic filesystem commit functions.
+ */
+
+#ifndef CAMPAIGN_SERVER_FS_COMMIT_HPP_INCLUDED
+#define CAMPAIGN_SERVER_FS_COMMIT_HPP_INCLUDED
+
+#include "filesystem.hpp"
+
+#include <boost/noncopyable.hpp>
+
+class config;
+
+namespace filesystem
+{
+
+/**
+ * Wrapper class that guarantees that file commit atomicity.
+ *
+ * It is possible for a signal or exception to cause a file write to be aborted
+ * in the middle of the process, leaving potentially inconsistent contents
+ * behind which may be read by the same or another application later and result
+ * in errors or further data loss.
+ *
+ * This wrapper prevents this by providing callers with an interface to request
+ * a write stream that will be actually associated to a temporary file. Once
+ * the caller is done with the file, it should call the commit() method to
+ * complete the process so that the temporary replaces the original in an
+ * atomic step.
+ *
+ * The rationale for using an explicit commit() method instead of the class
+ * destructor is that the destructor will also be invoked during exception
+ * handling, which could still cause the destination file to end up in an
+ * inconsistent state.
+ *
+ * Note that if the destructor is invoked before commit() is, the temporary
+ * file will be left behind. This is deliberate so as to provide a way for
+ * the user to look at the resulting file and optionally try to reconcile it
+ * against the original.
+ */
+class atomic_commit : private boost::noncopyable
+{
+public:
+	/**
+	 * Constructor.
+	 *
+	 * @throws filesystem::io_exception if the operation fails in some way.
+	 */
+	atomic_commit(const std::string& filename);
+
+	~atomic_commit();
+
+	/**
+	 * Returns the write stream associated with the file.
+	 *
+	 * Before commit() is invoked, this refers to the temporary file; afterwards,
+	 * to the destination.
+	 */
+	scoped_ostream& ostream()
+	{
+		return out_;
+	}
+
+	/**
+	 * Commits the new file contents to disk atomically.
+	 *
+	 * @throws filesystem::io_exception if the operation fails in some way.
+	 */
+	void commit();
+
+private:
+	std::string temp_name_;
+	std::string dest_name_;
+	scoped_ostream out_;
+#ifndef _WIN32
+	int outfd_;
+#endif
+};
+
+}
+
+#endif