From: Stephen Morris Date: Thu, 13 Jun 2019 13:13:54 +0000 (+0100) Subject: [#640,!351] Move from a static class to an instance class X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=927b836f122eb5cc0656112c7107f659cfbfb81a;p=thirdparty%2Fkea.git [#640,!351] Move from a static class to an instance class --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 132e498147..d86798d507 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -776,11 +776,11 @@ Dhcpv4Srv::run() { // send it as packets to Kea. Kea is supposed to process them and hopefully // not crash in the process. Once the packet processing is done, Kea should // let the know that it's ready for the next packet. This is done further - // down in this loop (see Fuzz::packetProcessed()). - Fuzz::init(4, &shutdown_); + // down in this loop by a call to the packetProcessed() method. + Fuzz fuzz_controller(4, &shutdown_); // // The next line is needed as a signature for AFL to recognise that we are - // running persistent fuzzing. + // running persistent fuzzing. This has to be in the main image file. __AFL_LOOP(0); #endif // ENABLE_AFL while (!shutdown_) { @@ -802,7 +802,7 @@ Dhcpv4Srv::run() { #ifdef ENABLE_AFL // Ok, this particular packet processing is done. If we are fuzzing, // let AFL know about it. - Fuzz::packetProcessed(); + fuzz_controller.packetProcessed(); #endif // ENABLE_AFL } diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 760a39944f..383c183aed 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -451,11 +451,11 @@ bool Dhcpv6Srv::run() { // send it as packets to Kea. Kea is supposed to process them and hopefully // not crash in the process. Once the packet processing is done, Kea should // let the know that it's ready for the next packet. This is done further - // down in this loop (see Fuzz::packetProcessed()). - Fuzz::init(6, &shutdown_); + // down in this loop by a call to the packetProcessed() method. + Fuzz fuzz_controller(6, &shutdown_); // // The next line is needed as a signature for AFL to recognise that we are - // running persistent fuzzing. + // running persistent fuzzing. This has to be in the main image file. __AFL_LOOP(0); #endif // ENABLE_AFL @@ -478,7 +478,7 @@ bool Dhcpv6Srv::run() { #ifdef ENABLE_AFL // Ok, this particular packet processing is done. If we are fuzzing, // let AFL know about it. - Fuzz::packetProcessed(); + fuzz_controller.packetProcessed(); #endif // ENABLE_AFL } diff --git a/src/lib/dhcpsrv/fuzz.cc b/src/lib/dhcpsrv/fuzz.cc index 1be2394b5f..b4a2b3e830 100644 --- a/src/lib/dhcpsrv/fuzz.cc +++ b/src/lib/dhcpsrv/fuzz.cc @@ -37,37 +37,16 @@ constexpr size_t Fuzz::BUFFER_SIZE; constexpr useconds_t Fuzz::SLEEP_INTERVAL; constexpr long Fuzz::LOOP_COUNT; -// Variables needed to synchronize between main and fuzzing threads. The -// fuzz_sync_ condition is notified by the fuzzing thread and waited for by the -// main thread. The main_sync_ condition is notified by the main thread and -// waited for by the fuzzing thread. -FuzzSynch Fuzz::fuzz_sync_; -FuzzSynch Fuzz::main_sync_; -std::thread Fuzz::fuzzing_thread_; - -// Address structure used to define the address/port used to send -// fuzzing data to the Kea server. -struct sockaddr_in6 Fuzz::servaddr6_; -struct sockaddr_in Fuzz::servaddr4_; -sockaddr* Fuzz::sockaddr_ptr = NULL; -size_t Fuzz::sockaddr_len = 0; - -// Pointer to the shutdown flag used by Kea. The fuzzing code will set this -// after the appropriate number of packets have been fuzzed. -volatile bool* Fuzz::shutdown_ptr_ = NULL; - -// FuzzSynch methods. FuzSynch is the class that encapsulates the +// FuzzSync methods. FuzSynch is the class that encapsulates the // synchronization process between the main and fuzzing threads. -// init - just set the predicate as false. -void FuzzSynch::init(const char* name) { - name_ = name; - ready_ = false; +// Constructor +FuzzSync::FuzzSync(const char* name) : ready_(false), name_(name) { } // Wait to be notified when the predicate is true void -FuzzSynch::wait(void) { +FuzzSync::wait(void) { LOG_DEBUG(fuzz_logger, FUZZ_DBG_TRACE_DETAIL, FUZZ_WAITING).arg(name_); unique_lock lock(mutex_); cond_.wait(lock, [=]() { return this->ready_; }); @@ -77,7 +56,7 @@ FuzzSynch::wait(void) { // Set predicate and notify the waiting thread to continue void -FuzzSynch::notify(void) { +FuzzSync::notify(void) { LOG_DEBUG(fuzz_logger, FUZZ_DBG_TRACE_DETAIL, FUZZ_SETTING).arg(name_); unique_lock lock(mutex_); ready_ = true; @@ -87,30 +66,97 @@ FuzzSynch::notify(void) { // Fuzz methods. +// Constructor +Fuzz::Fuzz(int ipversion, volatile bool* shutdown) : + fuzz_sync_("fuzz_sync"), main_sync_("main_sync"), address_(nullptr), + interface_(nullptr), loop_max_(LOOP_COUNT), port_(0), running_(false), + sockaddr_ptr(nullptr), sockaddr_len(0), shutdown_ptr_(nullptr) { + + try { + stringstream reason; // Used to construct exception messages + + // Store reference to shutdown flag. When the fuzzing loop has read + // the set number of packets from AFL, it will set this flag to trigger + // a Kea shutdown. + if (shutdown) { + shutdown_ptr_ = shutdown; + } else { + isc_throw(FuzzInitFail, "must pass shutdown flag to kea_fuzz_init"); + } + + // Set up address structures. + setAddress(ipversion); + + // Check if the hard-coded maximum loop count is being overridden + const char *loop_max_ptr = getenv("KEA_AFL_LOOP_MAX"); + if (loop_max_ptr != 0) { + try { + loop_max_ = boost::lexical_cast(loop_max_ptr); + } catch (const boost::bad_lexical_cast&) { + reason << "cannot convert port number specification " + << loop_max_ptr << " to an integer"; + isc_throw(FuzzInitFail, reason.str()); + } + + if (loop_max_ <= 0) { + reason << "KEA_AFL_LOOP_MAX is " << loop_max_ << ". " + << "It must be an integer greater than zero."; + isc_throw(FuzzInitFail, reason.str()); + } + } + + // Start the thread that reads the packets sent by AFL from stdin and + // passes them to the port on which Kea is listening. + fuzzing_thread_ = std::thread(&Fuzz::run, this); + + // Wait for the fuzzing thread to read its first packet from AFL and + // send it to the port on which Kea is listening. + fuzz_sync_.wait(); + + } catch (const FuzzInitFail& e) { + // AFL tends to make it difficult to find out what exactly has failed: + // make sure that the error is logged. + LOG_FATAL(fuzz_logger, FUZZ_INIT_FAIL).arg(e.what()); + throw; + } + + LOG_INFO(fuzz_logger, FUZZ_INIT_COMPLETE).arg(interface_).arg(address_) + .arg(port_).arg(loop_max_); +} + +// Destructor +Fuzz::~Fuzz() { + // The fuzzing thread should not be running when the fuzzing object + // goes out of scope. + if (running_) { + LOG_ERROR(fuzz_logger, FUZZ_THREAD_NOT_TERMINATED); + } +} + +// Parse IP address/port/interface and set up address structures. void Fuzz::setAddress(int ipversion) { stringstream reason; // Used in error messages // Get the environment for the fuzzing: interface, address and port. - const char *iface = getenv("KEA_AFL_INTERFACE"); - if (! iface) { + interface_ = getenv("KEA_AFL_INTERFACE"); + if (! interface_) { isc_throw(FuzzInitFail, "no fuzzing interface has been set"); } // Now the address. - const char *address = getenv("KEA_AFL_ADDRESS"); - if (address == 0) { + address_ = getenv("KEA_AFL_ADDRESS"); + if (address_ == 0) { isc_throw(FuzzInitFail, "no fuzzing address has been set"); } // ... and the port. - unsigned short port = 0; const char *port_ptr = getenv("KEA_AFL_PORT"); if (port_ptr == 0) { isc_throw(FuzzInitFail, "no fuzzing port has been set"); } try { - port = boost::lexical_cast(port_ptr); + port_ = boost::lexical_cast(port_ptr); } catch (const boost::bad_lexical_cast&) { reason << "cannot convert port number specification " << port_ptr << " to an integer"; @@ -118,39 +164,39 @@ Fuzz::setAddress(int ipversion) { } // Decide if the address is an IPv4 or IPv6 address. - if ((strstr(address, ".") != NULL) && (ipversion == 4)) { + if ((strstr(address_, ".") != NULL) && (ipversion == 4)) { // Assume an IPv4 address memset(&servaddr4_, 0, sizeof(servaddr4_)); servaddr4_.sin_family = AF_INET; - if (inet_pton(AF_INET, address, &servaddr4_.sin_addr) != 1) { + if (inet_pton(AF_INET, address_, &servaddr4_.sin_addr) != 1) { reason << "inet_pton() failed: can't convert " - << address << " to an IPv6 address" << endl; + << address_ << " to an IPv6 address" << endl; isc_throw(FuzzInitFail, reason.str()); } - servaddr4_.sin_port = htons(port); + servaddr4_.sin_port = htons(port_); sockaddr_ptr = reinterpret_cast(&servaddr4_); sockaddr_len = sizeof(servaddr4_); - } else if ((strstr(address, ":") != NULL) && (ipversion == 6)) { + } else if ((strstr(address_, ":") != NULL) && (ipversion == 6)) { // Set up the IPv6 address structure. memset(&servaddr6_, 0, sizeof (servaddr6_)); servaddr6_.sin6_family = AF_INET6; - if (inet_pton(AF_INET6, address, &servaddr6_.sin6_addr) != 1) { + if (inet_pton(AF_INET6, address_, &servaddr6_.sin6_addr) != 1) { reason << "inet_pton() failed: can't convert " - << address << " to an IPv6 address" << endl; + << address_ << " to an IPv6 address" << endl; isc_throw(FuzzInitFail, reason.str()); } - servaddr6_.sin6_port = htons(port); + servaddr6_.sin6_port = htons(port_); // Interface ID is needed for IPv6 address structures. - servaddr6_.sin6_scope_id = if_nametoindex(iface); + servaddr6_.sin6_scope_id = if_nametoindex(interface_); if (servaddr6_.sin6_scope_id == 0) { reason << "error retrieving interface ID for " - << iface << ": " << strerror(errno); + << interface_ << ": " << strerror(errno); isc_throw(FuzzInitFail, reason.str()); } @@ -158,53 +204,13 @@ Fuzz::setAddress(int ipversion) { sockaddr_len = sizeof(servaddr6_); } else { reason << "Expected IP version (" << ipversion << ") is not " - << "4 or 6, or the given address " << address << " does not " + << "4 or 6, or the given address " << address_ << " does not " << "match the IP version expected"; isc_throw(FuzzInitFail, reason.str()); } - LOG_DEBUG(fuzz_logger, FUZZ_DBG_TRACE, FUZZ_INTERFACE).arg(iface).arg(address).arg(port); } -// Initialization: create the thread artifacts and start the fuzzing thread. -void -Fuzz::init(int ipversion, volatile bool* shutdown_flag) { - try { - stringstream reason; //yy Used for exception messages - - // Store reference to shutdown flag. When the fuzzing loop has read - // the set number of packets from AFL, it will set this flag to trigger - // a Kea shutdown. - if (shutdown_flag) { - shutdown_ptr_ = shutdown_flag; - } else { - isc_throw(FuzzInitFail, "must pass shutdown flag to kea_fuzz_init"); - } - - // Initialize synchronization variables. - fuzz_sync_.init("fuzz_synch"); - main_sync_.init("main_synch"); - - // Set up address structures. - setAddress(ipversion); - - // Start the thread that reads the packets sent by AFL from stdin and - // passes them to the port on which Kea is listening. - fuzzing_thread_ = std::thread(Fuzz::run); - - // Wait for the fuzzing thread to read its first packet from AFL and - // send it to the port on which Kea is listening. - fuzz_sync_.wait(); - - } catch (const FuzzInitFail& e) { - // AFL tends to make it difficult to find out what exactly has failed: - // make sure that the error is logged. - LOG_FATAL(fuzz_logger, FUZZ_INIT_FAIL).arg(e.what()); - throw; - } - - LOG_DEBUG(fuzz_logger, FUZZ_DBG_TRACE, FUZZ_INIT_COMPLETE); -} // This is the main fuzzing function. It receives data from fuzzing engine. // That data is received to stdin and then sent over the configured UDP socket. @@ -212,6 +218,8 @@ Fuzz::init(int ipversion, volatile bool* shutdown_flag) { // that task being signalled by the main thread calling Fuzz::packetProcessed(). void Fuzz::run(void) { + running_ = true; + // Create the socket throw which packets read from stdin will be send // to the port on which Kea is listening. int sockfd = socket(AF_INET6, SOCK_DGRAM, 0); @@ -226,10 +234,10 @@ Fuzz::run(void) { // inside the read loop in the server process using __AFL_LOOP) to ensure // that thread running this function shuts down properly between each // restart of Kea. - auto loop = Fuzz::LOOP_COUNT; + auto loop = loop_max_; while (loop-- > 0) { // Read from stdin and continue reading (albeit after a pause) even - // if there is an error. Do the same with end of files. + // if there is an error. Do the same if an EOF is received. char buf[BUFFER_SIZE]; ssize_t length = read(0, buf, sizeof(buf)); if (length <= 0) { @@ -288,6 +296,8 @@ Fuzz::run(void) { // If the main thread is waiting, let it terminate as well. fuzz_sync_.notify(); + running_ = false; + return; } diff --git a/src/lib/dhcpsrv/fuzz.h b/src/lib/dhcpsrv/fuzz.h index b74d2eb65f..c51f18e587 100644 --- a/src/lib/dhcpsrv/fuzz.h +++ b/src/lib/dhcpsrv/fuzz.h @@ -28,15 +28,14 @@ namespace isc { /// This contains the variables and encapsulates the primitives required /// to manage the condition variables between the two threads. -class FuzzSynch { +class FuzzSync { public: - /// @brief Initialization + /// @brief Constructor /// - /// Objects of this type are declared static to allow them to be accessed - /// from mutiple thread. This function allows appropriate initialization. + /// Just set the name of the variable for debug message purposes. /// - /// @param name Name for debug messages - void init(const char* name); + /// @param name The name of the object, output in debug messages. + FuzzSync(const char* name); /// @brief Waits for condition notification /// @@ -78,21 +77,26 @@ private: class Fuzz { public: - /// @brief Initializes Kea fuzzing + /// @brief Constructor /// - /// This takes one parameter, which is a pointer to the shutdown flag. Kea - /// runs until something sets this flag to true, at which point it shuts - /// down. + /// Initializes member variables. + Fuzz(); + + /// @brief Constructor /// - /// In the case of fuzzing, the shutdown flag is set when a fixed number of - /// packets has been received from the fuzzer. After Kea exits, the fuzzer - /// will restart it. + /// Initializes fuzzing object and starts the fuzzing thread. /// /// @param ipversion Either 4 or 6 depending on what IP version the /// server responds to. /// @param shutdown Pointer to boolean flag that will be set to true to - /// trigger the shutdown procedure. - static void init(int ipversion, volatile bool* shutdown); + /// trigger the shutdown procedure. + Fuzz(int ipversion, volatile bool* shutdown); + + /// @brief Destructor + /// + /// Does some basic checks when going out of scope. The purpose of these + /// checks is to aid diagnosis in the event of problems with the fuzzing. + ~Fuzz(); /// @brief Main Kea Fuzzing Function /// @@ -109,7 +113,7 @@ public: /// any resource leaks (which are not caught by AFL) from getting too large /// and interfering with the fuzzing. AFL will automatically restart the /// program to continue fuzzing. - static void run(void); + void run(void); /// @brief Notify fuzzing thread that processing is complete /// @@ -122,7 +126,7 @@ public: /// /// If a shutdown has been initiated, this method waits for the fuzzing /// thread to exit before allowing the shutdown to continue. - static void packetProcessed(void); + void packetProcessed(void); /// @brief Populate address structures /// @@ -134,7 +138,7 @@ public: /// /// @throws FuzzInitFail Thrown if the address is not in the expected /// format. - static void setAddress(int ipversion); + void setAddress(int ipversion); /// @brief size of the buffer used to transfer data between AFL and Kea. static constexpr size_t BUFFER_SIZE = 65536; @@ -149,19 +153,24 @@ public: /// processing packets properly. static constexpr long LOOP_COUNT = 1000; - // Condition/mutext variables. The fuzz_XX_ variables are set by the + // Condition/mutex variables. The fuzz_XX_ variables are set by the // fuzzing thread and waited on by the main thread. The main_XX_ variables // are set by the main thread and waited on by the fuzzing thread. - static FuzzSynch fuzz_sync_; // Set by fuzzing thread - static FuzzSynch main_sync_; // Set by main thread + FuzzSync fuzz_sync_; // Set by fuzzing thread + FuzzSync main_sync_; // Set by main thread // Other member variables. - static std::thread fuzzing_thread_;//< Holds the thread ID - static struct sockaddr* sockaddr_ptr; //< Pointer to structure used - static size_t sockaddr_len; //< Length of the structure - static struct sockaddr_in servaddr4_; //< IPv6 address information - static struct sockaddr_in6 servaddr6_; //< IPv6 address information - static volatile bool* shutdown_ptr_; //< Pointer to shutdown flag + const char* address_; //< Pointer to address string + std::thread fuzzing_thread_;//< Holds the thread ID + const char* interface_; //< Pointer to interface string + long loop_max_; //< Maximum number of loop iterations + uint16_t port_; //< Port number to use + volatile bool running_; //< Set if the thread is running + struct sockaddr* sockaddr_ptr; //< Pointer to structure used + size_t sockaddr_len; //< Length of the structure + struct sockaddr_in servaddr4_; //< IPv6 address information + struct sockaddr_in6 servaddr6_; //< IPv6 address information + volatile bool* shutdown_ptr_; //< Pointer to shutdown flag }; diff --git a/src/lib/dhcpsrv/fuzz_messages.mes b/src/lib/dhcpsrv/fuzz_messages.mes index ff4bbd6d40..0ea8779a71 100644 --- a/src/lib/dhcpsrv/fuzz_messages.mes +++ b/src/lib/dhcpsrv/fuzz_messages.mes @@ -14,9 +14,10 @@ the fuzzer via stdin An informational message output during fuzzing initialization, this reports the details of the interface to be used for fuzzing. -% FUZZ_INIT_COMPLETE fuzz initialization complete -A debug message output when the fuzzing initialization function has completed -successfully. +% FUZZ_INIT_COMPLETE fuzz initialization complete: interface %1, address %2, port %3, max loops %4 +An informational message output when the fuzzing initialization function has +completed successfully. The parameters listed are those which must be/can be +set via environment variables. % FUZZ_INIT_FAIL fuzz initialization failure, reason: %1 An error message reported if the fuzzing initialization failed. The reason @@ -29,6 +30,11 @@ itself down and will be restarted by AFL. This is recommended by the AFL documentation as a way of avoiding issues if Kea gets itself into a funny state after running for a long time. +% FUZZ_LOOP_MAX fuzzing loop will run %1 time(s) before exiting +A debug message noting how many loops (i.e. packets from the fuzzer) the code +will process before Kea exits and the fuzzer restarts it. The hard-coded +value can be overridden by setting the environment variable KEA_AFL_LOOP_MAX. + % FUZZ_PACKET_PROCESSED_CALLED packetProcessed has been called A debug message indicating that the processing of a packet by Kea has finished and that the Fuzz::packetProcessed() method has been called. This @@ -77,6 +83,11 @@ through which is will copy data received on stdin from the AFL fuzzer to the port on which Kea is listening. The program will most likely hang if this occurs. +% FUZZ_THREAD_NOT_TERMINATED fuzzing thread has not terminated +An error message output in the destructor of the fuzzing object should the +object go out of scope with the fuzzing thread running. This message is +for diagnosing problems in the fuzzing code. + % FUZZ_THREAD_TERMINATED fuzzing thread has terminated A debug message, output when the main thread has detected that the fuzzing thread has terminated.