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.
This commit is contained in:
parent
b6847141f2
commit
28c5179636
4 changed files with 223 additions and 4 deletions
|
@ -2,5 +2,6 @@ addon/validation.cpp
|
||||||
campaign_server/addon_utils.cpp
|
campaign_server/addon_utils.cpp
|
||||||
campaign_server/blacklist.cpp
|
campaign_server/blacklist.cpp
|
||||||
campaign_server/campaign_server.cpp
|
campaign_server/campaign_server.cpp
|
||||||
|
campaign_server/fs_commit.cpp
|
||||||
server/server_base.cpp
|
server/server_base.cpp
|
||||||
server/simple_wml.cpp
|
server/simple_wml.cpp
|
||||||
|
|
|
@ -33,6 +33,7 @@
|
||||||
#include "campaign_server/addon_utils.hpp"
|
#include "campaign_server/addon_utils.hpp"
|
||||||
#include "campaign_server/blacklist.hpp"
|
#include "campaign_server/blacklist.hpp"
|
||||||
#include "campaign_server/control.hpp"
|
#include "campaign_server/control.hpp"
|
||||||
|
#include "campaign_server/fs_commit.hpp"
|
||||||
#include "version.hpp"
|
#include "version.hpp"
|
||||||
#include "hash.hpp"
|
#include "hash.hpp"
|
||||||
|
|
||||||
|
@ -374,8 +375,9 @@ void server::load_blacklist()
|
||||||
void server::write_config()
|
void server::write_config()
|
||||||
{
|
{
|
||||||
DBG_CS << "writing configuration and add-ons list to disk...\n";
|
DBG_CS << "writing configuration and add-ons list to disk...\n";
|
||||||
filesystem::scoped_ostream out = filesystem::ostream_file(cfg_file_);
|
filesystem::atomic_commit out(cfg_file_);
|
||||||
write(*out, cfg_);
|
write(*out.ostream(), cfg_);
|
||||||
|
out.commit();
|
||||||
DBG_CS << "... done\n";
|
DBG_CS << "... done\n";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -748,9 +750,10 @@ void server::handle_upload(const server::request& req)
|
||||||
add_license(data);
|
add_license(data);
|
||||||
|
|
||||||
{
|
{
|
||||||
filesystem::scoped_ostream campaign_file = filesystem::ostream_file(filename);
|
filesystem::atomic_commit campaign_file(cfg_file_);
|
||||||
config_writer writer(*campaign_file, true, compress_level_);
|
config_writer writer(*campaign_file.ostream(), true, compress_level_);
|
||||||
writer.write(data);
|
writer.write(data);
|
||||||
|
campaign_file.commit();
|
||||||
}
|
}
|
||||||
|
|
||||||
(*campaign)["size"] = filesystem::file_size(filename);
|
(*campaign)["size"] = filesystem::file_size(filename);
|
||||||
|
|
122
src/campaign_server/fs_commit.cpp
Normal file
122
src/campaign_server/fs_commit.cpp
Normal file
|
@ -0,0 +1,122 @@
|
||||||
|
/*
|
||||||
|
Copyright (C) 2016 - 2017 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
|
93
src/campaign_server/fs_commit.hpp
Normal file
93
src/campaign_server/fs_commit.hpp
Normal file
|
@ -0,0 +1,93 @@
|
||||||
|
/*
|
||||||
|
Copyright (C) 2016 - 2017 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.
|
||||||
|
*/
|
||||||
|
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#include "filesystem.hpp"
|
||||||
|
|
||||||
|
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
|
||||||
|
{
|
||||||
|
public:
|
||||||
|
/**
|
||||||
|
* Constructor.
|
||||||
|
*
|
||||||
|
* @throws filesystem::io_exception if the operation fails in some way.
|
||||||
|
*/
|
||||||
|
atomic_commit(const std::string& filename);
|
||||||
|
|
||||||
|
atomic_commit(const atomic_commit&) = delete;
|
||||||
|
atomic_commit& operator=(const atomic_commit&) = delete;
|
||||||
|
|
||||||
|
~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
|
||||||
|
};
|
||||||
|
|
||||||
|
}
|
Loading…
Add table
Reference in a new issue