[sedhcpv6] works so moving to improvements to come (tests, tests and more tests!)

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 54191c2d4fb558a5a34db68fddcc4e26913d8f75..91327fd7227daf13e1edaeec9f168f4a45613fe1 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -2887,8 +2887,8 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
         bool has_pubkey = false;
         if (query->getOption(D6O_PUBLIC_KEY)) {
             has_pubkey = true;
-           LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
-               .arg("public key");
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
+                .arg("public key");
             if (query->getOptions(D6O_PUBLIC_KEY).size() > 1) {
                 answer->addOption(createStatusCode(STATUS_UnspecFail,
                             "More than one public key option"));
@@ -2899,8 +2899,8 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
         bool has_cert = false;
         if (query->getOption(D6O_CERTIFICATE)) {
             has_cert = true;
-           LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
-               .arg("certificate");
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
+                .arg("certificate");
             if (query->getOptions(D6O_CERTIFICATE).size() > 1) {
                 answer->addOption(createStatusCode(STATUS_UnspecFail,
                             "More than one certificate option"));
@@ -2933,10 +2933,12 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
         }
         // Unsecure
         if (!signopt_required && !signopt) {
-           LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, SEDHCP6_UNSECURE);
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, SEDHCP6_UNSECURE);
             return (true);
         }
         // signopt is true
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
+            .arg("signature");
         // Either public key or certificate must available
         if (!has_pubkey && !has_cert) {
             answer->addOption(createStatusCode(STATUS_UnspecFail,
@@ -2956,6 +2958,8 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
             return (false);
         }
         // Check algorithms
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+            .arg("checking algorithms");
         uint8_t ha_id = signature->readInteger<uint8_t>(0);
         if ((ha_id != SHA_256) && (ha_id != SHA_512)) {
             answer->addOption(createStatusCode(STATUS_AlgorithmNotSupported,
@@ -2978,6 +2982,8 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
             key_kind = CERT;
         }
         // Create the asym crypto object
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+            .arg("creating RSA objects");
         vector<uint8_t> keybin;
         if (has_pubkey) {
             keybin = query->getOption(D6O_PUBLIC_KEY)->getData();
@@ -3001,8 +3007,12 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
                         "Malformed certificate option"));
             return (false);
         }
-        // Compare with the credential when it is available
-        if (!host_credential.empty()) {
+        // Compare with the credential for authorization
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+            .arg(signopt_required ?
+                 "comparing with config" :
+                 "not comparing with config");
+        if (signopt_required) {
             CfgSeDhcp6::AsymPtr cred(crypto.createAsym(host_credential,
                                                        "",
                                                        sign_algo,
@@ -3022,6 +3032,8 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
             }
         }
         // Handle the timestamp option
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+            .arg("handling timestamp");
         OptionPtr tmstmp_opt;
         Ntp rd_new;
         Ntp ts_new;
@@ -3032,8 +3044,8 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
             tmstmp_opt = query->getOption(D6O_TIMESTAMP);
         }
         if (tmstmp_opt) {
-           LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
-               .arg("timestamp");
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_RECEIVED)
+                .arg("timestamp");
             // Get timestamps in NTP format
             vector<uint8_t> tmstmp_bin = tmstmp_opt->getData();
             if (!ts_new.from_binary(tmstmp_bin)) {
@@ -3049,6 +3061,9 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
             // Verify the given timestamp
             bool valid = false;
             if (rd_last.is_zero() || ts_last.is_zero()) {
+                LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
+                          SEDHCP6_INCOMING_TRACE)
+                    .arg("new client");
                 valid = Ntp::verify_new(rd_new, ts_new);
                 if (!valid) {
                     answer->addOption(createStatusCode(STATUS_TimestampFail,
@@ -3059,6 +3074,9 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
                     update_tmstmp = true;
                 }
             } else {
+                LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
+                          SEDHCP6_INCOMING_TRACE)
+                    .arg("known client");
                 valid = Ntp::verify(rd_new, ts_new, rd_last, ts_last,
                                     &update_tmstmp);
                 if (!valid) {
@@ -3069,15 +3087,56 @@ bool Dhcpv6Srv::validateSeDhcpOptions(const Pkt6Ptr& query, Pkt6Ptr& answer,
             }
         }
 
-        // TODO check signature
+        // Check the signature
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+            .arg("checking signature");
+        if (!query->getSignatureOffset()) {
+            LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_CHECK_FAIL)
+                .arg("null signature offset");
+            return (false);
+        }
+        if (query->getSignatureOffset() + signature->len() >
+            query->rawEnd() - query->rawBegin()) {
+            LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_CHECK_FAIL)
+                .arg("signature offset overflow");
+            return (false);
+        }
+        OptionBuffer tbs(query->rawBegin(), query->rawEnd());
+        size_t sig_off = query->getSignatureOffset();
+        sig_off += signature->getHeaderLen() + 2;
+        size_t sig_len = signature->len();
+        sig_len -= signature->getHeaderLen() + 2;
+        memset(&tbs[sig_off], 0, sig_len);
+        OptionBuffer sig(query->rawBegin() + sig_off,
+                         query->rawBegin() + sig_off + sig_len);
+        bool valid = false;
+        ostringstream vermsg("signature verify failed");
+        try {
+            key->update(&tbs[0], tbs.size());
+            valid = key->verify(&sig[0], sig_len, BASIC);
+        } catch (const Exception& ex) {
+            vermsg.str("signature verify failed: ");
+            vermsg << ex.what();
+        } catch (...) {
+            vermsg.str("signature verify failed?!");
+        }
+        if (!valid) {
+            answer->addOption(createStatusCode(STATUS_SignatureFail,
+                                               vermsg.str()));
+            return (false);
+        }
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+            .arg("signature is valid");
 
         // Update timestamps
         if (update_tmstmp) {
-           Host* hp = const_cast<Host*>(ctx.host_.get());
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE)
+                .arg("updating timestamps");
+            Host* hp = const_cast<Host*>(ctx.host_.get());
             hp->setRDlast(rd_new);
             hp->setTSlast(ts_new);
-           LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
-                     SEDHCP6_TIMESTAMP_UPDATED);
+            LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
+                      SEDHCP6_TIMESTAMP_UPDATED);
         }
     }
 
