diff --git a/files/build_templates/snmp.service.j2 b/files/build_templates/snmp.service.j2 index 5c753dd651..9d3224b805 100644 --- a/files/build_templates/snmp.service.j2 +++ b/files/build_templates/snmp.service.j2 @@ -10,6 +10,7 @@ StartLimitIntervalSec=1200 StartLimitBurst=3 [Service] +ExecStartPre=/bin/bash -c 'end=$((SECONDS+20));while [ $SECONDS -lt $end ];do if /usr/bin/pgrep intfmgrd >/dev/null;then break;else sleep 1;fi;done' ExecStartPre=/usr/local/bin/{{docker_container_name}}.sh start ExecStart=/usr/local/bin/{{docker_container_name}}.sh wait ExecStop=/usr/local/bin/{{docker_container_name}}.sh stop diff --git a/src/sonic-platform-daemons b/src/sonic-platform-daemons index 198f3002ec..6064369143 160000 --- a/src/sonic-platform-daemons +++ b/src/sonic-platform-daemons @@ -1 +1 @@ -Subproject commit 198f3002ec7375dca56e2c0f95704ad2c6993204 +Subproject commit 6064369143c0a039a20fcb82c0d6cf32a9728f59 diff --git a/src/sonic-sairedis b/src/sonic-sairedis index 70242e7bdc..439b92618d 160000 --- a/src/sonic-sairedis +++ b/src/sonic-sairedis @@ -1 +1 @@ -Subproject commit 70242e7bdc1b49090e60040dbe2715d14b4165bd +Subproject commit 439b92618d007b6b0ac1e14f02ffdcd9ee9b0692 diff --git a/src/sonic-snmpagent b/src/sonic-snmpagent index 1477c36836..17a8bb205d 160000 --- a/src/sonic-snmpagent +++ b/src/sonic-snmpagent @@ -1 +1 @@ -Subproject commit 1477c368369db44f9a2f9deac0356bbd5fad6364 +Subproject commit 17a8bb205d2fddce8b645a990115ef2cc81c9f05 diff --git a/src/sonic-telemetry b/src/sonic-telemetry index 0b8843c320..a399febb24 160000 --- a/src/sonic-telemetry +++ b/src/sonic-telemetry @@ -1 +1 @@ -Subproject commit 0b8843c3200179d3ff6215df7050f3dbe28949f4 +Subproject commit a399febb249f3710b43798457f4804238bff3b31 diff --git a/src/tacacs/bash_tacplus/bash_tacplus.c b/src/tacacs/bash_tacplus/bash_tacplus.c index b184b8f14b..6e72e8f0a0 100644 --- a/src/tacacs/bash_tacplus/bash_tacplus.c +++ b/src/tacacs/bash_tacplus/bash_tacplus.c @@ -14,8 +14,8 @@ /* Remote user gecos prefix, which been assigned by nss_tacplus */ #define REMOTE_USER_GECOS_PREFIX "remote_user" -/* Default value for _SC_GETPW_R_SIZE_MAX */ -#define DEFAULT_SC_GETPW_R_SIZE_MAX 1024 +/* Default value for getpwent */ +#define DEFAULT_GETPWENT_SIZE_MAX 4096 /* Return value for is_local_user method */ #define IS_LOCAL_USER 0 @@ -31,6 +31,7 @@ /* Output syslog to mock method when build with UT */ #if defined (BASH_PLUGIN_UT) #define syslog mock_syslog +#define getpwent_r mock_getpwent_r #endif /* Tacacs+ log format */ @@ -42,7 +43,7 @@ /* Tacacs+ config file timestamp string length */ #define CONFIG_FILE_TIME_STAMP_LEN 100 -/* +/* Convert log to a string because va args resoursive issue: http://www.c-faq.com/varargs/handoff.html */ @@ -199,7 +200,7 @@ int tacacs_authorization( continue; } - // increase connected servers + // increase connected servers connected_servers++; result = send_authorization_message(server_fd, user, tty, host, task_id, cmd, args, argc); close(server_fd); @@ -279,7 +280,7 @@ void load_tacacs_config() } output_debug("TACACS+ control flag: 0x%x\n", tacacs_ctrl); - + if (tacacs_ctrl & AUTHORIZATION_FLAG_TACACS) { output_debug("TACACS+ per-command authorization enabled.\n"); } @@ -287,7 +288,7 @@ void load_tacacs_config() if (tacacs_ctrl & AUTHORIZATION_FLAG_LOCAL) { output_debug("Local per-command authorization enabled.\n"); } - + if (tacacs_ctrl & PAM_TAC_DEBUG) { output_debug("TACACS+ debug enabled.\n"); } @@ -350,40 +351,39 @@ int is_local_user(char *user) } struct passwd pwd; - struct passwd *pwdresult; - char *buf; - size_t bufsize = sysconf(_SC_GETPW_R_SIZE_MAX); - if (bufsize == -1) { - bufsize = DEFAULT_SC_GETPW_R_SIZE_MAX; - } + struct passwd *ppwd; + char buf[DEFAULT_GETPWENT_SIZE_MAX]; + int pwdresult; + int result = ERROR_CHECK_LOCAL_USER; + setpwent(); + while (1) { + pwdresult = getpwent_r(&pwd, buf, sizeof(buf), &ppwd); + if (pwdresult) { + // no more pw entry + break; + } - buf = malloc(bufsize); - if (buf == NULL) { - output_error("failed to allocate getpwnam_r buffer.\n"); - return ERROR_CHECK_LOCAL_USER; - } + if (strcmp(ppwd->pw_name, user) != 0) { + continue; + } - int s = getpwnam_r(user, &pwd, buf, bufsize, &pwdresult); - int result = IS_LOCAL_USER; - if (pwdresult == NULL) { - if (s == 0) - output_error("get user information user failed, user: %s not found\n", user); + // compare passwd entry, for remote user pw_gecos will start as 'remote_user' + if (strncmp(ppwd->pw_gecos, REMOTE_USER_GECOS_PREFIX, strlen(REMOTE_USER_GECOS_PREFIX)) == 0) { + output_debug("user: %s, UID: %d, GECOS: %s is remote user.\n", user, ppwd->pw_uid, ppwd->pw_gecos); + result = IS_REMOTE_USER; + } else { - output_error("get user information failed, user: %s, errorno: %d\n", user, s); + output_debug("user: %s, UID: %d, GECOS: %s is local user.\n", user, ppwd->pw_uid, ppwd->pw_gecos); + result = IS_LOCAL_USER; } - - result = ERROR_CHECK_LOCAL_USER; - } - else if (strncmp(pwd.pw_gecos, REMOTE_USER_GECOS_PREFIX, strlen(REMOTE_USER_GECOS_PREFIX)) == 0) { - output_debug("user: %s, UID: %d, GECOS: %s is remote user.\n", user, pwd.pw_uid, pwd.pw_gecos); - result = IS_REMOTE_USER; + break; } - else { - output_debug("user: %s, UID: %d, GECOS: %s is local user.\n", user, pwd.pw_uid, pwd.pw_gecos); - result = IS_LOCAL_USER; + endpwent(); + + if (result == ERROR_CHECK_LOCAL_USER) { + output_error("get user information user failed, user: %s not found\n", user); } - free(buf); return result; } @@ -482,7 +482,7 @@ int on_shell_execve (char *user, int shell_level, char *cmd, char **argv) } } - // return 0, so bash will continue run user command and will check user permission with linux permission check. + // return 0, so bash will continue run user command and will check user permission with linux permission check. output_debug("start local authorization for command %s with given arguments\n", cmd); return 0; -} \ No newline at end of file +} diff --git a/src/tacacs/bash_tacplus/unittest/mock_helper.c b/src/tacacs/bash_tacplus/unittest/mock_helper.c index 6edbdbe1ac..d2fc9424fc 100644 --- a/src/tacacs/bash_tacplus/unittest/mock_helper.c +++ b/src/tacacs/bash_tacplus/unittest/mock_helper.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -65,13 +66,13 @@ void initialize_tacacs_servers() getaddrinfo(buffer, "49", &hints, &servers); tac_srv[idx].addr = &(tac_srv_addr[idx]); memcpy(tac_srv[idx].addr, servers, sizeof(struct addrinfo)); - + tac_srv[idx].addr->ai_addr = &(tac_sock_addr[idx]); memcpy(tac_srv[idx].addr->ai_addr, servers->ai_addr, sizeof(struct sockaddr)); - + snprintf(tac_srv[idx].key, sizeof(tac_srv[idx].key), "key%d", idx); freeaddrinfo(servers); - + debug_printf("MOCK: initialize_tacacs_servers with index: %d, address: %p\n", idx, tac_srv[idx].addr); } } @@ -119,7 +120,7 @@ void tac_free_attrib(struct tac_attrib **attr) { memory_allocate_count--; debug_printf("MOCK: tac_free_attrib memory count: %d\n", memory_allocate_count); - + // the mock code here only free first allocated memory, because the mock tac_add_attrib implementation not allocate new memory. free(*attr); } @@ -133,7 +134,7 @@ int tac_author_send(int tac_fd, const char *user, char *tty, char *host,struct t // send auth message failed return -1; } - + return 0; } @@ -146,7 +147,7 @@ int tac_author_read(int tac_fd, struct areply *reply) { return -1; } - + if (TEST_SCEANRIO_CONNECTION_SEND_DENINED_RESULT == test_scenario) { reply->status = AUTHOR_STATUS_FAIL; @@ -155,7 +156,7 @@ int tac_author_read(int tac_fd, struct areply *reply) { reply->status = AUTHOR_STATUS_PASS_REPL; } - + return 0; } @@ -163,7 +164,7 @@ int tac_author_read(int tac_fd, struct areply *reply) int tac_connect_single(const struct addrinfo *address, const char *key, struct addrinfo *source_address, int timeout, char *vrfname) { debug_printf("MOCK: tac_connect_single with address: %p\n", address); - + switch (test_scenario) { case TEST_SCEANRIO_CONNECTION_ALL_FAILED: @@ -183,7 +184,7 @@ char *tac_ntop(const struct sockaddr *address) return tac_natop_result_buffer; } } - + return "UnknownTestAddress"; } @@ -198,12 +199,41 @@ void mock_syslog(int priority, const char *format, ...) { // set mock message data to buffer for UT. memset(mock_syslog_message_buffer, 0, sizeof(mock_syslog_message_buffer)); - + va_list args; va_start (args, format); // save message to buffer to UT check later vsnprintf(mock_syslog_message_buffer, sizeof(mock_syslog_message_buffer), format, args); va_end (args); - + debug_printf("MOCK: syslog: %s\n", mock_syslog_message_buffer); +} + +int mock_getpwent_r(struct passwd *restrict pwbuf, + char *buf, size_t buflen, + struct passwd **restrict pwbufp) +{ + static char* test_user = "test_user"; + static char* root_user = "root"; + static char* empty_gecos = ""; + static char* remote_gecos = "remote_user"; + *pwbufp = pwbuf; + switch (test_scenario) + { + case TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_RESULT: + case TEST_SCEANRIO_CONNECTION_SEND_DENINED_RESULT: + case TEST_SCEANRIO_IS_LOCAL_USER_REMOTE: + pwbuf->pw_name = test_user; + pwbuf->pw_gecos = remote_gecos; + pwbuf->pw_uid = 1000; + return 0; + case TEST_SCEANRIO_IS_LOCAL_USER_ROOT: + pwbuf->pw_name = root_user; + pwbuf->pw_gecos = empty_gecos; + pwbuf->pw_uid = 0; + return 0; + case TEST_SCEANRIO_IS_LOCAL_USER_NOT_FOUND: + return 1; + } + return 1; } \ No newline at end of file diff --git a/src/tacacs/bash_tacplus/unittest/mock_helper.h b/src/tacacs/bash_tacplus/unittest/mock_helper.h index 348b7810fc..7cc6779c70 100644 --- a/src/tacacs/bash_tacplus/unittest/mock_helper.h +++ b/src/tacacs/bash_tacplus/unittest/mock_helper.h @@ -24,11 +24,16 @@ /* Mock syslog buffer */ extern char mock_syslog_message_buffer[1024]; -#define TEST_SCEANRIO_CONNECTION_ALL_FAILED 1 -#define TEST_SCEANRIO_CONNECTION_SEND_FAILED_RESULT 2 -#define TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_READ_FAILED 3 -#define TEST_SCEANRIO_CONNECTION_SEND_DENINED_RESULT 4 -#define TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_RESULT 5 +#define TEST_SCEANRIO_CONNECTION_ALL_FAILED 1 +#define TEST_SCEANRIO_CONNECTION_SEND_FAILED_RESULT 2 +#define TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_READ_FAILED 3 +#define TEST_SCEANRIO_CONNECTION_SEND_DENINED_RESULT 4 +#define TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_RESULT 5 +#define TEST_SCEANRIO_LOAD_CHANGED_TACACS_CONFIG 6 +#define TEST_SCEANRIO_IS_LOCAL_USER_UNKNOWN 7 +#define TEST_SCEANRIO_IS_LOCAL_USER_NOT_FOUND 8 +#define TEST_SCEANRIO_IS_LOCAL_USER_ROOT 9 +#define TEST_SCEANRIO_IS_LOCAL_USER_REMOTE 10 /* Set test scenario for test*/ void set_test_scenario(int scenario); diff --git a/src/tacacs/bash_tacplus/unittest/plugin_test.c b/src/tacacs/bash_tacplus/unittest/plugin_test.c index 2617fd8249..ab12829cfb 100644 --- a/src/tacacs/bash_tacplus/unittest/plugin_test.c +++ b/src/tacacs/bash_tacplus/unittest/plugin_test.c @@ -5,6 +5,10 @@ #include "mock_helper.h" #include +#define IS_LOCAL_USER 0 +#define IS_REMOTE_USER 1 +#define ERROR_CHECK_LOCAL_USER 2 + /* tacacs debug flag */ extern int tacacs_ctrl; @@ -23,14 +27,14 @@ void testcase_tacacs_authorization_all_failed() { char *testargv[2]; testargv[0] = "arg1"; testargv[1] = "arg2"; - - + + // test connection failed case set_test_scenario(TEST_SCEANRIO_CONNECTION_ALL_FAILED); int result = tacacs_authorization("test_user","tty0","test_host","test_command",testargv,2); CU_ASSERT_STRING_EQUAL(mock_syslog_message_buffer, "Failed to connect to TACACS server(s)\n"); - + // check return value, -2 for all server not reachable CU_ASSERT_EQUAL(result, -2); } @@ -40,7 +44,7 @@ void testcase_tacacs_authorization_faled() { char *testargv[2]; testargv[0] = "arg1"; testargv[1] = "arg2"; - + // test connection failed case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_FAILED_RESULT); int result = tacacs_authorization("test_user","tty0","test_host","test_command",testargv,2); @@ -54,7 +58,7 @@ void testcase_tacacs_authorization_read_failed() { char *testargv[2]; testargv[0] = "arg1"; testargv[1] = "arg2"; - + // test connection failed case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_READ_FAILED); int result = tacacs_authorization("test_user","tty0","test_host","test_command",testargv,2); @@ -70,7 +74,7 @@ void testcase_tacacs_authorization_denined() { char *testargv[2]; testargv[0] = "arg1"; testargv[1] = "arg2"; - + // test connection denined case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_DENINED_RESULT); int result = tacacs_authorization("test_user","tty0","test_host","test_command",testargv,2); @@ -86,7 +90,7 @@ void testcase_tacacs_authorization_success() { char *testargv[2]; testargv[0] = "arg1"; testargv[1] = "arg2"; - + // test connection success case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_RESULT); int result = tacacs_authorization("test_user","tty0","test_host","test_command",testargv,2); @@ -100,7 +104,7 @@ void testcase_authorization_with_host_and_tty_success() { char *testargv[2]; testargv[0] = "arg1"; testargv[1] = "arg2"; - + // test connection success case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_RESULT); int result = authorization_with_host_and_tty("test_user","test_command",testargv,2); @@ -111,13 +115,15 @@ void testcase_authorization_with_host_and_tty_success() { /* Test check_and_load_changed_tacacs_config */ void testcase_check_and_load_changed_tacacs_config() { - + + set_test_scenario(TEST_SCEANRIO_LOAD_CHANGED_TACACS_CONFIG); + // test connection failed case check_and_load_changed_tacacs_config(); // check server config updated. CU_ASSERT_STRING_EQUAL(mock_syslog_message_buffer, "Server 2, address:TestAddress2, key:key2\n"); - + // check and load file again. check_and_load_changed_tacacs_config(); @@ -132,7 +138,7 @@ void testcase_on_shell_execve_success() { testargv[0] = "arg1"; testargv[1] = "arg2"; testargv[2] = 0; - + // test connection failed case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_SUCCESS_RESULT); on_shell_execve("test_user", 1, "test_command", testargv); @@ -147,7 +153,7 @@ void testcase_on_shell_execve_denined() { testargv[0] = "arg1"; testargv[1] = "arg2"; testargv[2] = 0; - + // test connection failed case set_test_scenario(TEST_SCEANRIO_CONNECTION_SEND_DENINED_RESULT); on_shell_execve("test_user", 1, "test_command", testargv); @@ -162,7 +168,7 @@ void testcase_on_shell_execve_failed() { testargv[0] = "arg1"; testargv[1] = "arg2"; testargv[2] = 0; - + // test connection failed case set_test_scenario(TEST_SCEANRIO_CONNECTION_ALL_FAILED); on_shell_execve("test_user", 1, "test_command", testargv); @@ -171,6 +177,43 @@ void testcase_on_shell_execve_failed() { CU_ASSERT_STRING_EQUAL(mock_syslog_message_buffer, "test_command not authorized by TACACS+ with given arguments, not executing\n"); } +/* Test is_local_user unknown user */ +void testcase_is_local_user_unknown() { + set_test_scenario(TEST_SCEANRIO_IS_LOCAL_USER_UNKNOWN); + int result = is_local_user("UNKNOWN"); + + // check unknown user is remote. + CU_ASSERT_EQUAL(result, IS_REMOTE_USER); +} + +/* Test is_local_user not found user */ +void testcase_is_local_user_not_found() { + set_test_scenario(TEST_SCEANRIO_IS_LOCAL_USER_NOT_FOUND); + int result = is_local_user("notexist"); + + // check unknown user is remote. + CU_ASSERT_EQUAL(result, ERROR_CHECK_LOCAL_USER); + CU_ASSERT_STRING_EQUAL(mock_syslog_message_buffer, "get user information user failed, user: notexist not found\n"); +} + +/* Test is_local_user root user */ +void testcase_is_local_user_root() { + set_test_scenario(TEST_SCEANRIO_IS_LOCAL_USER_ROOT); + int result = is_local_user("root"); + + // check unknown user is remote. + CU_ASSERT_EQUAL(result, IS_LOCAL_USER); +} + +/* Test is_local_user remote user */ +void testcase_is_local_user_remote() { + set_test_scenario(TEST_SCEANRIO_IS_LOCAL_USER_REMOTE); + int result = is_local_user("test_user"); + + // check unknown user is remote. + CU_ASSERT_EQUAL(result, IS_REMOTE_USER); +} + int main(void) { if (CUE_SUCCESS != CU_initialize_registry()) { return CU_get_error(); @@ -196,7 +239,11 @@ int main(void) { || !CU_add_test(ste, "Test testcase_check_and_load_changed_tacacs_config()...\n", testcase_check_and_load_changed_tacacs_config) || !CU_add_test(ste, "Test testcase_on_shell_execve_success()...\n", testcase_on_shell_execve_success) || !CU_add_test(ste, "Test testcase_on_shell_execve_denined()...\n", testcase_on_shell_execve_denined) - || !CU_add_test(ste, "Test testcase_on_shell_execve_failed()...\n", testcase_on_shell_execve_failed)) { + || !CU_add_test(ste, "Test testcase_on_shell_execve_failed()...\n", testcase_on_shell_execve_failed) + || !CU_add_test(ste, "Test testcase_is_local_user_unknown()...\n", testcase_is_local_user_unknown) + || !CU_add_test(ste, "Test testcase_is_local_user_not_found()...\n", testcase_is_local_user_not_found) + || !CU_add_test(ste, "Test testcase_is_local_user_root()...\n", testcase_is_local_user_root) + || !CU_add_test(ste, "Test testcase_is_local_user_remote()...\n", testcase_is_local_user_remote)) { CU_cleanup_registry(); return CU_get_error(); }