Parcourir la source

[master] Merge branch 'trac2994' (Initial hooks for DHCPv4 server)

Conflicts:
	ChangeLog
	src/bin/dhcp4/dhcp4_messages.mes
	src/bin/dhcp4/dhcp4_srv.cc
	src/bin/dhcp4/dhcp4_srv.h
Tomek Mrugalski il y a 11 ans
Parent
commit
589f61f8a6

+ 5 - 0
ChangeLog

@@ -1,3 +1,8 @@
+645.	[func]		tomek
+	Added initial set of hooks (pk4_receive, subnet4_select,
+	lease4_select, pkt4_send) to the DHCPv6 server.
+	(Trac #2994, git be65cfba939a6a7abd3c93931ce35c33d3e8247b)
+
 644.	[func]		marcin
 	b10-dhcp4, b10-dhcp6: Implemented selection of the interfaces
 	that server listens on, using Configuration Manager. It is

+ 4 - 1
doc/devel/mainpage.dox

@@ -37,6 +37,8 @@
  * <a href="http://bind10.isc.org/">BIND10 webpage (http://bind10.isc.org)</a>
  * @section hooksFramework Hooks Framework
  * - @subpage hooksComponentDeveloperGuide
+ * - @subpage dhcpv4Hooks
+ * - @subpage dhcpv6Hooks
  *
  * @section dnsMaintenanceGuide DNS Maintenance Guide
  * - Authoritative DNS (todo)
@@ -48,11 +50,12 @@
  *   - @subpage dhcpv4Session
  *   - @subpage dhcpv4ConfigParser
  *   - @subpage dhcpv4ConfigInherit
+ *   - @subpage dhcpv4Other
  * - @subpage dhcp6
  *   - @subpage dhcpv6Session
  *   - @subpage dhcpv6ConfigParser
  *   - @subpage dhcpv6ConfigInherit
- *   - @subpage dhcpv6Hooks
+ *   - @subpage dhcpv6Other
  * - @subpage libdhcp
  *   - @subpage libdhcpIntro
  *   - @subpage libdhcpRelay

+ 4 - 0
src/bin/dhcp4/dhcp4.dox

@@ -79,4 +79,8 @@ See \ref dhcpv6ConfigParser.
 Configuration inheritance in DHCPv4 follows exactly the same logic as its DHCPv6
 counterpart. See \ref dhcpv6ConfigInherit.
 
+@section dhcpv4Other Other DHCPv4 topics
+
+ For hooks API support in DHCPv4, see @ref dhcpv4Hooks.
+
 */

+ 138 - 0
src/bin/dhcp4/dhcp4_hooks.dox

@@ -0,0 +1,138 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+/**
+ @page dhcpv4Hooks The Hooks API for the DHCPv4 Server
+
+ @section dhcpv4HooksIntroduction Introduction
+ BIND10 features an API (the "Hooks" API) that allows user-written code to
+ be integrated into BIND 10 and called at specific points in its processing.
+ An overview of the API and a tutorial for writing such code can be found in
+ the @ref hooksDevelopersGuide.  Information for BIND 10 maintainers can be
+ found in the @ref hooksComponentDeveloperGuide.
+
+ This manual is more specialised and is aimed at developers of hook
+ code for the DHCPv4 server. It describes each hook point, what the callouts
+ attached to the hook are able to do, and the arguments passed to the
+ callouts.  Each entry in this manual has the following information:
+
+ - Name of the hook point.
+ - Arguments for the callout.  As well as the argument name and data type, the
+   information includes the direction, which can be one of:
+   - @b in - the server passes values to the callout but ignored any data
+     returned.
+   - @b out - the callout is expected to set this value.
+   - <b>in/out</b> - the server passes a value to the callout and uses whatever
+     value the callout sends back.  Note that the callout may choose not to
+     do any modification, in which case the server will use whatever value
+     it sent to the callout.
+ - Description of the hook. This explains where in the processing the hook
+   is located, the possible actions a callout attached to this hook could take,
+   and a description of the data passed to the callouts.
+ - Skip flag action: the action taken by the server if a callout chooses to set
+    the "skip" flag.
+
+@section dhcpv4HooksHookPoints Hooks in the DHCPv4 Server
+
+The following list is ordered by appearance of specific hook points during
+packet processing. Hook points that are not specific to packet processing
+(e.g. lease expiration) will be added to the end of this list.
+
+ @subsection dhcpv4HooksPkt4Receive pkt4_receive
+
+ - @b Arguments:
+   - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
+
+ - @b Description: this callout is executed when an incoming DHCPv4
+   packet is received and its content is parsed. The sole argument -
+   query4 - contains a pointer to an isc::dhcp::Pkt4 object that contains
+   all information regarding incoming packet, including its source and
+   destination addresses, interface over which it was received, a list
+   of all options present within and relay information.  All fields of
+   the Pkt4 object can be modified at this time, except data_. (data_
+   contains the incoming packet as raw buffer. By the time this hook is
+   reached, that information has already parsed and is available though
+   other fields in the Pkt4 object.  For this reason, it doesn't make
+   sense to modify it.)
+
+ - <b>Skip flag action</b>: If any callout sets the skip flag, the server will
+   drop the packet and start processing the next one.  The reason for the drop
+   will be logged if logging is set to the appropriate debug level.
+
+@subsection dhcpv4HooksSubnet4Select subnet4_select
+
+ - @b Arguments:
+   - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
+   - name: @b subnet4, type: isc::dhcp::Subnet4Ptr, direction: <b>in/out</b>
+   - name: @b subnet4collection, type: const isc::dhcp::Subnet4Collection *, direction: <b>in</b>
+
+ - @b Description: this callout is executed when a subnet is being
+   selected for the incoming packet. All parameters and addresses
+   will be assigned from that subnet. A callout can select a
+   different subnet if it wishes so, the list of all subnets currently
+   configured being provided as 'subnet4collection'. The list itself must
+   not be modified.
+
+ - <b>Skip flag action</b>: If any callout installed on 'subnet4_select'
+   sets the skip flag, the server will not select any subnet. Packet processing
+   will continue, but will be severely limited (i.e. only global options
+   will be assigned).
+
+@subsection dhcpv4HooksLeaseSelect lease4_select
+
+ - @b Arguments:
+   - name: @b subnet4, type: isc::dhcp::Subnet4Ptr, direction: <b>in</b>
+   - name: @b fake_allocation, type: bool, direction: <b>in</b>
+   - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: <b>in/out</b>
+
+ - @b Description: this callout is executed after the server engine
+   has selected a lease for client's request but before the lease
+   has been inserted into the database. Any modifications made to the
+   isc::dhcp::Lease4 object will be stored in the lease's record in the
+   database. The callout should make sure that any modifications are
+   sanity checked as the server will use that data as is with no further
+   checking.\n\n The server processes lease requests for DISCOVER and
+   REQUEST in a very similar way. The only major difference is that
+   for DISCOVER the lease is just selected, but not inserted into
+   the database.  It is possible to distinguish between DISCOVER and
+   REQUEST by checking value of the fake_allocation flag: a value of true
+   means that the lease won't be inserted into the database (DISCOVER),
+   a value of false means that it will (REQUEST).
+
+ - <b>Skip flag action</b>: If any callout installed on 'lease4_select'
+   sets the skip flag, the server will not assign any lease. Packet
+   processing will continue, but client will not get an address.
+
+@subsection dhcpv4HooksPkt4Send pkt4_send
+
+ - @b Arguments:
+   - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: <b>in/out</b>
+
+ - @b Description: this callout is executed when server's response
+   is about to be send back to the client. The sole argument - response4 -
+   contains a pointer to an isc::dhcp::Pkt4 object that contains the
+   packet, with set source and destination addresses, interface over which
+   it will be send, list of all options and relay information.  All fields
+   of the Pkt4 object can be modified at this time, except bufferOut_.
+   (This is scratch space used for constructing the packet after all
+   pkt4_send callouts are complete, so any changes to that field will
+   be overwritten.)
+
+ - <b>Skip flag action</b>: if any callout sets the skip flag, the server
+   will drop this response packet. However, the original request packet
+   from a client was processed, so server's state was most likely changed
+   (e.g. lease was allocated). Setting this flag merely stops the change
+   being communicated to the client.
+
+*/

+ 3 - 0
src/bin/dhcp4/dhcp4_log.h

@@ -38,6 +38,9 @@ const int DBG_DHCP4_COMMAND = DBGLVL_COMMAND;
 // Trace basic operations within the code.
 const int DBG_DHCP4_BASIC = DBGLVL_TRACE_BASIC;
 
+// Trace hook related operations
+const int DBG_DHCP4_HOOKS = DBGLVL_TRACE_BASIC;
+
 // Trace detailed operations, including errors raised when processing invalid
 // packets.  (These are not logged at severities of WARN or higher for fear
 // that a set of deliberately invalid packets set to the server could overwhelm

+ 20 - 1
src/bin/dhcp4/dhcp4_messages.mes

@@ -42,7 +42,7 @@ This critical error message indicates that the initial DHCPv4
 configuration has failed. The server will start, but nothing will be
 served until the configuration has been corrected.
 
-% DHCP4_CONFIG_NEW_SUBNET A new subnet has been added to configuration: %1
+% DHCP4_CONFIG_NEW_SUBNET a new subnet has been added to configuration: %1
 This is an informational message reporting that the configuration has
 been extended to include the specified IPv4 subnet.
 
@@ -70,6 +70,25 @@ This message is printed when DHCPv4 server disables an interface from being
 used to receive DHCPv4 traffic. Sockets on this interface will not be opened
 by the Interface Manager until interface is enabled.
 
+% DHCP4_HOOK_PACKET_RCVD_SKIP received DHCPv4 packet was dropped, because a callout set the skip flag.
+This debug message is printed when a callout installed on the pkt4_receive
+hook point sets the skip flag. For this particular hook point, the
+setting of the flag instructs the server to drop the packet.
+
+% DHCP4_HOOK_PACKET_SEND_SKIP prepared DHCPv6 response was not sent, because a callout set skip flag.
+This debug message is printed when a callout installed on the pkt4_send
+hook point sets the skip flag. For this particular hook point, the setting
+of the flag instructs the server to drop the packet. This means that
+the client will not get any response, even though the server processed
+client's request and acted on it (e.g. possibly allocated a lease).
+
+% DHCP4_HOOK_SUBNET4_SELECT_SKIP no subnet was selected, because a callout set skip flag.
+This debug message is printed when a callout installed on the
+subnet4_select hook point sets the skip flag. For this particular hook
+point, the setting of the flag instructs the server not to choose a
+subnet, an action that severely limits further processing; the server
+will be only able to offer global options - no addresses will be assigned.
+
 % DHCP4_LEASE_ADVERT lease %1 advertised (client client-id %2, hwaddr %3)
 This debug message indicates that the server successfully advertised
 a lease. It is up to the client to choose one server out of othe advertised

+ 183 - 7
src/bin/dhcp4/dhcp4_srv.cc

@@ -30,6 +30,8 @@
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/utils.h>
 #include <dhcpsrv/addr_utilities.h>
+#include <hooks/hooks_manager.h>
+#include <hooks/callout_handle.h>
 
 #include <boost/algorithm/string/erase.hpp>
 
@@ -39,9 +41,30 @@
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using namespace isc::hooks;
 using namespace isc::log;
 using namespace std;
 
+/// Structure that holds registered hook indexes
+struct Dhcp6Hooks {
+    int hook_index_pkt4_receive_;   ///< index for "pkt4_receive" hook point
+    int hook_index_subnet4_select_; ///< index for "subnet4_select" hook point
+    int hook_index_pkt4_send_;      ///< index for "pkt4_send" hook point
+
+    /// Constructor that registers hook points for DHCPv6 engine
+    Dhcp6Hooks() {
+        hook_index_pkt4_receive_   = HooksManager::registerHook("pkt4_receive");
+        hook_index_subnet4_select_ = HooksManager::registerHook("subnet4_select");
+        hook_index_pkt4_send_      = HooksManager::registerHook("pkt4_send");
+    }
+};
+
+// Declare a Hooks object. As this is outside any function or method, it
+// will be instantiated (and the constructor run) when the module is loaded.
+// As a result, the hook indexes will be defined before any method in this
+// module is called.
+Dhcp6Hooks Hooks;
+
 namespace isc {
 namespace dhcp {
 
@@ -59,7 +82,10 @@ static const char* SERVER_ID_FILE = "b10-dhcp4-serverid";
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
                      const bool direct_response_desired)
-    : port_(port), use_bcast_(use_bcast) {
+: serverid_(), shutdown_(true), alloc_engine_(), port_(port), 
+    use_bcast_(use_bcast), hook_index_pkt4_receive_(-1), 
+    hook_index_subnet4_select_(-1), hook_index_pkt4_send_(-1) {
+
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
         // First call to instance() will create IfaceMgr (it's a singleton)
@@ -104,6 +130,13 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
         // Instantiate allocation engine
         alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
 
+        // Register hook points
+        hook_index_pkt4_receive_   = Hooks.hook_index_pkt4_receive_;
+        hook_index_subnet4_select_ = Hooks.hook_index_subnet4_select_;
+        hook_index_pkt4_send_      = Hooks.hook_index_pkt4_send_;
+
+        /// @todo call loadLibraries() when handling configuration changes
+
     } catch (const std::exception &e) {
         LOG_ERROR(dhcp4_logger, DHCP4_SRV_CONSTRUCT_ERROR).arg(e.what());
         shutdown_ = true;
@@ -123,6 +156,16 @@ Dhcpv4Srv::shutdown() {
     shutdown_ = true;
 }
 
+Pkt4Ptr
+Dhcpv4Srv::receivePacket(int timeout) {
+    return (IfaceMgr::instance().receive4(timeout));
+}
+
+void
+Dhcpv4Srv::sendPacket(const Pkt4Ptr& packet) {
+    IfaceMgr::instance().send(packet);
+}
+
 bool
 Dhcpv4Srv::run() {
     while (!shutdown_) {
@@ -135,7 +178,7 @@ Dhcpv4Srv::run() {
         Pkt4Ptr rsp;
 
         try {
-            query = IfaceMgr::instance().receive4(timeout);
+            query = receivePacket(timeout);
         } catch (const std::exception& e) {
             LOG_ERROR(dhcp4_logger, DHCP4_PACKET_RECEIVE_FAIL).arg(e.what());
         }
@@ -157,6 +200,31 @@ Dhcpv4Srv::run() {
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUERY_DATA)
                       .arg(query->toText());
 
+            // Let's execute all callouts registered for packet_received
+            if (HooksManager::calloutsPresent(hook_index_pkt4_receive_)) {
+                CalloutHandlePtr callout_handle = getCalloutHandle(query);
+
+                // Delete previously set arguments
+                callout_handle->deleteAllArguments();
+
+                // Pass incoming packet as argument
+                callout_handle->setArgument("query4", query);
+
+                // Call callouts
+                HooksManager::callCallouts(hook_index_pkt4_receive_,
+                                           *callout_handle);
+
+                // Callouts decided to skip the next processing step. The next
+                // processing step would to process the packet, so skip at this
+                // stage means drop.
+                if (callout_handle->getSkip()) {
+                    LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_RCVD_SKIP);
+                    continue;
+                }
+
+                callout_handle->getArgument("query4", query);
+            }
+
             try {
                 switch (query->getType()) {
                 case DHCPDISCOVER:
@@ -221,13 +289,39 @@ Dhcpv4Srv::run() {
                 rsp->setIface(query->getIface());
                 rsp->setIndex(query->getIndex());
 
+                // Execute all callouts registered for packet6_send
+                if (HooksManager::calloutsPresent(hook_index_pkt4_send_)) {
+                    CalloutHandlePtr callout_handle = getCalloutHandle(query);
+
+                    // Delete all previous arguments
+                    callout_handle->deleteAllArguments();
+
+                    // Clear skip flag if it was set in previous callouts
+                    callout_handle->setSkip(false);
+
+                    // Set our response
+                    callout_handle->setArgument("response4", rsp);
+
+                    // Call all installed callouts
+                    HooksManager::callCallouts(hook_index_pkt4_send_,
+                                               *callout_handle);
+
+                    // Callouts decided to skip the next processing step. The next
+                    // processing step would to send the packet, so skip at this
+                    // stage means "drop response".
+                    if (callout_handle->getSkip()) {
+                        LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_PACKET_SEND_SKIP);
+                        continue;
+                    }
+                }
+
                 LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
                           DHCP4_RESPONSE_DATA)
                           .arg(rsp->getType()).arg(rsp->toText());
 
                 if (rsp->pack()) {
                     try {
-                        IfaceMgr::instance().send(rsp);
+                        sendPacket(rsp);
                     } catch (const std::exception& e) {
                         LOG_ERROR(dhcp4_logger, DHCP4_PACKET_SEND_FAIL).arg(e.what());
                     }
@@ -515,12 +609,15 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     // allocation.
     bool fake_allocation = (question->getType() == DHCPDISCOVER);
 
+    CalloutHandlePtr callout_handle = getCalloutHandle(question);
+
     // Use allocation engine to pick a lease for this client. Allocation engine
     // will try to honour the hint, but it is just a hint - some other address
     // may be used instead. If fake_allocation is set to false, the lease will
     // be inserted into the LeaseMgr as well.
     Lease4Ptr lease = alloc_engine_->allocateAddress4(subnet, client_id, hwaddr,
-                                                      hint, fake_allocation);
+                                                      hint, fake_allocation,
+                                                      callout_handle);
 
     if (lease) {
         // We have a lease! Let's set it in the packet and send it back to
@@ -633,6 +730,9 @@ Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
 
 Pkt4Ptr
 Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
+
+    sanityCheck(discover, FORBIDDEN);
+
     Pkt4Ptr offer = Pkt4Ptr
         (new Pkt4(DHCPOFFER, discover->getTransid()));
 
@@ -783,17 +883,50 @@ Dhcpv4Srv::serverReceivedPacketName(uint8_t type) {
 Subnet4Ptr
 Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
 
+    Subnet4Ptr subnet;
     // Is this relayed message?
     IOAddress relay = question->getGiaddr();
-    if (relay.toText() == "0.0.0.0") {
+    static const IOAddress notset("0.0.0.0");
 
+    if (relay != notset) {
         // Yes: Use relay address to select subnet
-        return (CfgMgr::instance().getSubnet4(relay));
+        subnet = CfgMgr::instance().getSubnet4(relay);
     } else {
 
         // No: Use client's address to select subnet
-        return (CfgMgr::instance().getSubnet4(question->getRemoteAddr()));
+        subnet = CfgMgr::instance().getSubnet4(question->getRemoteAddr());
+    }
+
+    /// @todo Implement getSubnet4(interface-name)
+
+    // Let's execute all callouts registered for packet_received
+    if (HooksManager::calloutsPresent(hook_index_subnet4_select_)) {
+        CalloutHandlePtr callout_handle = getCalloutHandle(question);
+
+        // We're reusing callout_handle from previous calls
+        callout_handle->deleteAllArguments();
+
+        // Set new arguments
+        callout_handle->setArgument("query4", question);
+        callout_handle->setArgument("subnet4", subnet);
+        callout_handle->setArgument("subnet4collection", CfgMgr::instance().getSubnets4());
+
+        // Call user (and server-side) callouts
+        HooksManager::callCallouts(hook_index_subnet4_select_, *callout_handle);
+
+        // Callouts decided to skip this step. This means that no subnet will be
+        // selected. Packet processing will continue, but it will be severly limited
+        // (i.e. only global options will be assigned)
+        if (callout_handle->getSkip()) {
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_SUBNET4_SELECT_SKIP);
+            return (Subnet4Ptr());
+        }
+
+        // Use whatever subnet was specified by the callout
+        callout_handle->getArgument("subnet4", subnet);
     }
+
+    return (subnet);
 }
 
 void
@@ -819,6 +952,49 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) {
         // do nothing here
         ;
     }
+
+    // If there is HWAddress set and it is non-empty, then we're good
+    if (pkt->getHWAddr() && !pkt->getHWAddr()->hwaddr_.empty()) {
+        return;
+    }
+
+    // There has to be something to uniquely identify the client:
+    // either non-zero MAC address or client-id option present (or both)
+    OptionPtr client_id = pkt->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+
+    // If there's no client-id (or a useless one is provided, i.e. 0 length)
+    if (!client_id || client_id->len() == client_id->getHeaderLen()) {
+        isc_throw(RFCViolation, "Missing or useless client-id and no HW address "
+                  " provided in message "
+                  << serverReceivedPacketName(pkt->getType()));
+    }
+}
+
+isc::hooks::CalloutHandlePtr Dhcpv4Srv::getCalloutHandle(const Pkt4Ptr& pkt) {
+    // This method returns a CalloutHandle for a given packet. It is guaranteed
+    // to return the same callout_handle (so user library contexts are
+    // preserved). This method works well if the server processes one packet
+    // at a time. Once the server architecture is extended to cover parallel
+    // packets processing (e.g. delayed-ack, some form of buffering etc.), this
+    // method has to be extended (e.g. store callouts in a map and use pkt as
+    // a key). Additional code would be required to release the callout handle
+    // once the server finished processing.
+
+    CalloutHandlePtr callout_handle;
+    static Pkt4Ptr old_pointer;
+
+    if (!callout_handle ||
+        old_pointer != pkt) {
+        // This is the first packet or a different packet than previously
+        // passed to getCalloutHandle()
+
+        // Remember the pointer to this packet
+        old_pointer = pkt;
+
+        callout_handle = HooksManager::createCalloutHandle();
+    }
+
+    return (callout_handle);
 }
 
 void

+ 24 - 0
src/bin/dhcp4/dhcp4_srv.h

@@ -20,6 +20,7 @@
 #include <dhcp/option.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/alloc_engine.h>
+#include <hooks/callout_handle.h>
 
 #include <boost/noncopyable.hpp>
 
@@ -336,6 +337,18 @@ protected:
     /// initiate server shutdown procedure.
     volatile bool shutdown_;
 
+    /// @brief dummy wrapper around IfaceMgr::receive4
+    ///
+    /// This method is useful for testing purposes, where its replacement
+    /// simulates reception of a packet. For that purpose it is protected.
+    virtual Pkt4Ptr receivePacket(int timeout);
+
+    /// @brief dummy wrapper around IfaceMgr::send()
+    ///
+    /// This method is useful for testing purposes, where its replacement
+    /// simulates transmission of a packet. For that purpose it is protected.
+    virtual void sendPacket(const Pkt4Ptr& pkt);
+
 private:
 
     /// @brief Constructs netmask option based on subnet4
@@ -353,6 +366,17 @@ private:
     uint16_t port_;  ///< UDP port number on which server listens.
     bool use_bcast_; ///< Should broadcast be enabled on sockets (if true).
 
+    /// @brief returns callout handle for specified packet
+    ///
+    /// @param pkt packet for which the handle should be returned
+    ///
+    /// @return a callout handle to be used in hooks related to said packet
+    isc::hooks::CalloutHandlePtr getCalloutHandle(const Pkt4Ptr& pkt);
+
+    /// Indexes for registered hook points
+    int hook_index_pkt4_receive_;
+    int hook_index_subnet4_select_;
+    int hook_index_pkt4_send_;
 };
 
 }; // namespace isc::dhcp

+ 811 - 5
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -16,6 +16,7 @@
 #include <sstream>
 
 #include <asiolink/io_address.h>
+#include <config/ccsession.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/iface_mgr.h>
 #include <dhcp/option.h>
@@ -26,12 +27,15 @@
 #include <dhcp/pkt_filter_inet.h>
 #include <dhcp4/dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
+#include <dhcp4/config_parser.h>
 #include <hooks/server_hooks.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/utils.h>
 #include <gtest/gtest.h>
+#include <hooks/server_hooks.h>
+#include <hooks/hooks_manager.h>
 
 #include <boost/scoped_ptr.hpp>
 
@@ -43,7 +47,9 @@
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::data;
 using namespace isc::asiolink;
+using namespace isc::hooks;
 
 namespace {
 
@@ -80,6 +86,55 @@ public:
         : Dhcpv4Srv(port, "type=memfile", false, false) {
     }
 
+    /// @brief fakes packet reception
+    /// @param timeout ignored
+    ///
+    /// The method receives all packets queued in receive queue, one after
+    /// another. Once the queue is empty, it initiates the shutdown procedure.
+    ///
+    /// See fake_received_ field for description
+    virtual Pkt4Ptr receivePacket(int /*timeout*/) {
+
+        // If there is anything prepared as fake incoming traffic, use it
+        if (!fake_received_.empty()) {
+            Pkt4Ptr pkt = fake_received_.front();
+            fake_received_.pop_front();
+            return (pkt);
+        }
+
+        // If not, just trigger shutdown and return immediately
+        shutdown();
+        return (Pkt4Ptr());
+    }
+
+    /// @brief fake packet sending
+    ///
+    /// Pretend to send a packet, but instead just store it in fake_send_ list
+    /// where test can later inspect server's response.
+    virtual void sendPacket(const Pkt4Ptr& pkt) {
+        fake_sent_.push_back(pkt);
+    }
+
+    /// @brief adds a packet to fake receive queue
+    ///
+    /// See fake_received_ field for description
+    void fakeReceive(const Pkt4Ptr& pkt) {
+        fake_received_.push_back(pkt);
+    }
+
+    virtual ~NakedDhcpv4Srv() {
+    }
+
+    /// @brief packets we pretend to receive
+    ///
+    /// Instead of setting up sockets on interfaces that change between OSes, it
+    /// is much easier to fake packet reception. This is a list of packets that
+    /// we pretend to have received. You can schedule new packets to be received
+    /// using fakeReceive() and NakedDhcpv4Srv::receivePacket() methods.
+    list<Pkt4Ptr> fake_received_;
+
+    list<Pkt4Ptr> fake_sent_;
+
     using Dhcpv4Srv::adjustRemoteAddr;
     using Dhcpv4Srv::processDiscover;
     using Dhcpv4Srv::processRequest;
@@ -98,11 +153,11 @@ static const char* SRVID_FILE = "server-id-test.txt";
 
 /// @brief Dummy Packet Filtering class.
 ///
-/// This class reports capability to respond directly to the
-/// client which doesn't have address configured yet.
+/// This class reports capability to respond directly to the client which
+/// doesn't have address configured yet.
 ///
-/// All packet and socket handling functions do nothing because
-/// they are not used in unit tests.
+/// All packet and socket handling functions do nothing because they are not
+/// used in unit tests.
 class PktFilterTest : public PktFilter {
 public:
 
@@ -154,6 +209,16 @@ public:
 
         // it's ok if that fails. There should not be such a file anyway
         unlink(SRVID_FILE);
+
+        const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+
+        // There must be some interface detected
+        if (ifaces.empty()) {
+            // We can't use ASSERT in constructor
+            ADD_FAILURE() << "No interfaces detected.";
+        }
+
+        valid_iface_ = ifaces.begin()->getName();
     }
 
     virtual ~Dhcpv4SrvTest() {
@@ -565,6 +630,13 @@ public:
 
     /// @brief A client-id used in most tests
     ClientIdPtr client_id_;
+
+    int rcode_;
+
+    ConstElementPtr comment_;
+
+    // Name of a valid network interface
+    string valid_iface_;
 };
 
 // Sanity check. Verifies that both Dhcpv4Srv and its derived
@@ -968,6 +1040,7 @@ TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) {
     Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
     dis->setRemoteAddr(IOAddress("192.0.2.1"));
     dis->setYiaddr(hint);
+    dis->setHWAddr(generateHWAddr(6));
 
     // Pass it to the server and get an offer
     Pkt4Ptr offer = srv->processDiscover(dis);
@@ -1329,8 +1402,9 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
     ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
 
     Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    pkt->setHWAddr(generateHWAddr(6));
 
-    // Client-id is optional for information-request, so
+    // Server-id is optional for information-request, so
     EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::OPTIONAL));
 
     // Empty packet, no server-id
@@ -1344,6 +1418,11 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
     // Server-id is forbidden, but present => exception
     EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::FORBIDDEN),
                  RFCViolation);
+
+    // There's no client-id and no HWADDR. Server needs something to
+    // identify the client
+    pkt->setHWAddr(generateHWAddr(0));
+    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY), RFCViolation);
 }
 
 // This test verifies that incoming (positive) RELEASE can be handled properly.
