From: Stephen Morris Date: Tue, 6 Aug 2019 10:58:16 +0000 (+0100) Subject: [#640,!351] Make code changes suggested by review X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9240f4d876258e35ffa74ccdae3e0aae13060fff;p=thirdparty%2Fkea.git [#640,!351] Make code changes suggested by review --- diff --git a/src/lib/dhcpsrv/fuzz.cc b/src/lib/dhcpsrv/fuzz.cc index 1619c71811..c838ca6df2 100644 --- a/src/lib/dhcpsrv/fuzz.cc +++ b/src/lib/dhcpsrv/fuzz.cc @@ -39,26 +39,30 @@ constexpr long Fuzz::MAX_LOOP_COUNT; // Constructor Fuzz::Fuzz(int ipversion, uint16_t port) : - address_(nullptr), interface_(nullptr), loop_max_(MAX_LOOP_COUNT), - port_(port), sockaddr_len_(0), sockaddr_ptr_(nullptr), sockfd_(-1) { + loop_max_(MAX_LOOP_COUNT), sockaddr_len_(0), sockaddr_ptr_(nullptr), + sockfd_(-1) { try { stringstream reason; // Used to construct exception messages - // Set up address structures. - setAddress(ipversion); + // Get the values of the environment variables used to control the + // fuzzing. - // Create the socket through which packets read from stdin will be sent - // to the port on which Kea is listening. This is closed in the - // destructor. - sockfd_ = socket((ipversion == 4) ? AF_INET : AF_INET6, SOCK_DGRAM, 0); - if (sockfd_ < 0) { - LOG_FATAL(fuzz_logger, FUZZ_SOCKET_CREATE_FAIL) - .arg(strerror(errno)); - return; + // Specfies the interface to be used to pass packets from AFL to Kea. + const char* interface = getenv("KEA_AFL_INTERFACE"); + if (! interface) { + isc_throw(FuzzInitFail, "no fuzzing interface has been set"); + } + + // The address on the interface to be used. + const char* address = getenv("KEA_AFL_ADDRESS"); + if (address == 0) { + isc_throw(FuzzInitFail, "no fuzzing address has been set"); } - // Check if the hard-coded maximum loop count is being overridden + // Number of Kea packet-read loops before Kea exits and AFL starts a + // new instance. This is optional: the default is set by the constant + // MAX_LOOP_COUNT. const char *loop_max_ptr = getenv("KEA_AFL_LOOP_MAX"); if (loop_max_ptr != 0) { try { @@ -76,15 +80,28 @@ Fuzz::Fuzz(int ipversion, uint16_t port) : } } + // Set up address structures used to route the packets from AFL to Kea. + createAddressStructures(ipversion, interface, address, port); + + // Create the socket through which packets read from stdin will be sent + // to the port on which Kea is listening. This is closed in the + // destructor. + sockfd_ = socket((ipversion == 4) ? AF_INET : AF_INET6, SOCK_DGRAM, 0); + if (sockfd_ < 0) { + LOG_FATAL(fuzz_logger, FUZZ_SOCKET_CREATE_FAIL) + .arg(strerror(errno)); + return; + } + + LOG_INFO(fuzz_logger, FUZZ_INIT_COMPLETE).arg(interface).arg(address) + .arg(port).arg(loop_max_); + } 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 @@ -92,69 +109,57 @@ Fuzz::~Fuzz() { static_cast(close(sockfd_)); } -// Parse IP address/port/interface and set up address structures. +// Set up address structures. void -Fuzz::setAddress(int ipversion) { +Fuzz::createAddressStructures(int ipversion, const char* interface, + const char* address, uint16_t port) { stringstream reason; // Used in error messages - // Get the environment for the fuzzing: interface, address and port. - interface_ = getenv("KEA_AFL_INTERFACE"); - if (! interface_) { - isc_throw(FuzzInitFail, "no fuzzing interface has been set"); - } - - // Now the address. (The port is specified via the "-p" command-line - // switch and passed to this object through the constructor.) - address_ = getenv("KEA_AFL_ADDRESS"); - if (address_ == 0) { - isc_throw(FuzzInitFail, "no fuzzing address has been set"); - } - // Set up the appropriate data structure depending on the address given. - if ((strstr(address_, ":") != NULL) && (ipversion == 6)) { + if ((ipversion == 6) && (strstr(address, ":") != NULL)) { // Expecting IPv6 and the address contains a colon, so assume it is an // an IPv6 address. 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(interface_); + servaddr6_.sin6_scope_id = if_nametoindex(interface); if (servaddr6_.sin6_scope_id == 0) { reason << "error retrieving interface ID for " - << interface_ << ": " << strerror(errno); + << interface << ": " << strerror(errno); isc_throw(FuzzInitFail, reason.str()); } sockaddr_ptr_ = reinterpret_cast(&servaddr6_); sockaddr_len_ = sizeof(servaddr6_); - } else if ((strstr(address_, ".") != NULL) && (ipversion == 4)) { + } else if ((ipversion == 4) && (strstr(address, ".") != NULL)) { // Expecting an IPv4 address and it contains a dot, so assume it is. // This check is done after the IPv6 check, as it is possible for an - // IPv4 address to be emnbedded in an IPv6 one. + // IPv4 address to be embedded in an IPv6 one. 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 { 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()); } @@ -165,7 +170,7 @@ Fuzz::setAddress(int ipversion) { // This is the main fuzzing function. It receives data from fuzzing engine over // stdin and then sends it to the configured UDP socket. void -Fuzz::transfer(void) { +Fuzz::transfer(void) const { // Read from stdin. Just return if nothing is read (or there is an error) // and hope that this does not cause a hang. @@ -175,8 +180,7 @@ Fuzz::transfer(void) { // Save the errno in case there was an error because if debugging is // enabled, the following LOG_DEBUG call may destroy its value. int errnum = errno; - LOG_DEBUG(fuzz_logger, FUZZ_DBG_TRACE_DETAIL, FUZZ_DATA_READ) - .arg(length); + LOG_DEBUG(fuzz_logger, FUZZ_DBG_TRACE_DETAIL, FUZZ_DATA_READ).arg(length); if (length > 0) { // Now send the data to the UDP port on which Kea is listening. diff --git a/src/lib/dhcpsrv/fuzz.h b/src/lib/dhcpsrv/fuzz.h index 0c6fd93eaa..c71e6c5387 100644 --- a/src/lib/dhcpsrv/fuzz.h +++ b/src/lib/dhcpsrv/fuzz.h @@ -54,7 +54,7 @@ public: /// /// This is below the maximum size of data that we will allow Kea to put /// into a single UDP datagram so as to avoid any "data too big" errors - /// when trying to send it to the port on which Kea lsutens. + /// when trying to send it to the port on which Kea listens. static constexpr size_t MAX_SEND_SIZE = 64000; /// @brief Number of packets Kea will process before shutting down. @@ -62,7 +62,7 @@ public: /// After the shutdown, AFL will restart it. This safety switch is here for /// eliminating cases where Kea goes into a weird state and stops /// processing packets properly. This can be overridden by setting the - /// environment variable FUZZ_AFL_LOOP_MAX. + /// environment variable KEA_AFL_LOOP_MAX. static constexpr long MAX_LOOP_COUNT = 1000; @@ -88,7 +88,7 @@ public: /// Called immediately prior to Kea reading data, this reads stdin (where /// AFL will have sent the packet being tested) and copies the data to the /// interface on which Kea is listening. - void transfer(void); + void transfer(void) const; /// @brief Return Max Loop Count /// @@ -103,23 +103,25 @@ public: } private: - /// @brief Populate address structures + /// @brief Create address structures /// - /// Decodes the environment variables used to pass address/port information - /// to the program and sets up the appropriate address structures. + /// Create the address structures describing the address/port on whick Kea + /// is listening for packets from AFL. /// /// @param ipversion Either 4 or 6 depending on which IP version address /// is expected. + /// @param interface Interface through which the fuzzer is sending packets + /// to Kea. + /// @param address Address on the interface that will be used. + /// @param port Port to be used. /// /// @throws FuzzInitFail Thrown if the address is not in the expected /// format. - void setAddress(int ipversion); + void createAddressStructures(int ipversion, const char* interface, + const char* address, uint16_t port); // Other member variables. - const char* address_; //< Pointer to address string - const char* interface_; //< Pointer to interface string long loop_max_; //< Maximum number of loop iterations - uint16_t port_; //< Port number to use size_t sockaddr_len_; //< Length of the structure struct sockaddr* sockaddr_ptr_; //< Pointer to structure used struct sockaddr_in servaddr4_; //< IPv6 address information