Merge branch 'master' into zmq_warmboot_upstream_master

sonic-net · Sep 18, 2023 · 436f7c9 · 436f7c9
2 parents b120b29 + 13ef25b
commit 436f7c9
Show file tree

Hide file tree

Showing 5 changed files with 210 additions and 6 deletions.
diff --git a/.gitignore b/.gitignore
@@ -77,6 +77,7 @@ tests/tests
 tests/mock_tests/tests_response_publisher
 tests/mock_tests/tests_fpmsyncd
 tests/mock_tests/tests_intfmgrd
+tests/mock_tests/tests_teammgrd
 tests/mock_tests/tests_portsyncd
 
 

diff --git a/cfgmgr/macsecmgr.cpp b/cfgmgr/macsecmgr.cpp
@@ -34,6 +34,20 @@ constexpr std::uint64_t RETRY_TIME = 30;
 /* retry interval, in millisecond */
 constexpr std::uint64_t RETRY_INTERVAL = 100;
 
+/*
+ * The input cipher_str is the encoded string which can be either of length 66 bytes or 130 bytes.
+ *
+ * 66 bytes of length, for 128-byte cipher suite
+ *   - first 2 bytes of the string will be the index from the magic salt string.
+ *   - remaining 64 bytes will be encoded string from the 32-byte plain text CAK input string.
+ *
+ * 130 bytes of length, for 256-byte cipher suite
+ *   - first 2 bytes of the string will be the index from the magic salt string.
+ *   - remaining 128 bytes will be encoded string from the 32 byte plain text CAK input string.
+*/
+constexpr std::size_t AES_LEN_128_BYTE = 66;
+constexpr std::size_t AES_LEN_256_BYTE = 130;
+
 static void lexical_convert(const std::string &policy_str, MACsecMgr::MACsecProfile::Policy & policy)
 {
     SWSS_LOG_ENTER();
@@ -78,6 +92,60 @@ static void lexical_convert(const std::string &cipher_str, MACsecMgr::MACsecProf
     }
 }
 