@@ -1539,4 +1618,731 @@ TEST_F(Dhcpv4SrvTest, ServerID) {
     EXPECT_EQ(srvid_text, text);
 }
 
+/// @todo Implement tests for subnetSelect See tests in dhcp6_srv_unittest.cc:
+/// selectSubnetAddr, selectSubnetIface, selectSubnetRelayLinkaddr,
+/// selectSubnetRelayInterfaceId. Note that the concept of interface-id is not
+/// present in the DHCPv4, so not everything is applicable directly.
+/// See ticket #3057
+
+// Checks if hooks are registered properly.
+TEST_F(Dhcpv4SrvTest, Hooks) {
+    NakedDhcpv4Srv srv(0);
+
+    // check if appropriate hooks are registered
+    int hook_index_pkt4_received = -1;
+    int hook_index_select_subnet = -1;
+    int hook_index_pkt4_send     = -1;
+
+    // check if appropriate indexes are set
+    EXPECT_NO_THROW(hook_index_pkt4_received = ServerHooks::getServerHooks()
+                    .getIndex("pkt4_receive"));
+    EXPECT_NO_THROW(hook_index_select_subnet = ServerHooks::getServerHooks()
+                    .getIndex("subnet4_select"));
+    EXPECT_NO_THROW(hook_index_pkt4_send     = ServerHooks::getServerHooks()
+                    .getIndex("pkt4_send"));
+
+    EXPECT_TRUE(hook_index_pkt4_received > 0);
+    EXPECT_TRUE(hook_index_select_subnet > 0);
+    EXPECT_TRUE(hook_index_pkt4_send > 0);
+}
+
+    // a dummy MAC address
+    const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
+
+    // A dummy MAC address, padded with 0s
+    const uint8_t dummyChaddr[16] = {0, 1, 2, 3, 4, 5, 0, 0,
+                                     0, 0, 0, 0, 0, 0, 0, 0 };
+
+    // Let's use some creative test content here (128 chars + \0)
+    const uint8_t dummyFile[] = "Lorem ipsum dolor sit amet, consectetur "
+        "adipiscing elit. Proin mollis placerat metus, at "
+        "lacinia orci ornare vitae. Mauris amet.";
+
+    // Yet another type of test content (64 chars + \0)
+    const uint8_t dummySname[] = "Lorem ipsum dolor sit amet, consectetur "
+        "adipiscing elit posuere.";
+
+/// @brief a class dedicated to Hooks testing in DHCPv4 server
+///
+/// This class has a number of static members, because each non-static
+/// method has implicit 'this' parameter, so it does not match callout
+/// signature and couldn't be registered. Furthermore, static methods
+/// can't modify non-static members (for obvious reasons), so many
+/// fields are declared static. It is still better to keep them as
+/// one class rather than unrelated collection of global objects.
+class HooksDhcpv4SrvTest : public Dhcpv4SrvTest {
+
+public:
+
+    /// @brief creates Dhcpv4Srv and prepares buffers for callouts
+    HooksDhcpv4SrvTest() {
+
+        // Allocate new DHCPv6 Server
+        srv_ = new NakedDhcpv4Srv(0);
+
+        // clear static buffers
+        resetCalloutBuffers();
+    }
+
+    /// @brief destructor (deletes Dhcpv4Srv)
+    virtual ~HooksDhcpv4SrvTest() {
+
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("pkt4_receive");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("pkt4_send");
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("subnet4_select");
+
+        delete srv_;
+    }
+
+    /// @brief creates an option with specified option code
+    ///
+    /// This method is static, because it is used from callouts
+    /// that do not have a pointer to HooksDhcpv4SSrvTest object
+    ///
+    /// @param option_code code of option to be created
+    ///
+    /// @return pointer to create option object
+    static OptionPtr createOption(uint16_t option_code) {
+
+        char payload[] = {
+            0xa, 0xb, 0xc, 0xe, 0xf, 0x10, 0x11, 0x12, 0x13, 0x14
+        };
+
+        OptionBuffer tmp(payload, payload + sizeof(payload));
+        return OptionPtr(new Option(Option::V4, option_code, tmp));
+    }
+
+    /// @brief Generates test packet.
+    ///
+    /// Allocates and generates on-wire buffer that represents test packet, with all
+    /// fixed fields set to non-zero values.  Content is not always reasonable.
+    ///
+    /// See generateTestPacket1() function that returns exactly the same packet as
+    /// Pkt4 object.
+    ///
+    /// @return pointer to allocated Pkt4 object
+    // Returns a vector containing a DHCPv4 packet header.
+    Pkt4Ptr
+    generateSimpleDiscover() {
+
+        // That is only part of the header. It contains all "short" fields,
+        // larger fields are constructed separately.
+        uint8_t hdr[] = {
+            1, 6, 6, 13,            // op, htype, hlen, hops,
+            0x12, 0x34, 0x56, 0x78, // transaction-id
+            0, 42, 0x80, 0x00,      // 42 secs, BROADCAST flags
+            192, 0, 2, 1,           // ciaddr
+            1, 2, 3, 4,             // yiaddr
+            192, 0, 2, 255,         // siaddr
+            255, 255, 255, 255,     // giaddr
+        };
+
+        // Initialize the vector with the header fields defined above.
+        vector<uint8_t> buf(hdr, hdr + sizeof(hdr));
+
+        // Append the large header fields.
+        copy(dummyChaddr, dummyChaddr + Pkt4::MAX_CHADDR_LEN, back_inserter(buf));
+        copy(dummySname, dummySname + Pkt4::MAX_SNAME_LEN, back_inserter(buf));
+        copy(dummyFile, dummyFile + Pkt4::MAX_FILE_LEN, back_inserter(buf));
+
+        // Should now have all the header, so check.  The "static_cast" is used
+        // to get round an odd bug whereby the linker appears not to find the
+        // definition of DHCPV4_PKT_HDR_LEN if it appears within an EXPECT_EQ().
+        EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), buf.size());
+
+        // Add magic cookie
+        buf.push_back(0x63);
+        buf.push_back(0x82);
+        buf.push_back(0x53);
+        buf.push_back(0x63);
+
+        // Add message type DISCOVER
+        buf.push_back(static_cast<uint8_t>(DHO_DHCP_MESSAGE_TYPE));
+        buf.push_back(1); // length (just one byte)
+        buf.push_back(static_cast<uint8_t>(DHCPDISCOVER));
+
+        return (Pkt4Ptr(new Pkt4(&buf[0], buf.size())));
+    }
+
+    /// test callback that stores received callout name and pkt4 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_receive_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("pkt4_receive");
+
+        callout_handle.getArgument("query4", callback_pkt4_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    /// test callback that changes client-id value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_receive_change_clientid(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("query4", pkt);
+
+        // get rid of the old client-id
+        pkt->delOption(DHO_DHCP_CLIENT_IDENTIFIER);
+
+        // add a new option
+        pkt->addOption(createOption(DHO_DHCP_CLIENT_IDENTIFIER));
+
+        // carry on as usual
+        return pkt4_receive_callout(callout_handle);
+    }
+
+    /// test callback that deletes client-id
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_receive_delete_clientid(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("query4", pkt);
+
+        // get rid of the old client-id (and no HWADDR)
+        vector<uint8_t> mac;
+        pkt->delOption(DHO_DHCP_CLIENT_IDENTIFIER);
+        pkt->setHWAddr(1, 0, mac); // HWtype 1, hwardware len = 0
+
+        // carry on as usual
+        return pkt4_receive_callout(callout_handle);
+    }
+
+    /// test callback that sets skip flag
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_receive_skip(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("query4", pkt);
+
+        callout_handle.setSkip(true);
+
+        // carry on as usual
+        return pkt4_receive_callout(callout_handle);
+    }
+
+    /// Test callback that stores received callout name and pkt4 value
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_send_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("pkt4_send");
+
+        callout_handle.getArgument("response4", callback_pkt4_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    // Test callback that changes server-id
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_send_change_serverid(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("response4", pkt);
+
+        // get rid of the old server-id
+        pkt->delOption(DHO_DHCP_SERVER_IDENTIFIER);
+
+        // add a new option
+        pkt->addOption(createOption(DHO_DHCP_SERVER_IDENTIFIER));
+
+        // carry on as usual
+        return pkt4_send_callout(callout_handle);
+    }
+
+    /// test callback that deletes server-id
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_send_delete_serverid(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("response4", pkt);
+
+        // get rid of the old client-id
+        pkt->delOption(DHO_DHCP_SERVER_IDENTIFIER);
+
+        // carry on as usual
+        return pkt4_send_callout(callout_handle);
+    }
+
+    /// Test callback that sets skip flag
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    pkt4_send_skip(CalloutHandle& callout_handle) {
+
+        Pkt4Ptr pkt;
+        callout_handle.getArgument("response4", pkt);
+
+        callout_handle.setSkip(true);
+
+        // carry on as usual
+        return pkt4_send_callout(callout_handle);
+    }
+
+    /// Test callback that stores received callout name and subnet4 values
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    subnet4_select_callout(CalloutHandle& callout_handle) {
+        callback_name_ = string("subnet4_select");
+
+        callout_handle.getArgument("query4", callback_pkt4_);
+        callout_handle.getArgument("subnet4", callback_subnet4_);
+        callout_handle.getArgument("subnet4collection", callback_subnet4collection_);
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    /// Test callback that picks the other subnet if possible.
+    /// @param callout_handle handle passed by the hooks framework
+    /// @return always 0
+    static int
+    subnet4_select_different_subnet_callout(CalloutHandle& callout_handle) {
+
+        // Call the basic calllout to record all passed values
+        subnet4_select_callout(callout_handle);
+
+        const Subnet4Collection* subnets;
+        Subnet4Ptr subnet;
+        callout_handle.getArgument("subnet4", subnet);
+        callout_handle.getArgument("subnet4collection", subnets);
+
+        // Let's change to a different subnet
+        if (subnets->size() > 1) {
+            subnet = (*subnets)[1]; // Let's pick the other subnet
+            callout_handle.setArgument("subnet4", subnet);
+        }
+
+        return (0);
+    }
+
+    /// resets buffers used to store data received by callouts
+    void resetCalloutBuffers() {
+        callback_name_ = string("");
+        callback_pkt4_.reset();
+        callback_subnet4_.reset();
+        callback_subnet4collection_ = NULL;
+        callback_argument_names_.clear();
+    }
+
+    /// pointer to Dhcpv4Srv that is used in tests
+    NakedDhcpv4Srv* srv_;
+
+    // The following fields are used in testing pkt4_receive_callout
+
+    /// String name of the received callout
+    static string callback_name_;
+
+    /// Pkt4 structure returned in the callout
+    static Pkt4Ptr callback_pkt4_;
+
+    /// Pointer to a subnet received by callout
+    static Subnet4Ptr callback_subnet4_;
+
+    /// A list of all available subnets (received by callout)
+    static const Subnet4Collection* callback_subnet4collection_;
+
+    /// A list of all received arguments
+    static vector<string> callback_argument_names_;
+};
+
+// The following fields are used in testing pkt4_receive_callout.
+// See fields description in the class for details
+string HooksDhcpv4SrvTest::callback_name_;
+Pkt4Ptr HooksDhcpv4SrvTest::callback_pkt4_;
+Subnet4Ptr HooksDhcpv4SrvTest::callback_subnet4_;
+const Subnet4Collection* HooksDhcpv4SrvTest::callback_subnet4collection_;
+vector<string> HooksDhcpv4SrvTest::callback_argument_names_;
+
+
+// Checks if callouts installed on pkt4_received are indeed called and the
+// all necessary parameters are passed.
+//
+// Note that the test name does not follow test naming convention,
+// but the proper hook name is "pkt4_receive".
+TEST_F(HooksDhcpv4SrvTest, simple_pkt4_receive) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_receive", pkt4_receive_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // check that the callback called is indeed the one we installed
+    EXPECT_EQ("pkt4_receive", callback_name_);
+
+    // check that pkt4 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_pkt4_.get() == sol.get());
+
+    // Check that all expected parameters are there
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back(string("query4"));
+
+    EXPECT_TRUE(expected_argument_names == callback_argument_names_);
+}
+
+// Checks if callouts installed on pkt4_received is able to change
+// the values and the parameters are indeed used by the server.
+TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_receive) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_receive", pkt4_receive_change_clientid));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // check that the server did send a reposonce
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+
+    // Make sure that we received a response
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+    ASSERT_TRUE(adv);
+
+    // Get client-id...
+    OptionPtr clientid = adv->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+
+    // ... and check if it is the modified value
+    OptionPtr expected = createOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    EXPECT_TRUE(clientid->equal(expected));
+}
+
+// Checks if callouts installed on pkt4_received is able to delete
+// existing options and that change impacts server processing (mandatory
+// client-id option is deleted, so the packet is expected to be dropped)
+TEST_F(HooksDhcpv4SrvTest, deleteClientId_pkt4_receive) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_receive", pkt4_receive_delete_clientid));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // Check that the server dropped the packet and did not send a response
+    ASSERT_EQ(0, srv_->fake_sent_.size());
+}
+
+// Checks if callouts installed on pkt4_received is able to set skip flag that
+// will cause the server to not process the packet (drop), even though it is valid.
+TEST_F(HooksDhcpv4SrvTest, skip_pkt4_receive) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_receive", pkt4_receive_skip));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // check that the server dropped the packet and did not produce any response
+    ASSERT_EQ(0, srv_->fake_sent_.size());
+}
+
+
+// Checks if callouts installed on pkt4_send are indeed called and the
+// all necessary parameters are passed.
+TEST_F(HooksDhcpv4SrvTest, simple_pkt4_send) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_send", pkt4_send_callout));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("pkt4_send", callback_name_);
+
+    // Check that there is one packet sent
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+
+    // Check that pkt4 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_pkt4_.get() == adv.get());
+
+    // Check that all expected parameters are there
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back(string("response4"));
+    EXPECT_TRUE(expected_argument_names == callback_argument_names_);
+}
+
+// Checks if callouts installed on pkt4_send is able to change
+// the values and the packet sent contains those changes
+TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_send) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_send", pkt4_send_change_serverid));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // check that the server did send a reposonce
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+
+    // Make sure that we received a response
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+    ASSERT_TRUE(adv);
+
+    // Get client-id...
+    OptionPtr clientid = adv->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+
+    // ... and check if it is the modified value
+    OptionPtr expected = createOption(DHO_DHCP_SERVER_IDENTIFIER);
+    EXPECT_TRUE(clientid->equal(expected));
+}
+
+// Checks if callouts installed on pkt4_send is able to delete
+// existing options and that server applies those changes. In particular,
+// we are trying to send a packet without server-id. The packet should
+// be sent
+TEST_F(HooksDhcpv4SrvTest, deleteServerId_pkt4_send) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_send", pkt4_send_delete_serverid));
+
+    // Let's create a simple DISCOVER
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // Check that the server indeed sent a malformed ADVERTISE
+    ASSERT_EQ(1, srv_->fake_sent_.size());
+
+    // Get that ADVERTISE
+    Pkt4Ptr adv = srv_->fake_sent_.front();
+    ASSERT_TRUE(adv);
+
+    // Make sure that it does not have server-id
+    EXPECT_FALSE(adv->getOption(DHO_DHCP_SERVER_IDENTIFIER));
+}
+
+// Checks if callouts installed on pkt4_skip is able to set skip flag that
+// will cause the server to not process the packet (drop), even though it is valid.
+TEST_F(HooksDhcpv4SrvTest, skip_pkt4_send) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "pkt4_send", pkt4_send_skip));
+
+    // Let's create a simple REQUEST
+    Pkt4Ptr sol = Pkt4Ptr(generateSimpleDiscover());
+
+    // Simulate that we have received that traffic
+    srv_->fakeReceive(sol);
+
+    // Server will now process to run its normal loop, but instead of calling
+    // IfaceMgr::receive4(), it will read all packets from the list set by
+    // fakeReceive()
+    // In particular, it should call registered pkt4_receive callback.
+    srv_->run();
+
+    // check that the server dropped the packet and did not produce any response
+    ASSERT_EQ(0, srv_->fake_sent_.size());
+}
+
+// This test checks if subnet4_select callout is triggered and reports
+// valid parameters
+TEST_F(HooksDhcpv4SrvTest, subnet4_select) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "subnet4_select", subnet4_select_callout));
+
+    // Configure 2 subnets, both directly reachable over local interface
+    // (let's not complicate the matter with relays)
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.0/25\" ],"
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"interface\": \"" + valid_iface_ + "\" "
+        " }, {"
+        "    \"pool\": [ \"192.0.3.0/25\" ],"
+        "    \"subnet\": \"192.0.3.0/24\" "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ConstElementPtr status;
+
+    // Configure the server and make sure the config is accepted
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    comment_ = config::parseAnswer(rcode_, status);
+    ASSERT_EQ(0, rcode_);
+
+    // Prepare discover packet. Server should select first subnet for it
+    Pkt4Ptr sol = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    sol->setRemoteAddr(IOAddress("192.0.2.1"));
+    sol->setIface(valid_iface_);
+    OptionPtr clientid = generateClientId();
+    sol->addOption(clientid);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr adv = srv_->processDiscover(sol);
+
+    // check if we get response at all
+    ASSERT_TRUE(adv);
+
+    // Check that the callback called is indeed the one we installed
+    EXPECT_EQ("subnet4_select", callback_name_);
+
+    // Check that pkt4 argument passing was successful and returned proper value
+    EXPECT_TRUE(callback_pkt4_.get() == sol.get());
+
+    const Subnet4Collection* exp_subnets = CfgMgr::instance().getSubnets4();
+
+    // The server is supposed to pick the first subnet, because of matching
+    // interface. Check that the value is reported properly.
+    ASSERT_TRUE(callback_subnet4_);
+    EXPECT_EQ(exp_subnets->front().get(), callback_subnet4_.get());
+
+    // Server is supposed to report two subnets
+    ASSERT_EQ(exp_subnets->size(), callback_subnet4collection_->size());
+
+    // Compare that the available subnets are reported as expected
+    EXPECT_TRUE((*exp_subnets)[0].get() == (*callback_subnet4collection_)[0].get());
+    EXPECT_TRUE((*exp_subnets)[1].get() == (*callback_subnet4collection_)[1].get());
+}
+
+// This test checks if callout installed on subnet4_select hook point can pick
+// a different subnet.
+TEST_F(HooksDhcpv4SrvTest, subnet_select_change) {
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "subnet4_select", subnet4_select_different_subnet_callout));
+
+    // Configure 2 subnets, both directly reachable over local interface
+    // (let's not complicate the matter with relays)
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.0/25\" ],"
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"interface\": \"" + valid_iface_ + "\" "
+        " }, {"
+        "    \"pool\": [ \"192.0.3.0/25\" ],"
+        "    \"subnet\": \"192.0.3.0/24\" "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+    ConstElementPtr status;
+
+    // Configure the server and make sure the config is accepted
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    comment_ = config::parseAnswer(rcode_, status);
+    ASSERT_EQ(0, rcode_);
+
+    // Prepare discover packet. Server should select first subnet for it
+    Pkt4Ptr sol = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    sol->setRemoteAddr(IOAddress("192.0.2.1"));
+    sol->setIface(valid_iface_);
+    OptionPtr clientid = generateClientId();
+    sol->addOption(clientid);
+
+    // Pass it to the server and get an advertise
+    Pkt4Ptr adv = srv_->processDiscover(sol);
+
+    // check if we get response at all
+    ASSERT_TRUE(adv);
+
+    // The response should have an address from second pool, so let's check it
+    IOAddress addr = adv->getYiaddr();
+    EXPECT_NE("0.0.0.0", addr.toText());
+
+    // Get all subnets and use second subnet for verification
+    const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4();
+    ASSERT_EQ(2, subnets->size());
+
+    // Advertised address must belong to the second pool (in subnet's range,
+    // in dynamic pool)
+    EXPECT_TRUE((*subnets)[1]->inRange(addr));
+    EXPECT_TRUE((*subnets)[1]->inPool(addr));
+}
+
+
+
 } // end of anonymous namespace