@@ -3099,17 +3158,17 @@ void Dhcpv6Srv::appendSeDhcpOptions(Pkt6Ptr& answer) {
     if (state->getSignAnswers() && key && cred) {
         // Add the credential (public key or certificate) option
         uint16_t cred_type = D6O_PUBLIC_KEY;
-       string opt_name = "public key";
+        string opt_name = "public key";
         if (cred->getAsymKeyKind() == CERT) {
             cred_type = D6O_CERTIFICATE;
-           opt_name = "certificate";
+            opt_name = "certificate";
         }
         OptionBuffer buf = cred->exportkey(cred->getAsymKeyKind(), ASN1);
         OptionPtr cred_opt(new Option(Option::V6, cred_type, buf));
         answer->addOption(cred_opt);
-       LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_ADDED)
-           .arg(opt_name)
-           .arg(cred_opt->len());
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_ADDED)
+            .arg(opt_name)
+            .arg(cred_opt->len());
 
         // Add the signature option
         uint8_t ha_id = SHA_256;
@@ -3117,7 +3176,7 @@ void Dhcpv6Srv::appendSeDhcpOptions(Pkt6Ptr& answer) {
             ha_id = SHA_512;
         }
         uint8_t sa_id = RSASSA_PKCS1v1_5;
-        size_t sig_len = (key->getKeySize() + 7) / 8;
+        size_t sig_len = key->getSignatureLength(BASIC);
         OptionBuffer sig(sig_len + 2);
         sig[0] = ha_id;
         sig[1] = sa_id;
@@ -3126,9 +3185,9 @@ void Dhcpv6Srv::appendSeDhcpOptions(Pkt6Ptr& answer) {
         assert(sig_def);
         OptionCustomPtr sig_opt(new OptionCustom(*sig_def, Option::V6, sig));
         answer->addOption(sig_opt);
-       LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_ADDED)
-           .arg("signature")
-           .arg(sig_opt->len());
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_ADDED)
+            .arg("signature")
+            .arg(sig_opt->len());
     }
 
     // Add timestamps
