From f9bf9cdd1c4eb5f65fccd3ba471c063d132f7f1d Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Mon, 13 Nov 2017 11:14:02 +0900 Subject: [PATCH] iscsi: change the return value of ACL func to bool The following functions returns 1 and 0 for succcess and error, respectively: - spdk_iscsi_tgt_node_allow_ipv6() - spdk_iscsi_tgt_node_allow_ipv4() - spdk_iscsi_tgt_node_allow_netmask() - spdk_iscsi_tgt_node_access() Using bool for this purpose will avoid our misunderstanding. Change-Id: I927876e0503c0eee5364e829a4713f9a345996f6 Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/383664 Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris Reviewed-by: Ben Walker Tested-by: SPDK Automated Test System --- lib/iscsi/iscsi.c | 17 ++--- lib/iscsi/tgt_node.c | 66 ++++++++++---------- lib/iscsi/tgt_node.h | 6 +- test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 55 ++++++++++++++-- test/unit/lib/iscsi/param.c/param_ut.c | 4 +- test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c | 53 ++++++++-------- 6 files changed, 119 insertions(+), 82 deletions(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index 8ac54d169e..ff6ccedbf6 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -1391,7 +1391,7 @@ spdk_iscsi_op_login_check_target(struct spdk_iscsi_conn *conn, struct spdk_iscsi_tgt_node **target) { - int rc; + bool result; struct iscsi_bhs_login_rsp *rsph; rsph = (struct iscsi_bhs_login_rsp *)&rsp_pdu->bhs; @@ -1403,17 +1403,10 @@ spdk_iscsi_op_login_check_target(struct spdk_iscsi_conn *conn, rsph->status_detail = ISCSI_LOGIN_TARGET_NOT_FOUND; return SPDK_ISCSI_LOGIN_ERROR_RESPONSE; } - rc = spdk_iscsi_tgt_node_access(conn, *target, - conn->initiator_name, - conn->initiator_addr); - if (rc < 0) { - SPDK_WARNLOG("lu_access() failed\n"); - /* Not found */ - rsph->status_class = ISCSI_CLASS_INITIATOR_ERROR; - rsph->status_detail = ISCSI_LOGIN_TARGET_NOT_FOUND; - return SPDK_ISCSI_LOGIN_ERROR_RESPONSE; - } - if (rc == 0) { + result = spdk_iscsi_tgt_node_access(conn, *target, + conn->initiator_name, + conn->initiator_addr); + if (!result) { SPDK_ERRLOG("access denied\n"); /* Not found */ rsph->status_class = ISCSI_CLASS_INITIATOR_ERROR; diff --git a/lib/iscsi/tgt_node.c b/lib/iscsi/tgt_node.c index 5fd242fcfb..d10552439f 100644 --- a/lib/iscsi/tgt_node.c +++ b/lib/iscsi/tgt_node.c @@ -52,7 +52,7 @@ #define MAX_TMPBUF 1024 #define MAX_MASKBUF 128 -static int +static bool spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr) { struct in6_addr in6_mask; @@ -64,13 +64,13 @@ spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr) int i; if (netmask[0] != '[') - return 0; + return false; p = strchr(netmask, ']'); if (p == NULL) - return 0; + return false; n = p - (netmask + 1); if (n + 1 > sizeof mask) - return 0; + return false; memcpy(mask, netmask + 1, n); mask[n] = '\0'; @@ -79,7 +79,7 @@ spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr) if (p[0] == '/') { bits = (int) strtol(p + 1, NULL, 10); if (bits < 0 || bits > 128) - return 0; + return false; } else { bits = 128; } @@ -92,25 +92,25 @@ spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr) /* presentation to network order binary */ if (inet_pton(AF_INET6, mask, &in6_mask) <= 0 || inet_pton(AF_INET6, addr, &in6_addr) <= 0) { - return 0; + return false; } /* check 128bits */ for (i = 0; i < (bits / 8); i++) { if (in6_mask.s6_addr[i] != in6_addr.s6_addr[i]) - return 0; + return false; } if (bits % 8) { bmask = (0xffU << (8 - (bits % 8))) & 0xffU; if ((in6_mask.s6_addr[i] & bmask) != (in6_addr.s6_addr[i] & bmask)) - return 0; + return false; } /* match */ - return 1; + return true; } -static int +static bool spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr) { struct in_addr in4_mask; @@ -127,7 +127,7 @@ spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr) } n = p - netmask; if (n + 1 > sizeof mask) - return 0; + return false; memcpy(mask, netmask, n); mask[n] = '\0'; @@ -135,7 +135,7 @@ spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr) if (p[0] == '/') { bits = (int) strtol(p + 1, NULL, 10); if (bits < 0 || bits > 32) - return 0; + return false; } else { bits = 32; } @@ -143,48 +143,47 @@ spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr) /* presentation to network order binary */ if (inet_pton(AF_INET, mask, &in4_mask) <= 0 || inet_pton(AF_INET, addr, &in4_addr) <= 0) { - return 0; + return false; } /* check 32bits */ bmask = (0xffffffffULL << (32 - bits)) & 0xffffffffU; if ((ntohl(in4_mask.s_addr) & bmask) != (ntohl(in4_addr.s_addr) & bmask)) - return 0; + return false; /* match */ - return 1; + return true; } -static int +static bool spdk_iscsi_tgt_node_allow_netmask(const char *netmask, const char *addr) { if (netmask == NULL || addr == NULL) - return 0; + return false; if (strcasecmp(netmask, "ALL") == 0) - return 1; + return true; if (netmask[0] == '[') { /* IPv6 */ if (spdk_iscsi_tgt_node_allow_ipv6(netmask, addr)) - return 1; + return true; } else { /* IPv4 */ if (spdk_iscsi_tgt_node_allow_ipv4(netmask, addr)) - return 1; + return true; } - return 0; + return false; } -int +bool spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, struct spdk_iscsi_tgt_node *target, const char *iqn, const char *addr) { struct spdk_iscsi_portal_grp *pg; struct spdk_iscsi_init_grp *igp; - int rc; int i, j, k; if (conn == NULL || target == NULL || iqn == NULL || addr == NULL) - return 0; + return false; pg = conn->portal->group; SPDK_DEBUGLOG(SPDK_TRACE_ISCSI, "pg=%d, iqn=%s, addr=%s\n", @@ -205,7 +204,7 @@ spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, "access denied from %s (%s) to %s (%s:%s,%d)\n", iqn, addr, target->name, conn->portal->host, conn->portal->port, conn->portal->group->tag); - return 0; + return false; } /* allow initiators */ if (strcasecmp(igp->initiators[j], "ALL") == 0 @@ -215,10 +214,9 @@ spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, SPDK_DEBUGLOG(SPDK_TRACE_ISCSI, "netmask=%s, addr=%s\n", igp->netmasks[k], addr); - rc = spdk_iscsi_tgt_node_allow_netmask(igp->netmasks[k], addr); - if (rc > 0) { + if (spdk_iscsi_tgt_node_allow_netmask(igp->netmasks[k], addr)) { /* OK netmask */ - return 1; + return true; } } /* NG netmask in this group */ @@ -230,17 +228,17 @@ spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, SPDK_DEBUGLOG(SPDK_TRACE_ISCSI, "access denied from %s (%s) to %s (%s:%s,%d)\n", iqn, addr, target->name, conn->portal->host, conn->portal->port, conn->portal->group->tag); - return 0; + return false; } -static int +static bool spdk_iscsi_tgt_node_visible(struct spdk_iscsi_tgt_node *target, const char *iqn) { struct spdk_iscsi_init_grp *igp; int i, j; if (target == NULL || iqn == NULL) - return 0; + return false; for (i = 0; i < target->maxmap; i++) { /* iqn is initiator group? */ @@ -250,18 +248,18 @@ spdk_iscsi_tgt_node_visible(struct spdk_iscsi_tgt_node *target, const char *iqn) && (strcasecmp(&igp->initiators[j][1], "ALL") == 0 || strcasecmp(&igp->initiators[j][1], iqn) == 0)) { /* NG */ - return 0; + return false; } if (strcasecmp(igp->initiators[j], "ALL") == 0 || strcasecmp(igp->initiators[j], iqn) == 0) { /* OK iqn, no check addr */ - return 1; + return true; } } } /* NG */ - return 0; + return false; } int diff --git a/lib/iscsi/tgt_node.h b/lib/iscsi/tgt_node.h index ed87f83d22..378f0f37a5 100644 --- a/lib/iscsi/tgt_node.h +++ b/lib/iscsi/tgt_node.h @@ -99,9 +99,9 @@ spdk_iscsi_tgt_node_construct(int target_index, int no_auth_chap, int auth_chap, int auth_chap_mutual, int auth_group, int header_digest, int data_digest); -int spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, - struct spdk_iscsi_tgt_node *target, const char *iqn, - const char *addr); +bool spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, + struct spdk_iscsi_tgt_node *target, const char *iqn, + const char *addr); struct spdk_iscsi_tgt_node *spdk_iscsi_find_tgt_node(const char *target_name); int spdk_iscsi_tgt_node_reset(struct spdk_iscsi_tgt_node *target, uint64_t lun); diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index 21781e2eef..3168d782cf 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -45,18 +45,31 @@ #include "iscsi/portal_grp.h" #include "scsi/scsi_internal.h" +#define UT_TARGET_NAME1 "iqn.2017-11.spdk.io:t0001" +#define UT_TARGET_NAME2 "iqn.2017-11.spdk.io:t0002" +#define UT_INITIATOR_NAME1 "iqn.2017-11.spdk.io:i0001" +#define UT_INITIATOR_NAME2 "iqn.2017-11.spdk.io:i0002" + struct spdk_iscsi_tgt_node * spdk_iscsi_find_tgt_node(const char *target_name) { - return NULL; + if (strcasecmp(target_name, UT_TARGET_NAME1) == 0) { + return (struct spdk_iscsi_tgt_node *)1; + } else { + return NULL; + } } -int +bool spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, struct spdk_iscsi_tgt_node *target, const char *iqn, const char *addr) { - return 0; + if (strcasecmp(conn->initiator_name, UT_INITIATOR_NAME1) == 0) { + return true; + } else { + return false; + } } int @@ -98,6 +111,39 @@ spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id) return dev->lun[lun_id]; } +static void +op_login_check_target_test(void) +{ + struct spdk_iscsi_conn conn; + struct spdk_iscsi_pdu rsp_pdu; + struct spdk_iscsi_tgt_node *target; + int rc; + + /* expect sucess */ + snprintf(conn.initiator_name, sizeof(conn.initiator_name), + "%s", UT_INITIATOR_NAME1); + + rc = spdk_iscsi_op_login_check_target(&conn, &rsp_pdu, + UT_TARGET_NAME1, &target); + CU_ASSERT(rc == 0); + + /* expect failure */ + snprintf(conn.initiator_name, sizeof(conn.initiator_name), + "%s", UT_INITIATOR_NAME1); + + rc = spdk_iscsi_op_login_check_target(&conn, &rsp_pdu, + UT_TARGET_NAME2, &target); + CU_ASSERT(rc != 0); + + /* expect failure */ + snprintf(conn.initiator_name, sizeof(conn.initiator_name), + "%s", UT_INITIATOR_NAME2); + + rc = spdk_iscsi_op_login_check_target(&conn, &rsp_pdu, + UT_TARGET_NAME1, &target); + CU_ASSERT(rc != 0); +} + static void maxburstlength_test(void) { @@ -206,7 +252,8 @@ main(int argc, char **argv) } if ( - CU_add_test(suite, "maxburstlength test", maxburstlength_test) == NULL + CU_add_test(suite, "login check target test", op_login_check_target_test) == NULL + || CU_add_test(suite, "maxburstlength test", maxburstlength_test) == NULL ) { CU_cleanup_registry(); return CU_get_error(); diff --git a/test/unit/lib/iscsi/param.c/param_ut.c b/test/unit/lib/iscsi/param.c/param_ut.c index a0bb5474ec..81c2eadde6 100644 --- a/test/unit/lib/iscsi/param.c/param_ut.c +++ b/test/unit/lib/iscsi/param.c/param_ut.c @@ -48,12 +48,12 @@ spdk_iscsi_find_tgt_node(const char *target_name) return NULL; } -int +bool spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn, struct spdk_iscsi_tgt_node *target, const char *iqn, const char *addr) { - return 0; + return false; } int diff --git a/test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c b/test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c index 674e8140c0..6abed3f257 100644 --- a/test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c +++ b/test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c @@ -111,69 +111,69 @@ config_file_fail_cases(void) static void allow_ipv6_allowed(void) { - int rc; + bool result; char *netmask; char *addr; netmask = "[2001:ad6:1234::]/48"; addr = "2001:ad6:1234:5678:9abc::"; - rc = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr); - CU_ASSERT(rc != 0); + result = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr); + CU_ASSERT(result == true); - rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); - CU_ASSERT(rc != 0); + result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); + CU_ASSERT(result == true); } static void allow_ipv6_denied(void) { - int rc; + bool result; char *netmask; char *addr; netmask = "[2001:ad6:1234::]/56"; addr = "2001:ad6:1234:5678:9abc::"; - rc = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr); - CU_ASSERT(rc == 0); + result = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr); + CU_ASSERT(result == false); - rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); - CU_ASSERT(rc == 0); + result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); + CU_ASSERT(result == false); } static void allow_ipv4_allowed(void) { - int rc; + bool result; char *netmask; char *addr; netmask = "192.168.2.0/24"; addr = "192.168.2.1"; - rc = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr); - CU_ASSERT(rc != 0); + result = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr); + CU_ASSERT(result == true); - rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); - CU_ASSERT(rc != 0); + result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); + CU_ASSERT(result == true); } static void allow_ipv4_denied(void) { - int rc; + bool result; char *netmask; char *addr; netmask = "192.168.2.0"; addr = "192.168.2.1"; - rc = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr); - CU_ASSERT(rc == 0); + result = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr); + CU_ASSERT(result == false); - rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); - CU_ASSERT(rc == 0); + result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr); + CU_ASSERT(result == false); } static void @@ -187,7 +187,7 @@ node_access_allowed(void) char *initiators[] = {"iqn.2017-10.spdk.io:0001"}; char *netmasks[] = {"192.168.2.0/24"}; char *iqn, *addr; - int rc; + bool result; /* portal group initialization */ memset(&pg, 0, sizeof(struct spdk_iscsi_portal_grp)); @@ -223,9 +223,8 @@ node_access_allowed(void) iqn = "iqn.2017-10.spdk.io:0001"; addr = "192.168.2.1"; - rc = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); - CU_ASSERT(rc == 1); - + result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); + CU_ASSERT(result == true); } static void @@ -238,7 +237,7 @@ node_access_denied_by_empty_netmask(void) struct spdk_iscsi_portal portal; char *initiators[] = {"iqn.2017-10.spdk.io:0001"}; char *iqn, *addr; - int rc; + bool result; /* portal group initialization */ memset(&pg, 0, sizeof(struct spdk_iscsi_portal_grp)); @@ -274,8 +273,8 @@ node_access_denied_by_empty_netmask(void) iqn = "iqn.2017-10.spdk.io:0001"; addr = "192.168.3.1"; - rc = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); - CU_ASSERT(rc == 0); + result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr); + CU_ASSERT(result == false); }