+ 4 - 0
src/bin/dhcp6/dhcp6.dox

@@ -92,4 +92,8 @@
 
  @todo Add section about setting up options and their definitions with bindctl.
 
+ @section dhcpv6Other Other DHCPv6 topics
+
+ For hooks API support in DHCPv6, see @ref dhcpv6Hooks.
+
  */

+ 4 - 2
src/bin/dhcp6/dhcp6_hooks.dox

@@ -110,8 +110,10 @@ packet processing. Hook points that are not specific to packet processing
    means that the lease won't be inserted into the database (SOLICIT),
    a value of false means that it will (REQUEST).
 
- - <b>Skip flag action</b>: the "skip" flag is ignored by the server on this
-   hook.
+ - <b>Skip flag action</b>: If any callout installed on 'lease6_select'
+   sets the skip flag, the server will not assign that particular lease.
+   Packet processing will continue and the client may get other addresses
+   or prefixes if it requested more than one address and/or prefix.
 
 @subsection dhcpv6HooksPkt6Send pkt6_send
 

+ 17 - 15
src/bin/dhcp6/dhcp6_messages.mes

@@ -76,23 +76,25 @@ used to receive DHCPv6 traffic. Sockets on this interface will not be opened
 by the Interface Manager until interface is enabled.
 
 % DHCP6_HOOK_PACKET_RCVD_SKIP received DHCPv6 packet was dropped, because a callout set skip flag.