@@ -3139,9 +3198,9 @@ void Dhcpv6Srv::appendSeDhcpOptions(Pkt6Ptr& answer) {
         const OptionBuffer buf = val.to_binary();
         OptionPtr tmsmtp_opt(new Option(Option::V6, D6O_TIMESTAMP, buf));
         answer->addOption(tmsmtp_opt);
-       LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_ADDED)
-           .arg("timestamp")
-           .arg(tmsmtp_opt->len());
+        LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_OPTION_ADDED)
+            .arg("timestamp")
+            .arg(tmsmtp_opt->len());
     }
 }
 
@@ -3150,27 +3209,68 @@ void Dhcpv6Srv::finalizeSignature(Pkt6Ptr& tbs) {
     ConstCfgSeDhcp6Ptr state =
         CfgMgr::instance().getCurrentCfg()->getCfgSeDhcp6();
     if (!state) {
-       LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
-           .arg("no secure DHCPv6 configuration state");
-       return;
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("no secure DHCPv6 configuration state");
+        return;
     }
+
+    // Sanity checks (configuration)
     if (!state->getSignAnswers()) {
-       LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
-           .arg("Signing answers is disabled");
-       return;
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("Signing answers is disabled");
+        return;
     }
     CfgSeDhcp6::AsymPtr key = state->getPrivateKey();
     if (!key) {
-       LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
-           .arg("No private key configured");
-       return;
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("No private key configured");
+        return;
     }
+
+    // Sanity checks (offset)
     if (!tbs->getSignatureOffset()) {
-       LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
-           .arg("null signature offset");
-       return;
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("null signature offset");
+        return;
+    }
+    size_t sig_off0 = tbs->getSignatureOffset();
+    size_t sig_off = sig_off0 + Option::OPTION6_HDR_LEN + 2;
+    size_t sig_len = key->getSignatureLength(BASIC);
+    if (sig_off + sig_len > tbs->rawEnd() - tbs->rawBegin()) {
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("signature offset overflow");
+        return;
+    }
+
+    // Sanity checks (option)
+    uint16_t opt_type = *(tbs->rawBegin() + sig_off0) << 8;
+    opt_type |= *(tbs->rawBegin() + sig_off0 + 1);
+    if (opt_type != D6O_SIGNATURE) {
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("signature option type mismatch");
+        return;
+    }
+    uint16_t opt_len = *(tbs->rawBegin() + sig_off0 + 2) << 8;
+    opt_len |= *(tbs->rawBegin() + sig_off0 + 3);
+    if (opt_len != sig_len + 2) {
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("signature option length mismatch");
+        return;
+    }
+
+    uint8_t* start = const_cast<uint8_t*>(tbs->rawBegin());
+    try {
+        key->update(tbs->rawBegin(), tbs->rawEnd() - tbs->rawBegin());
+        key->sign(start + sig_off, sig_len, BASIC);
+    } catch (const Exception& ex) {
+        ostringstream sigmsg("signature sign failed: ");
+        sigmsg << ex.what();
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg(sigmsg.str());
+    } catch (...) {
+        LOG_ERROR(dhcp6_logger, SEDHCP6_SIGNATURE_FINALIZE_FAIL)
+            .arg("signature sign failed?!");
     }
-    // TODO
 }
 
 };

diff --git a/src/bin/dhcp6/sedhcp6_messages.mes b/src/bin/dhcp6/sedhcp6_messages.mes
index c98ccae435cca576e88c2f56bea191c76ddc9ef7..9c0ae141405981530443597d5dc90dc47693be22 100644 (file)
--- a/src/bin/dhcp6/sedhcp6_messages.mes
+++ b/src/bin/dhcp6/sedhcp6_messages.mes
@@ -22,6 +22,11 @@ to an answer packet.
 This debug message is printed when a secure DHCPv6 option is found
 during incoming packet validation.
 
+% SEDHCP6_SIGNATURE_CHECK_FAIL signature check error: %1
+This error message indicates that the signature check has failed.
+This is a "should not happen" condition which reflects an internal
+error.
+
 % SEDHCP6_SIGNATURE_FINALIZE_FAIL signature finalize error: %1
 This error message indicates that the signature finalize has failed.
 This is a "should not happen" condition which reflects an internal
@@ -30,6 +35,10 @@ error.
 % SEDHCP6_TIMESTAMP_UPDATED timestamps have been updated
 This debug message is printed when the host timestamps have been updated.
 
+% SEDHCP6_INCOMING_TRACE secure DHCPv6 processing: %1
+This debug message is a generic trace for secure DHCPv6 incoming packet
+processing.
+
 % SEDHCP6_UNSECURE incoming packet is unsecure
 This debug message is printed when the secure DHCPv6 validation
 concludes the incoming packet is unsecure.

diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index 13ad242f50571bcf888431c90526275738946d52..948c06a4ff1f8e71d5cfe823f4c7198538fff8f1 100644 (file)
--- a/src/lib/dhcp/pkt6.cc
+++ b/src/lib/dhcp/pkt6.cc
@@ -189,6 +189,8 @@ Pkt6::pack() {
 
 void
 Pkt6::packUDP() {
+    size_t begin_offset;
+    size_t end_offset;
     try {
         // Make sure that the buffer is empty before we start writting to it.
         buffer_out_.clear();
@@ -236,6 +238,9 @@ Pkt6::packUDP() {
 
         }
 
+        // Keep the offset at the beginning, i.e., after headers
+        begin_offset = buffer_out_.getLength();
+
         // DHCPv6 header: message-type (1 octect) + transaction id (3 octets)
         buffer_out_.writeUint8(msg_type_);
         // store 3-octet transaction-id
@@ -253,11 +258,19 @@ Pkt6::packUDP() {
             }
             it->second->pack(buffer_out_);
         }
+
+        // Keep the offset at the end, i.e., before tailers
+        end_offset = buffer_out_.getLength();
     }
     catch (const Exception& e) {
        // An exception is thrown and message will be written to Logger
        isc_throw(InvalidOperation, e.what());
     }
+    // Record the (possibly relayed) packet's boundaries
+    raw_begin_ = static_cast<const uint8_t*>(buffer_out_.getData())
+        + begin_offset;
+    raw_end_ = static_cast<const uint8_t*>(buffer_out_.getData())
+        + end_offset;
 }
 
 void
@@ -317,8 +330,8 @@ Pkt6::unpackMsg(OptionBufferConstIter begin, OptionBufferConstIter end) {
     }
 
     // Keep begin and end for secure DHCPv6
-    raw_begin_ = begin;
-    raw_end_ = end;
+    raw_begin_ = &(*begin);
+    raw_end_ = raw_begin_ + std::distance(begin, end);
 
     msg_type_ = *begin++;
 

diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index 4cb0774aadc19bb7e98b00b472694a6f882a3ae7..639a4663d7ce93f6e078ceba2193dc0537eff152 100644 (file)
--- a/src/lib/dhcp/pkt6.h
+++ b/src/lib/dhcp/pkt6.h
@@ -165,12 +165,12 @@ public:
     virtual std::string toText() const;
 
     /// @brief Returns the begin of the (possibly relayed) packet
