From: Francis Dupont Date: Sun, 31 May 2015 18:29:20 +0000 (+0200) Subject: [sedhcpv6] works so moving to improvements to come (tests, tests and more tests!) X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=6de07084e23a83f6b76f5d2b9b2ddd2461d4443c;p=thirdparty%2Fkea.git [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 54191c2d4f..91327fd722 100644 --- 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(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 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 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(ctx.host_.get()); + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, SEDHCP6_INCOMING_TRACE) + .arg("updating timestamps"); + Host* hp = const_cast(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(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 c98ccae435..9c0ae14140 100644 --- 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 13ad242f50..948c06a4ff 100644 --- 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(buffer_out_.getData()) + + begin_offset; + raw_end_ = static_cast(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 4cb0774aad..639a4663d7 100644 --- 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 8efada9555..8d4bc9b9c1 100644 --- 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 23e86d00ca..221306e21f 100644 --- 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 AsymPtr; + typedef boost::shared_ptr 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 912ba8af84..09d2d04625 100644 --- 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(dur.total_seconds()); - uint64_t fcvt = dur.fractional_seconds() * 65536U; + ntp_sec_ = static_cast(dur.total_seconds()) + EPOCH_ADJUST; + uint64_t fcvt = static_cast(dur.fractional_seconds()) * 65536U; fcvt /= time_duration::ticks_per_second(); ntp_fraction_ = static_cast(fcvt & 0xffff); } @@ -79,13 +79,19 @@ bool Ntp::from_binary(const std::vector binary) if (binary.size() != 8) { return (false); } - ntp_sec_ = static_cast(binary[0]) << 40; - ntp_sec_ |= static_cast(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 Ntp::to_binary() const double Ntp::secs(time_t base) const { - double ret = static_cast(ntp_sec_ - base - EPOCH_ADJUST); - ret += static_cast(ntp_fraction_ * (1./65536.)); + double ret = static_cast(ntp_sec_); + ret -= base + EPOCH_ADJUST; + ret += static_cast(ntp_fraction_) * (1./65536.); return (ret); }