-This debug message is printed when a callout installed on pkt6_received
-hook point sets skip flag. For this particular hook point, the setting
-of the flag by a callout instructs the server to drop the packet.
+This debug message is printed when a callout installed on the pkt6_receive
+hook point sets the skip flag. For this particular hook point, the
+setting of the flag by a callout instructs the server to drop the packet.
 
-% DHCP6_HOOK_PACKET_SEND_SKIP Prepared DHCPv6 response was not sent, because a callout set skip flag.
-This debug message is printed when a callout installed on pkt6_send
-hook point sets skip flag. For this particular hook point, the setting
+% DHCP6_HOOK_PACKET_SEND_SKIP prepared DHCPv6 response was not sent, because a callout set skip flag.
+This debug message is printed when a callout installed on the pkt6_send
+hook point sets the skip flag. For this particular hook point, the setting
 of the flag by a callout instructs the server to drop the packet. This
 effectively means that the client will not get any response, even though
 the server processed client's request and acted on it (e.g. possibly
 allocated a lease).
 
-% DHCP6_HOOK_SUBNET6_SELECT_SKIP No subnet was selected, because a callout set skip flag.
-This debug message is printed when a callout installed on subnet6_select
-hook point sets a skip flag. It means that the server was told that no subnet
-should be selected. This severely limits further processing - server will be only
-able to offer global options. No addresses or prefixes could be assigned.
+% DHCP6_HOOK_SUBNET6_SELECT_SKIP no subnet was selected, because a callout set skip flag.
+This debug message is printed when a callout installed on the
+subnet6_select hook point sets the skip flag. For this particular hook
+point, the setting of the flag instructs the server not to choose a
+subnet, an action that severely limits further processing; the server
+will be only able to offer global options - no addresses or prefixes
+will be assigned.
 
 % DHCP6_LEASE_ADVERT lease %1 advertised (client duid=%2, iaid=%3)
 This debug message indicates that the server successfully advertised