+
+
+/* Decodes a Type 7 encoded input.
+ *
+ * The Type 7 encoding consists of two decimal digits(encoding the salt), followed a series of hexadecimal characters,
+ * two for every byte in the encoded password. An example encoding(of "password") is 044B0A151C36435C0D.
+ * This has a salt/offset of 4 (04 in the example), and encodes password via 4B0A151C36435C0D.
+ *
+ * The algorithm is a straightforward XOR Cipher that relies on the following ascii-encoded 53-byte constant:
+ *    "dsfd;kfoA,.iyewrkldJKDHSUBsgvca69834ncxv9873254k;fg87"
+ *
+ * Decode()
+ *    Get the salt index from the first 2 chars
+ *    For each byte in the provided text after the encoded salt:
+ *        j = (salt index + 1) % 53
+ *        XOR the i'th byte of the password with the j'th byte of the magic constant.
+ *        append to the decoded string.
+ */
+static std::string decodeKey(const std::string &cipher_str, const MACsecMgr::MACsecProfile::CipherSuite & cipher_suite)
+{
+    int salts[] = { 0x64, 0x73, 0x66, 0x64, 0x3B, 0x6B, 0x66, 0x6F, 0x41, 0x2C, 0x2E, 0x69, 0x79, 0x65, 0x77, 0x72, 0x6B, 0x6C, 0x64, 0x4A, 0x4B, 0x44, 0x48, 0x53, 0x55, 0x42, 0x73, 0x67, 0x76, 0x63, 0x61, 0x36, 0x39, 0x38, 0x33, 0x34, 0x6E, 0x63, 0x78, 0x76, 0x39, 0x38, 0x37, 0x33, 0x32, 0x35, 0x34, 0x6B, 0x3B, 0x66, 0x67, 0x38, 0x37 };
+
+    std::string decodedPassword = std::string("");
+    std::string cipher_hex_str = std::string("");
+    unsigned int hex_int, saltIdx;
+
+    if ((cipher_suite == MACsecMgr::MACsecProfile::CipherSuite::GCM_AES_128) ||
+        (cipher_suite == MACsecMgr::MACsecProfile::CipherSuite::GCM_AES_XPN_128))
+    {
+        if (cipher_str.length() != AES_LEN_128_BYTE)
+            throw std::invalid_argument("Invalid length for cipher_string : " + cipher_str);
+    }
+    else if ((cipher_suite == MACsecMgr::MACsecProfile::CipherSuite::GCM_AES_256) ||
+             (cipher_suite == MACsecMgr::MACsecProfile::CipherSuite::GCM_AES_XPN_256))
+    {
+        if (cipher_str.length() != AES_LEN_256_BYTE)
+            throw std::invalid_argument("Invalid length for cipher_string : " + cipher_str);
+    }
+
+    // Get the salt index from the cipher_str
+    saltIdx = (unsigned int) stoi(cipher_str.substr(0,2));
+
+    // Convert the hex string (eg: "aabbcc") to hex integers (eg: 0xaa, 0xbb, 0xcc) taking a substring of 2 chars at a time
+    // and do xor with the magic salt string
+    for (size_t i = 2; i < cipher_str.length(); i += 2) {
+        std::stringstream ss;
+        ss << std::hex << cipher_str.substr(i,2);
+        ss >> hex_int;
+        decodedPassword += (char)(hex_int ^ salts[saltIdx++ % (sizeof(salts)/sizeof(salts[0]))]);
+    }
+
+    return decodedPassword;
+}
+
 template<class T>
 static bool get_value(
     const MACsecMgr::TaskArgs & ta,
@@ -699,7 +767,7 @@ bool MACsecMgr::configureMACsec(
             port_name,
             network_id,
             "mka_cak",
-            profile.primary_cak);
+            decodeKey(profile.primary_cak, profile.cipher_suite));
 
         wpa_cli_exec_and_check(
             session.sock,

diff --git a/cfgmgr/teammgr.cpp b/cfgmgr/teammgr.cpp
@@ -307,6 +307,8 @@ void TeamMgr::doLagTask(Consumer &consumer)
             {
                 if (addLag(alias, min_links, fallback, fast_rate) == task_need_retry)
                 {
+                    // If LAG creation fails, we need to clean up any potentially orphaned teamd processes
+                    removeLag(alias);
                     it++;
                     continue;
                 }
@@ -627,7 +629,7 @@ task_process_status TeamMgr::addLag(const string &alias, int min_links, bool fal
     SWSS_LOG_INFO("Port channel %s teamd configuration: %s",
             alias.c_str(), conf.str().c_str());
 
-    string warmstart_flag = WarmStart::isWarmStart() ? " -w -o " : " -r ";
+    string warmstart_flag = WarmStart::isWarmStart() ? " -w -o" : " -r";
 
     cmd << TEAMD_CMD
         << warmstart_flag
@@ -654,9 +656,42 @@ bool TeamMgr::removeLag(const string &alias)
 
     stringstream cmd;
     string res;
+    pid_t pid;
 
-    cmd << TEAMD_CMD << " -k -t " << shellquote(alias);
-    EXEC_WITH_ERROR_THROW(cmd.str(), res);
+    try
+    {
+        std::stringstream cmd;
+        cmd << "cat " << shellquote("/var/run/teamd/" + alias + ".pid");
+        EXEC_WITH_ERROR_THROW(cmd.str(), res);
+    }
+    catch (const std::exception &e)
+    {
+        SWSS_LOG_NOTICE("Failed to remove non-existent port channel %s pid...", alias.c_str());
+        return false;
+    }
+
+    try
+    {
+        pid = static_cast<pid_t>(std::stoul(res, nullptr, 10));
+        SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
+    }
+    catch (const std::exception &e)
+    {
+        SWSS_LOG_ERROR("Failed to read port channel %s pid: %s", alias.c_str(), e.what());
+        return false;
+    }
+
+    try
+    {
+        std::stringstream cmd;
+        cmd << "kill -TERM " << pid;
+        EXEC_WITH_ERROR_THROW(cmd.str(), res);
+    }
+    catch (const std::exception &e)
+    {
+        SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, e.what());
+        return false;
+    }
 
     SWSS_LOG_NOTICE("Stop port channel %s", alias.c_str());
 

diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am
@@ -5,9 +5,9 @@ DASH_PROTO_DIR = $(top_srcdir)/orchagent/dash/proto
 
 CFLAGS_SAI = -I /usr/include/sai
 
-TESTS = tests tests_intfmgrd tests_portsyncd tests_fpmsyncd tests_response_publisher
+TESTS = tests tests_intfmgrd tests_teammgrd tests_portsyncd tests_fpmsyncd tests_response_publisher
 
-noinst_PROGRAMS = tests tests_intfmgrd tests_portsyncd tests_fpmsyncd tests_response_publisher
+noinst_PROGRAMS = tests tests_intfmgrd tests_teammgrd tests_portsyncd tests_fpmsyncd tests_response_publisher
 
 LDADD_SAI = -lsaimeta -lsaimetadata -lsaivs -lsairedis
 
@@ -190,6 +190,28 @@ tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTE
 tests_intfmgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
         -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main
 
+## teammgrd unit tests
+
+tests_teammgrd_SOURCES = teammgrd/teammgr_ut.cpp \
+                         $(top_srcdir)/cfgmgr/teammgr.cpp \
+                         $(top_srcdir)/lib/subintf.cpp \
+                         $(top_srcdir)/lib/recorder.cpp \
+                         $(top_srcdir)/orchagent/orch.cpp \
+                         $(top_srcdir)/orchagent/request_parser.cpp \
+                         mock_orchagent_main.cpp \
+                         mock_dbconnector.cpp \
+                         mock_table.cpp \
+                         mock_hiredis.cpp \
+                         fake_response_publisher.cpp \
+                         mock_redisreply.cpp \
+                         common/mock_shell_command.cpp
+
+tests_teammgrd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/lib
+tests_teammgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
+tests_teammgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_teammgrd_INCLUDES)
+tests_teammgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
+        -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main
+
 ## fpmsyncd unit tests
 
 tests_fpmsyncd_SOURCES = fpmsyncd/test_fpmlink.cpp \

diff --git a/tests/mock_tests/teammgrd/teammgr_ut.cpp b/tests/mock_tests/teammgrd/teammgr_ut.cpp
@@ -0,0 +1,78 @@
+#include "gtest/gtest.h"
+#include "../mock_table.h"
+#include "teammgr.h"
+
+extern int (*callback)(const std::string &cmd, std::string &stdout);
+extern std::vector<std::string> mockCallArgs;
+
+int cb(const std::string &cmd, std::string &stdout)
+{
+    mockCallArgs.push_back(cmd);
+    if (cmd.find("/usr/bin/teamd -r -t PortChannel1") != std::string::npos)
+    {
+        return 1;
+    }
+    else if (cmd.find("cat \"/var/run/teamd/PortChannel1.pid\"") != std::string::npos)
+    {
+        stdout = "1234";
+        return 0;
+    }
+    return 0;
+}
+
+namespace teammgr_ut
+{
+    struct TeamMgrTest : public ::testing::Test
+    {
+        std::shared_ptr<swss::DBConnector> m_config_db;
+        std::shared_ptr<swss::DBConnector> m_app_db;
+        std::shared_ptr<swss::DBConnector> m_state_db;
+        std::vector<TableConnector> cfg_lag_tables;
+
+        virtual void SetUp() override
+        {
+            testing_db::reset();
+            m_config_db = std::make_shared<swss::DBConnector>("CONFIG_DB", 0);
+            m_app_db = std::make_shared<swss::DBConnector>("APPL_DB", 0);
+            m_state_db = std::make_shared<swss::DBConnector>("STATE_DB", 0);
+
+            swss::Table metadata_table = swss::Table(m_config_db.get(), CFG_DEVICE_METADATA_TABLE_NAME);
+            std::vector<swss::FieldValueTuple> vec;
+            vec.emplace_back("mac", "01:23:45:67:89:ab");
+            metadata_table.set("localhost", vec);
+
+            TableConnector conf_lag_table(m_config_db.get(), CFG_LAG_TABLE_NAME);
+            TableConnector conf_lag_member_table(m_config_db.get(), CFG_LAG_MEMBER_TABLE_NAME);
+            TableConnector state_port_table(m_state_db.get(), STATE_PORT_TABLE_NAME);
+
+            std::vector<TableConnector> tables = {
+                conf_lag_table,
+                conf_lag_member_table,
+                state_port_table
+            };
+
+            cfg_lag_tables = tables;
+            mockCallArgs.clear();
+            callback = cb;
+        }
+    };
+
+    TEST_F(TeamMgrTest, testProcessKilledAfterAddLagFailure)
+    {
+        swss::TeamMgr teammgr(m_config_db.get(), m_app_db.get(), m_state_db.get(), cfg_lag_tables);
+        swss::Table cfg_lag_table = swss::Table(m_config_db.get(), CFG_LAG_TABLE_NAME);
+        cfg_lag_table.set("PortChannel1", { { "admin_status", "up" },
+                                            { "mtu", "9100" },
+                                            { "lacp_key", "auto" },
+                                            { "min_links", "2" } });
+        teammgr.addExistingData(&cfg_lag_table);
+        teammgr.doTask();
+        int kill_cmd_called = 0;
+        for (auto cmd : mockCallArgs){
+            if (cmd.find("kill -TERM 1234") != std::string::npos){
+                kill_cmd_called++;
+            }
+        }
+        ASSERT_EQ(kill_cmd_called, 1);
+    }
+}