-    OptionBufferConstIter rawBegin() const {
+    const uint8_t* rawBegin() const {
         return (raw_begin_);
     }
 
     /// @brief Returns the end of the (possibly relayed) packet
-    OptionBufferConstIter rawEnd() const {
+    const uint8_t* rawEnd() const {
         return (raw_end_);
     }
 
@@ -445,10 +445,10 @@ protected:
     uint8_t msg_type_;
 
     /// Begin of the raw packet
-    OptionBufferConstIter raw_begin_;
+    const uint8_t* raw_begin_;
 
     /// End of the raw packet
-    OptionBufferConstIter raw_end_;
+    const uint8_t* raw_end_;
 
     /// Offset of the signature option
     size_t signature_offset_;

diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc
index 8efada95551008a2db7752cb7bd11ce89cf49835..8d4bc9b9c16b374386ba6f788a9361269b6e3c42 100644 (file)
--- a/src/lib/dhcp/tests/pkt6_unittest.cc
+++ b/src/lib/dhcp/tests/pkt6_unittest.cc
@@ -287,7 +287,7 @@ TEST_F(Pkt6Test, unpack_solicit1) {
 
     // Check for length
     EXPECT_EQ(98, sol->len());
-    size_t raw_len = std::distance(sol->rawBegin(), sol->rawEnd());
+    size_t raw_len = sol->rawEnd() - sol->rawBegin();
     EXPECT_EQ(98, raw_len);
     EXPECT_EQ(0, sol->getSignatureOffset());
 
@@ -573,10 +573,10 @@ TEST_F(Pkt6Test, relayUnpack) {
     ASSERT_EQ(2, msg->relay_info_.size());
 
     // Check the raw stuff
-    OptionBufferConstIter begin = msg->rawBegin();
-    OptionBufferConstIter end = msg->rawEnd();
+    const uint8_t* begin = msg->rawBegin();
+    const uint8_t* end = msg->rawEnd();
     EXPECT_EQ(DHCPV6_SOLICIT, *begin);
-    EXPECT_EQ(54, std::distance(begin, end));
+    EXPECT_EQ(54, end - begin);
     EXPECT_EQ(0, msg->getSignatureOffset());
 
     OptionPtr opt;

diff --git a/src/lib/dhcpsrv/cfg_sedhcp6.h b/src/lib/dhcpsrv/cfg_sedhcp6.h
index 23e86d00ca707a1967e51a22df9f376b08df3b89..221306e21ff1617b176e9d9e5b306001f5692714 100644 (file)
--- a/src/lib/dhcpsrv/cfg_sedhcp6.h
+++ b/src/lib/dhcpsrv/cfg_sedhcp6.h
@@ -31,7 +31,7 @@ class CfgSeDhcp6 {
 public:
 
     /// @brief Pointer to constant asym crypto objet
-    typedef boost::shared_ptr<const isc::cryptolink::Asym> AsymPtr;
+    typedef boost::shared_ptr<isc::cryptolink::Asym> AsymPtr;
 
     /// @brief Constructor.
     CfgSeDhcp6(bool sign_answers = false,

diff --git a/src/lib/util/ntp_utils.cc b/src/lib/util/ntp_utils.cc
index 912ba8af8408a7008ff729cccc04f1b193662a04..09d2d046254685247abd0070945c87d324c5205c 100644 (file)
--- a/src/lib/util/ntp_utils.cc
+++ b/src/lib/util/ntp_utils.cc
@@ -58,10 +58,10 @@ Ntp::Ntp(const struct timeval* tv)
 
 Ntp::Ntp(const ptime pt)
 {
-    ptime epoch(date(1900, Jan, 1));
+    ptime epoch(date(1970, Jan, 1));
     time_duration dur(pt - epoch);
-    ntp_sec_ = static_cast<uint64_t>(dur.total_seconds());
-    uint64_t fcvt = dur.fractional_seconds() * 65536U;
+    ntp_sec_ = static_cast<uint32_t>(dur.total_seconds()) + EPOCH_ADJUST;
+    uint64_t fcvt = static_cast<uint64_t>(dur.fractional_seconds()) * 65536U;
     fcvt /= time_duration::ticks_per_second();
     ntp_fraction_ = static_cast<uint16_t>(fcvt & 0xffff);
 }
@@ -79,13 +79,19 @@ bool Ntp::from_binary(const std::vector<uint8_t> binary)
     if (binary.size() != 8) {
         return (false);
     }
-    ntp_sec_ = static_cast<uint64_t>(binary[0]) << 40;
-    ntp_sec_ |= static_cast<uint64_t>(binary[1]) << 32;
-    ntp_sec_ |= binary[2] << 24;
-    ntp_sec_ |= binary[3] << 16;
-    ntp_sec_ |= binary[4] << 8;
+    ntp_sec_ = binary[0];
+    ntp_sec_ <<= 8;
+    ntp_sec_ |= binary[1];
+    ntp_sec_ <<= 8;
+    ntp_sec_ |= binary[2];
+    ntp_sec_ <<= 8;
+    ntp_sec_ |= binary[3];
+    ntp_sec_ <<= 8;
+    ntp_sec_ |= binary[4];
+    ntp_sec_ <<= 8;
     ntp_sec_ |= binary[5];
-    ntp_fraction_ = binary[6] << 8;
+    ntp_fraction_ = binary[6];
+    ntp_fraction_ <<= 8;
     ntp_fraction_ |= binary[7];
     return (true);
 }
@@ -106,8 +112,9 @@ std::vector<uint8_t> Ntp::to_binary() const
 
 double Ntp::secs(time_t base) const
 {
-    double ret = static_cast<double>(ntp_sec_ - base - EPOCH_ADJUST);
-    ret += static_cast<double>(ntp_fraction_ * (1./65536.));
+    double ret = static_cast<double>(ntp_sec_);
+    ret -= base + EPOCH_ADJUST;
+    ret += static_cast<double>(ntp_fraction_) * (1./65536.);
     return (ret);
 }