@@ -193,10 +195,10 @@ actions and committal of changes failed.  The message has been output in
 response to a non-BIND 10 exception being raised.  Additional messages
 may give further information.
 
-The most likely cause of this is that the specification file for the server
-(which details the allowable contents of the configuration) is not correct for
-this version of BIND 10.  This former may be the result of an interrupted
-installation of an update to BIND 10.
+The most likely cause of this is that the specification file for the
+server (which details the allowable contents of the configuration) is
+not correct for this version of BIND 10.  This may be the result of an
+interrupted installation of an update to BIND 10.
 
 % DHCP6_PARSER_FAIL failed to create or run parser for configuration element %1: %2
 On receipt of message containing details to a change of its configuration,

+ 0 - 21
src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -34,7 +34,6 @@
 #include <dhcpsrv/utils.h>
 #include <util/buffer.h>
 #include <util/range_utilities.h>
-
 #include <hooks/server_hooks.h>
 #include <hooks/hooks_manager.h>
 
@@ -1855,26 +1854,6 @@ TEST_F(Dhcpv6SrvTest, Hooks) {
     EXPECT_TRUE(hook_index_pkt6_send > 0);
 }
 
-// This function returns buffer for empty packet (just DHCPv6 header)
-Pkt6* captureEmpty() {
-    Pkt6* pkt;
-    uint8_t data[4];
-    data[0] = 1; // type 1 = SOLICIT
-    data[1] = 0xca; // trans-id = 0xcafe01
-    data[2] = 0xfe;
-    data[3] = 0x01;
-
-    pkt = new Pkt6(data, sizeof(data));
-    pkt->setRemotePort(546);
-    pkt->setRemoteAddr(IOAddress("fe80::1"));
-    pkt->setLocalPort(0);
-    pkt->setLocalAddr(IOAddress("ff02::1:2"));
-    pkt->setIndex(2);
-    pkt->setIface("eth0");
-
-    return (pkt);
-}
-
 // This function returns buffer for very simple Solicit
 Pkt6* captureSimpleSolicit() {
     Pkt6* pkt;

+ 98 - 16
src/lib/dhcpsrv/alloc_engine.cc

@@ -29,11 +29,13 @@ using namespace isc::hooks;
 namespace {
 
 /// Structure that holds registered hook indexes
-struct Dhcp6Hooks {
+struct AllocEngineHooks {
+    int hook_index_lease4_select_; ///< index for "lease4_receive" hook point
     int hook_index_lease6_select_; ///< index for "lease6_receive" hook point
 
     /// Constructor that registers hook points for AllocationEngine
-    Dhcp6Hooks() {
+    AllocEngineHooks() {
+        hook_index_lease4_select_ = HooksManager::registerHook("lease4_select");
         hook_index_lease6_select_ = HooksManager::registerHook("lease6_select");
     }
 };
@@ -42,7 +44,7 @@ struct Dhcp6Hooks {
 // will be instantiated (and the constructor run) when the module is loaded.
 // As a result, the hook indexes will be defined before any method in this
 // module is called.
-Dhcp6Hooks Hooks;
+AllocEngineHooks Hooks;
 
 }; // anonymous namespace
 
@@ -187,6 +189,7 @@ AllocEngine::AllocEngine(AllocType engine_type, unsigned int attempts)
     }
 
     // Register hook points
+    hook_index_lease4_select_ = Hooks.hook_index_lease4_select_;
     hook_index_lease6_select_ = Hooks.hook_index_lease6_select_;
 }
 
@@ -312,7 +315,8 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
                               const ClientIdPtr& clientid,
                               const HWAddrPtr& hwaddr,
                               const IOAddress& hint,
-                              bool fake_allocation /* = false */ ) {
+                              bool fake_allocation,
+                              const isc::hooks::CalloutHandlePtr& callout_handle) {
 
     try {
         // Allocator is always created in AllocEngine constructor and there is
@@ -365,7 +369,8 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
                 /// implemented
 
                 // The hint is valid and not currently used, let's create a lease for it
-                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint, fake_allocation);
+                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint,
+                                               callout_handle, fake_allocation);
 
                 // It can happen that the lease allocation failed (we could have lost
                 // the race condition. That means that the hint is lo longer usable and
@@ -376,7 +381,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
             } else {
                 if (existing->expired()) {
                     return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                              fake_allocation));
+                                              callout_handle, fake_allocation));
                 }
 
             }
@@ -410,7 +415,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
                 // there's no existing lease for selected candidate, so it is
                 // free. Let's allocate it.
                 Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, candidate,
-                                              fake_allocation);
+                                               callout_handle, fake_allocation);
                 if (lease) {
                     return (lease);
                 }
@@ -421,7 +426,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
             } else {
                 if (existing->expired()) {
                     return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                              fake_allocation));
+                                              callout_handle, fake_allocation));
                 }
             }
 
@@ -513,9 +518,9 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
 
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
-        // won't be inserted into the
+        // won't be inserted into the database.
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP);
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_SELECT_SKIP);
             return (Lease6Ptr());
         }
 
@@ -541,6 +546,7 @@ Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
                                          const SubnetPtr& subnet,
                                          const ClientIdPtr& clientid,
                                          const HWAddrPtr& hwaddr,
+                                         const isc::hooks::CalloutHandlePtr& callout_handle,
                                          bool fake_allocation /*= false */ ) {
 
     if (!expired->expired()) {
@@ -563,6 +569,44 @@ Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
     /// @todo: log here that the lease was reused (there's ticket #2524 for
     /// logging in libdhcpsrv)
 
+    // Let's execute all callouts registered for lease4_select
+    if (callout_handle &&
+        HooksManager::getHooksManager().calloutsPresent(hook_index_lease4_select_)) {
+
+        // Delete all previous arguments
+        callout_handle->deleteAllArguments();
+
+        // Pass necessary arguments
+
+        // Subnet from which we do the allocation. Convert the general subnet
+        // pointer to a pointer to a Subnet4.  Note that because we are using
+        // boost smart pointers here, we need to do the cast using the boost
+        // version of dynamic_pointer_cast.
+        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(subnet);
+        callout_handle->setArgument("subnet4", subnet4);
+
+        // Is this solicit (fake = true) or request (fake = false)
+        callout_handle->setArgument("fake_allocation", fake_allocation);
+
+        // The lease that will be assigned to a client
+        callout_handle->setArgument("lease4", expired);
+
+        // Call the callouts
+        HooksManager::callCallouts(hook_index_lease6_select_, *callout_handle);
+
+        // Callouts decided to skip the action. This means that the lease is not
+        // assigned, so the client will get NoAddrAvail as a result. The lease
+        // won't be inserted into the database.
+        if (callout_handle->getSkip()) {
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+            return (Lease4Ptr());
+        }
+
+        // Let's use whatever callout returned. Hopefully it is the same lease
+        // we handled to it.
+        callout_handle->getArgument("lease4", expired);
+    }
+
     if (!fake_allocation) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease4(expired);
@@ -587,16 +631,13 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
                                subnet->getPreferred(), subnet->getValid(),
                                subnet->getT1(), subnet->getT2(), subnet->getID()));
 
-    // Let's execute all callouts registered for lease6_ia_added
+    // Let's execute all callouts registered for lease6_select
     if (callout_handle &&
         HooksManager::getHooksManager().calloutsPresent(hook_index_lease6_select_)) {
 
         // Delete all previous arguments
         callout_handle->deleteAllArguments();
 
-        // Clear skip flag if it was set in previous callouts
-        callout_handle->setSkip(false);
-
         // Pass necessary arguments
 
         // Subnet from which we do the allocation
@@ -611,9 +652,9 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
 
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
-        // won't be inserted into the
+        // won't be inserted into the database.
         if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP);
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE6_SELECT_SKIP);
             return (Lease6Ptr());
         }
 
@@ -654,6 +695,7 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
                                     const DuidPtr& clientid,
                                     const HWAddrPtr& hwaddr,
                                     const IOAddress& addr,
+                                    const isc::hooks::CalloutHandlePtr& callout_handle,
                                     bool fake_allocation /*= false */ ) {
     if (!hwaddr) {
         isc_throw(BadValue, "Can't create a lease with NULL HW address");
@@ -671,6 +713,46 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
                                subnet->getT1(), subnet->getT2(), now,
                                subnet->getID()));
 
+    // Let's execute all callouts registered for lease4_select
+    if (callout_handle &&
+        HooksManager::getHooksManager().calloutsPresent(hook_index_lease4_select_)) {
+
+        // Delete all previous arguments
+        callout_handle->deleteAllArguments();
+
+        // Pass necessary arguments
+
+        // Subnet from which we do the allocation (That's as far as we can go
+        // with using SubnetPtr to point to Subnet4 object. Users should not
+        // be confused with dynamic_pointer_casts. They should get a concrete
+        // pointer (Subnet4Ptr) pointing to a Subnet4 object.
+        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(subnet);
+        callout_handle->setArgument("subnet4", subnet4);
+
+        // Is this solicit (fake = true) or request (fake = false)
+        callout_handle->setArgument("fake_allocation", fake_allocation);
+
+        // Pass the intended lease as well
+        callout_handle->setArgument("lease4", lease);
+
+        // This is the first callout, so no need to clear any arguments
+        HooksManager::callCallouts(hook_index_lease4_select_, *callout_handle);
+
+        // Callouts decided to skip the action. This means that the lease is not
+        // assigned, so the client will get NoAddrAvail as a result. The lease
+        // won't be inserted into the database.
+        if (callout_handle->getSkip()) {
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+            return (Lease4Ptr());
+        }
+
+        // Let's use whatever callout returned. Hopefully it is the same lease
+        // we handled to it.
+        callout_handle->getArgument("lease4", lease);
+    }
+
+
+
     if (!fake_allocation) {
         // That is a real (REQUEST) allocation
         bool status = LeaseMgrFactory::instance().addLease(lease);

+ 16 - 4
src/lib/dhcpsrv/alloc_engine.h

@@ -194,13 +194,16 @@ protected:
     /// @param hint a hint that the client provided
     /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
     ///        an address for DISCOVER that is not really allocated (true)
+    /// @param callout_handle a callout handle (used in hooks). A lease callouts
+    ///        will be executed if this parameter is passed.
     /// @return Allocated IPv4 lease (or NULL if allocation failed)
     Lease4Ptr
     allocateAddress4(const SubnetPtr& subnet,
                      const ClientIdPtr& clientid,
                      const HWAddrPtr& hwaddr,
                      const isc::asiolink::IOAddress& hint,
-                     bool fake_allocation);
+                     bool fake_allocation,
+                     const isc::hooks::CalloutHandlePtr& callout_handle);
 
     /// @brief Renews a IPv4 lease
     ///
@@ -262,6 +265,9 @@ private:
     /// @param clientid client identifier
     /// @param hwaddr client's hardware address
     /// @param addr an address that was selected and is confirmed to be available
+    /// @param callout_handle a callout handle (used in hooks). A lease callouts
+    ///        will be executed if this parameter is passed (and there are callouts
+    ///        registered)
     /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
     ///        an address for DISCOVER that is not really allocated (true)
     /// @return allocated lease (or NULL in the unlikely case of the lease just
@@ -269,6 +275,7 @@ private:
     Lease4Ptr createLease4(const SubnetPtr& subnet, const DuidPtr& clientid,
                            const HWAddrPtr& hwaddr,
                            const isc::asiolink::IOAddress& addr,
+                           const isc::hooks::CalloutHandlePtr& callout_handle,
                            bool fake_allocation = false);
 
     /// @brief creates a lease and inserts it in LeaseMgr if necessary
@@ -282,7 +289,8 @@ private:
     /// @param iaid IAID from the IA_NA container the client sent to us
     /// @param addr an address that was selected and is confirmed to be available
     /// @param callout_handle a callout handle (used in hooks). A lease callouts
-    ///        will be executed if this parameter is passed.
+    ///        will be executed if this parameter is passed (and there are callouts
+    ///        registered)
     /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
     ///        an address for SOLICIT that is not really allocated (true)
     /// @return allocated lease (or NULL in the unlikely case of the lease just
@@ -302,6 +310,8 @@ private:
     /// @param subnet subnet the lease is allocated from
     /// @param clientid client identifier
     /// @param hwaddr client's hardware address
+    /// @param callout_handle a callout handle (used in hooks). A lease callouts
+    ///        will be executed if this parameter is passed.
     /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
     ///        an address for DISCOVER that is not really allocated (true)
     /// @return refreshed lease
@@ -309,6 +319,7 @@ private:
     Lease4Ptr reuseExpiredLease(Lease4Ptr& expired, const SubnetPtr& subnet,
                                 const ClientIdPtr& clientid,
                                 const HWAddrPtr& hwaddr,
+                                const isc::hooks::CalloutHandlePtr& callout_handle,
                                 bool fake_allocation = false);
 
     /// @brief Reuses expired IPv6 lease
@@ -338,8 +349,9 @@ private:
     /// @brief number of attempts before we give up lease allocation (0=unlimited)
     unsigned int attempts_;
 
-    /// @brief hook name index (used in hooks callouts)
-    int hook_index_lease6_select_;
+    // hook name indexes (used in hooks callouts)
+    int hook_index_lease4_select_; ///< index for lease4_select hook
+    int hook_index_lease6_select_; ///< index for lease6_select hook
 };
 
 }; // namespace isc::dhcp

+ 9 - 1
src/lib/dhcpsrv/cfgmgr.h

@@ -212,6 +212,15 @@ public:
     /// completely new?
     void deleteSubnets6();
 
+    /// @brief returns const reference to all subnets6
+    ///
+    /// This is used in a hook (subnet4_select), where the hook is able
+    /// to choose a different subnet. Server code has to offer a list
+    /// of possible choices (i.e. all subnets).
+    /// @return a pointer to const Subnet6 collection
+    const Subnet4Collection* getSubnets4() {
+        return (&subnets4_);
+    }
 
     /// @brief returns const reference to all subnets6
     ///
@@ -223,7 +232,6 @@ public:
         return (&subnets6_);
     }
 
-
     /// @brief get IPv4 subnet by address
     ///
     /// Finds a matching subnet, based on an address. This can be used

+ 11 - 5
src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -135,11 +135,17 @@ the database access parameters are changed: in the latter case, the
 server closes the currently open database, and opens a database using
 the new parameters.
 
-% DHCPSRV_HOOK_LEASE6_IA_ADD_SKIP Lease6 (non-temporary) creation was skipped, because of callout skip flag.
-This debug message is printed when a callout installed on lease6_assign
-hook point sets a skip flag. It means that the server was told that no lease6
-should be assigned. The server will not put that lease in its database and the client
-will get a NoAddrsAvail for that IA_NA option.
+% DHCPSRV_HOOK_LEASE4_SELECT_SKIP Lease4 creation was skipped, because of callout skip flag.
+This debug message is printed when a callout installed on lease4_select
+hook point sets the skip flag. It means that the server was told that
+no lease4 should be assigned. The server will not put that lease in its
+database and the client will get a NAK packet.
+
+% DHCPSRV_HOOK_LEASE6_SELECT_SKIP Lease6 (non-temporary) creation was skipped, because of callout skip flag.
+This debug message is printed when a callout installed on lease6_select
+hook point sets the skip flag. It means that the server was told that
+no lease6 should be assigned. The server will not put that lease in its
+database and the client will get a NoAddrsAvail for that IA_NA option.
 
 % DHCPSRV_INVALID_ACCESS invalid database access string: %1
 This is logged when an attempt has been made to parse a database access string

+ 242 - 18
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -35,6 +35,7 @@
 
 #include <iostream>
 #include <sstream>
+#include <algorithm>
 #include <set>
 #include <time.h>
 
@@ -592,7 +593,8 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
     ASSERT_TRUE(engine);
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
-                                               IOAddress("0.0.0.0"), false);
+                                               IOAddress("0.0.0.0"), false,
+                                               CalloutHandlePtr());
 
     // Check that we got a lease
     ASSERT_TRUE(lease);
@@ -615,7 +617,8 @@ TEST_F(AllocEngine4Test, fakeAlloc4) {
     ASSERT_TRUE(engine);
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
-                                               IOAddress("0.0.0.0"), true);
+                                               IOAddress("0.0.0.0"), true,
+                                               CalloutHandlePtr());
 
     // Check that we got a lease
     ASSERT_TRUE(lease);
@@ -638,7 +641,7 @@ TEST_F(AllocEngine4Test, allocWithValidHint4) {
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("192.0.2.105"),
-                                               false);
+                                               false, CalloutHandlePtr());
 
     // Check that we got a lease
     ASSERT_TRUE(lease);
@@ -678,7 +681,7 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
     // twice.
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("192.0.2.106"),
-                                               false);
+                                               false, CalloutHandlePtr());
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -712,7 +715,7 @@ TEST_F(AllocEngine4Test, allocBogusHint4) {
     // with the normal allocation
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
                                                IOAddress("10.1.1.1"),
-                                               false);
+                                               false, CalloutHandlePtr());
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -739,18 +742,19 @@ TEST_F(AllocEngine4Test, allocateAddress4Nulls) {
 
     // Allocations without subnet are not allowed
     Lease4Ptr lease = engine->allocateAddress4(SubnetPtr(), clientid_, hwaddr_,
-                                               IOAddress("0.0.0.0"), false);
+                                               IOAddress("0.0.0.0"), false,
+                                               CalloutHandlePtr());
     EXPECT_FALSE(lease);
 
     // Allocations without HW address are not allowed
     lease = engine->allocateAddress4(subnet_, clientid_, HWAddrPtr(),
-                                     IOAddress("0.0.0.0"), false);
+                                     IOAddress("0.0.0.0"), false, CalloutHandlePtr());
     EXPECT_FALSE(lease);
 
     // Allocations without client-id are allowed
     clientid_ = ClientIdPtr();
     lease = engine->allocateAddress4(subnet_, ClientIdPtr(), hwaddr_,
-                                     IOAddress("0.0.0.0"), false);
+                                     IOAddress("0.0.0.0"), false, CalloutHandlePtr());
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -854,7 +858,7 @@ TEST_F(AllocEngine4Test, smallPool4) {
     cfg_mgr.addSubnet4(subnet_);
 
     Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
-                                               false);
+                                               false, CalloutHandlePtr());
 
     // Check that we got that single lease
     ASSERT_TRUE(lease);
@@ -902,7 +906,8 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
     // else, so the allocation should fail
 
     Lease4Ptr lease2 = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
-                                                IOAddress("0.0.0.0"), false);
+                                                IOAddress("0.0.0.0"), false,
+                                                CalloutHandlePtr());
     EXPECT_FALSE(lease2);
 }
 
@@ -935,7 +940,7 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
 
     // CASE 1: Asking for any address
     lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
-                                     true);
+                                     true, CalloutHandlePtr());
     // Check that we got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr.toText(), lease->addr_.toText());
@@ -945,7 +950,7 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
 
     // CASE 2: Asking specifically for this address
     lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress(addr.toText()),
-                                     true);
+                                     true, CalloutHandlePtr());
     // Check that we got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr.toText(), lease->addr_.toText());
@@ -972,7 +977,8 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
 
     // A client comes along, asking specifically for this address
     lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
-                                     IOAddress(addr.toText()), false);
+                                     IOAddress(addr.toText()), false,
+                                     CalloutHandlePtr());
 
     // Check that he got that single lease
     ASSERT_TRUE(lease);
@@ -986,6 +992,8 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     detailCompareLease(lease, from_mgr);
 }
 
+/// @todo write renewLease6
+
 // This test checks if a lease is really renewed when renewLease4 method is
 // called
 TEST_F(AllocEngine4Test, renewLease4) {
@@ -1144,13 +1152,13 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
 
     // Initialize Hooks Manager
     vector<string> libraries; // no libraries at this time
-    HooksManager::getHooksManager().loadLibraries(libraries);
+    HooksManager::loadLibraries(libraries);
 
     // Install pkt6_receive_callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "lease6_select", lease6_select_callout));
 
-    CalloutHandlePtr callout_handle = HooksManager::getHooksManager().createCalloutHandle();
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
 
     Lease6Ptr lease = engine->allocateAddress6(subnet_, duid_, iaid_, IOAddress("::"),
                                                false, callout_handle);
@@ -1174,7 +1182,7 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     ASSERT_TRUE(callback_subnet6_);
     EXPECT_EQ(subnet_->toText(), callback_subnet6_->toText());
 
-    EXPECT_EQ(callback_fake_allocation_, false);
+    EXPECT_FALSE(callback_fake_allocation_);
 
     // Check if all expected parameters are reported. It's a bit tricky, because
     // order may be different. If the test starts failing, because someone tweaked
@@ -1183,6 +1191,10 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     expected_argument_names.push_back("fake_allocation");
     expected_argument_names.push_back("lease6");
     expected_argument_names.push_back("subnet6");
+
+    sort(callback_argument_names_.begin(), callback_argument_names_.end());
+    sort(expected_argument_names.begin(), expected_argument_names.end());
+
     EXPECT_TRUE(callback_argument_names_ == expected_argument_names);
 }
 
@@ -1205,7 +1217,7 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
 
     // Initialize Hooks Manager
     vector<string> libraries; // no libraries at this time
-    HooksManager::getHooksManager().loadLibraries(libraries);
+    HooksManager::loadLibraries(libraries);
 
     // Install a callout
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
@@ -1213,7 +1225,7 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
 
     // Normally, dhcpv6_srv would passed the handle when calling allocateAddress6,
     // but in tests we need to create it on our own.
-    CalloutHandlePtr callout_handle = HooksManager::getHooksManager().createCalloutHandle();
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
 
     // Call allocateAddress6. Callouts should be triggered here.
     Lease6Ptr lease = engine->allocateAddress6(subnet_, duid_, iaid_, IOAddress("::"),
@@ -1240,4 +1252,216 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
     EXPECT_EQ(valid_override_, from_mgr->valid_lft_);
 }
 
+
+/// @brief helper class used in Hooks testing in AllocEngine4
+///
+/// It features a couple of callout functions and buffers to store
+/// the data that is accessible via callouts.
+class HookAllocEngine4Test : public AllocEngine4Test {
+public:
+    HookAllocEngine4Test() {
+        resetCalloutBuffers();
+    }
+
+    virtual ~HookAllocEngine4Test() {
+        HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+            "lease4_select");
+    }
+
+    /// @brief clears out buffers, so callouts can store received arguments
+    void resetCalloutBuffers() {
+        callback_name_ = string("");
+        callback_subnet4_.reset();
+        callback_fake_allocation_ = false;
+        callback_lease4_.reset();
+        callback_argument_names_.clear();
+        callback_addr_original_ = IOAddress("::");
+        callback_addr_updated_ = IOAddress("::");
+    }
+
+    /// callback that stores received callout name and received values
+    static int
+    lease4_select_callout(CalloutHandle& callout_handle) {
+
+        callback_name_ = string("lease4_select");
+
+        callout_handle.getArgument("subnet4", callback_subnet4_);
+        callout_handle.getArgument("fake_allocation", callback_fake_allocation_);
+        callout_handle.getArgument("lease4", callback_lease4_);
+
+        callback_addr_original_ = callback_lease4_->addr_;
+
+        callback_argument_names_ = callout_handle.getArgumentNames();
+        return (0);
+    }
+
+    /// callback that overrides the lease with different values
+    static int
+    lease4_select_different_callout(CalloutHandle& callout_handle) {
+
+        // Let's call the basic callout, so it can record all parameters
+        lease4_select_callout(callout_handle);
+
+        // Now we need to tweak the least a bit
+        Lease4Ptr lease;
+        callout_handle.getArgument("lease4", lease);
+        callback_addr_updated_ = addr_override_;
+        lease->addr_ = callback_addr_updated_;
+        lease->t1_ = t1_override_;
+        lease->t2_ = t2_override_;
+        lease->valid_lft_ = valid_override_;
+
+        return (0);
+    }
+
+    // Values to be used in callout to override lease4 content
+    static const IOAddress addr_override_;
+    static const uint32_t t1_override_;
+    static const uint32_t t2_override_;
+    static const uint32_t valid_override_;
+
+    // Callback will store original and overridden values here
+    static IOAddress callback_addr_original_;
+    static IOAddress callback_addr_updated_;
+
+    // Buffers (callback will store received values here)
+    static string callback_name_;
+    static Subnet4Ptr callback_subnet4_;
+    static Lease4Ptr callback_lease4_;
+    static bool callback_fake_allocation_;
+    static vector<string> callback_argument_names_;
+};
+
+// For some reason intialization within a class makes the linker confused.
+// linker complains about undefined references if they are defined within
+// the class declaration.
+const IOAddress HookAllocEngine4Test::addr_override_("192.0.3.1");
+const uint32_t HookAllocEngine4Test::t1_override_ = 4000;
+const uint32_t HookAllocEngine4Test::t2_override_ = 7000;
+const uint32_t HookAllocEngine4Test::valid_override_ = 9000;
+
+IOAddress HookAllocEngine4Test::callback_addr_original_("::");
+IOAddress HookAllocEngine4Test::callback_addr_updated_("::");
+
+string HookAllocEngine4Test::callback_name_;
+Subnet4Ptr HookAllocEngine4Test::callback_subnet4_;
+Lease4Ptr HookAllocEngine4Test::callback_lease4_;
+bool HookAllocEngine4Test::callback_fake_allocation_;
+vector<string> HookAllocEngine4Test::callback_argument_names_;
+
+// This test checks if the lease4_select callout is executed and expected
+// parameters as passed.
+TEST_F(HookAllocEngine4Test, lease4_select) {
+
+    // Note: The following order is working as expected:
+    // 1. create AllocEngine (that register hook points)
+    // 2. call loadLibraries()
+    //
+    // This order, however, causes segfault in HooksManager
+    // 1. call loadLibraries()
+    // 2. create AllocEngine (that register hook points)
+
+    // Create allocation engine (hook names are registered in its ctor)
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
+    ASSERT_TRUE(engine);
+
+    // Initialize Hooks Manager
+    vector<string> libraries; // no libraries at this time
+    HooksManager::loadLibraries(libraries);
+
+    // Install pkt4_receive_callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_select", lease4_select_callout));
+
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+
+    Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
+                                               IOAddress("0.0.0.0"),
+                                               false, callout_handle);
+    // Check that we got a lease
+    ASSERT_TRUE(lease);
+
+    // Do all checks on the lease
+    checkLease4(lease);
+
+    // Check that the lease is indeed in LeaseMgr
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+
+    // Check that callouts were indeed called
+    EXPECT_EQ("lease4_select", callback_name_);
+
+    // Now check that the lease in LeaseMgr has the same parameters
+    ASSERT_TRUE(callback_lease4_);
+    detailCompareLease(callback_lease4_, from_mgr);
+
+    ASSERT_TRUE(callback_subnet4_);
+    EXPECT_EQ(subnet_->toText(), callback_subnet4_->toText());
+
+    EXPECT_EQ(callback_fake_allocation_, false);
+
+    // Check if all expected parameters are reported. It's a bit tricky, because
+    // order may be different. If the test starts failing, because someone tweaked
+    // hooks engine, we'll have to implement proper vector matching (ignoring order)
+    vector<string> expected_argument_names;
+    expected_argument_names.push_back("fake_allocation");
+    expected_argument_names.push_back("lease4");
+    expected_argument_names.push_back("subnet4");
+    EXPECT_TRUE(callback_argument_names_ == expected_argument_names);
+}
+
+// This test checks if lease4_select callout is able to override the values
+// in a lease4.
+TEST_F(HookAllocEngine4Test, change_lease4_select) {
+
+    // Make sure that the overridden values are different than the ones from
+    // subnet originally used to create the lease
+    ASSERT_NE(t1_override_, subnet_->getT1());
+    ASSERT_NE(t2_override_, subnet_->getT2());
+    ASSERT_NE(valid_override_, subnet_->getValid());
+    ASSERT_FALSE(subnet_->inRange(addr_override_));
+
+    // Create allocation engine (hook names are registered in its ctor)
+    boost::scoped_ptr<AllocEngine> engine;
+    ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
+    ASSERT_TRUE(engine);
+
+    // Initialize Hooks Manager
+    vector<string> libraries; // no libraries at this time
+    HooksManager::loadLibraries(libraries);
+
+    // Install a callout
+    EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+                        "lease4_select", lease4_select_different_callout));
+
+    // Normally, dhcpv4_srv would passed the handle when calling allocateAddress4,
+    // but in tests we need to create it on our own.
+    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+
+    // Call allocateAddress4. Callouts should be triggered here.
+    Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),
+                                               false, callout_handle);
+    // Check that we got a lease
+    ASSERT_TRUE(lease);
+
+    // See if the values overridden by callout are there
+    EXPECT_TRUE(lease->addr_.equals(addr_override_));
+    EXPECT_EQ(t1_override_, lease->t1_);
+    EXPECT_EQ(t2_override_, lease->t2_);
+    EXPECT_EQ(valid_override_, lease->valid_lft_);
+
+    // Now check if the lease is in the database
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+
+    // Check if values in the database are overridden
+    EXPECT_TRUE(from_mgr->addr_.equals(addr_override_));
+    EXPECT_EQ(t1_override_, from_mgr->t1_);
+    EXPECT_EQ(t2_override_, from_mgr->t2_);
+    EXPECT_EQ(valid_override_, from_mgr->valid_lft_);
+}
+
+
+
 }; // End of anonymous namespace