| Message ID | 20230113175308.50900-1-mic@digikod.net (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | selftests/landlock: Improve ptrace_test with Yama | expand |
On 13/01/2023 18:53, Mickaël Salaün wrote: > I found some issues running this new ptrace_test. Indeed, Yama doesn't > restrict PTRACE_MODE_READ whereas Landlock does. I also changed some > comments and socpe conditions. In fact, the final SKIP() wasn't a good > idea because it masks such potential errors; let's only use a log > message instead. I also removed the first SKIP() because we want > everything to be tested, and the MODE_READ does still matter anyway. > > Please review this patch and squash it in yours. > > Signed-off-by: Mickaël Salaün <mic@digikod.net> > Link: https://lore.kernel.org/r/20230113175308.50900-1-mic@digikod.net > --- > .../testing/selftests/landlock/ptrace_test.c | 106 ++++++++++-------- > 1 file changed, 57 insertions(+), 49 deletions(-) > > diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c > index c9d9f3001d0f..3c3ba37bf8ec 100644 > --- a/tools/testing/selftests/landlock/ptrace_test.c > +++ b/tools/testing/selftests/landlock/ptrace_test.c > @@ -19,7 +19,7 @@ > > #include "common.h" > > -/* copy from yama_lsm.c */ > +/* Copied from security/yama/yama_lsm.c */ > #define YAMA_SCOPE_DISABLED 0 > #define YAMA_SCOPE_RELATIONAL 1 > #define YAMA_SCOPE_CAPABILITY 2 > @@ -70,7 +70,7 @@ static int get_yama_ptrace_scope(void) > { > int ret; > char buf[2] = {}; > - int fd = open("/proc/sys/kernel/yama/ptrace_scope", O_RDONLY); > + const int fd = open("/proc/sys/kernel/yama/ptrace_scope", O_RDONLY); > > if (fd < 0) > return 0; > @@ -260,46 +260,48 @@ TEST_F(hierarchy, trace) > int yama_ptrace_scope; > char buf_parent; > long ret; > - bool can_trace_child, can_trace_parent; > + bool can_read_child, can_trace_child, can_read_parent, can_trace_parent; > > yama_ptrace_scope = get_yama_ptrace_scope(); > ASSERT_LE(0, yama_ptrace_scope); > > - if (yama_ptrace_scope >= YAMA_SCOPE_NO_ATTACH) > - SKIP(return, "Yama forbids any ptrace use (scope %d)", > - yama_ptrace_scope); > + if (yama_ptrace_scope > YAMA_SCOPE_DISABLED) > + TH_LOG("Incomplete tests due to Yama restrictions (scope %d)", > + yama_ptrace_scope); > > /* > - * can_trace_child: if a parent process can trace its child process. > - * > - * There are two conditions concerning landlock: > - * 1> the parent and child processes are in the same landlock domain or > - * one beneath it (case: domain_both = true). > - * 2> yama allows tracing children (up to YAMA_SCOPE_RELATIONAL). > - * Both 1 and 2 need to be met for can_trace_child to be true. > - * > - * If a parent process has its own domain not shared with the child > - * process (case:domain_parent = true), then the parent can't trace the > - * child. > + * can_read_child is true if a parent process can read its child > + * process, which is only the case when the parent process is not > + * isolated from the child with a dedicated Landlock domain. > */ > - can_trace_child = !variant->domain_parent && > - yama_ptrace_scope < YAMA_SCOPE_CAPABILITY; > + can_read_child = !variant->domain_parent; > > /* > - * can_trace_parent: if a child process can trace its parent process. > - * > - * There are two conditions concerning landlock: > - * 1> the parent and child process are in the same landlock domain or > - * one beneath it.(case: domain_both = true). > - * 2> yama is disabled (YAMA_SCOPE_DISABLED). > - * Both 1 and 2 need to be met for can_trace_parent to be true. > - * > - * If a child process has its own domain not shared with the parent > - * process (case:domain_child = true, then the child can't trace the > - * parent. > + * can_trace_child is true if a parent process can trace its child > + * process. This depends on two conditions: > + * - The parent process is not isolated from the child with a dedicated > + * Landlock domain. > + * - Yama allows tracing children (up to YAMA_SCOPE_RELATIONAL). > */ > - can_trace_parent = !variant->domain_child && > - yama_ptrace_scope < YAMA_SCOPE_RELATIONAL; > + can_trace_child = can_read_child && > + yama_ptrace_scope <= YAMA_SCOPE_RELATIONAL; > + > + /* > + * can_read_parent is true if a child process can read its parent > + * process, which is only the case when the child process is not > + * isolated from the parent with a dedicated Landlock domain. > + */ > + can_read_parent = !variant->domain_child; > + > + /* > + * can_trace_parent is true if a child process can trace its parent > + * process. This depends on two conditions: > + * - The child process is not isolated from the parent with a dedicated > + * Landlock domain. > + * - Yama is disabled (YAMA_SCOPE_DISABLED). > + */ > + can_trace_parent = can_read_parent && > + yama_ptrace_scope <= YAMA_SCOPE_DISABLED; > > /* > * Removes all effective and permitted capabilities to not interfere > @@ -330,16 +332,21 @@ TEST_F(hierarchy, trace) > /* Waits for the parent to be in a domain, if any. */ > ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1)); > > - /* Tests PTRACE_ATTACH and PTRACE_MODE_READ on the parent. */ > + /* Tests PTRACE_MODE_READ on the parent. */ > err_proc_read = test_ptrace_read(parent); > + if (can_read_parent) { > + EXPECT_EQ(0, err_proc_read); > + } else { > + EXPECT_EQ(EACCES, err_proc_read); > + } > + > + /* Tests PTRACE_ATTACH on the parent. */ > ret = ptrace(PTRACE_ATTACH, parent, NULL, 0); > - if (!can_trace_parent) { > + if (can_trace_parent) { > + EXPECT_EQ(0, ret); > + } else { > EXPECT_EQ(-1, ret); > EXPECT_EQ(EPERM, errno); > - EXPECT_EQ(EACCES, err_proc_read); > - } else { > - EXPECT_EQ(0, ret); > - EXPECT_EQ(0, err_proc_read); > } > if (ret == 0) { > ASSERT_EQ(parent, waitpid(parent, &status, 0)); > @@ -349,11 +356,11 @@ TEST_F(hierarchy, trace) > > /* Tests child PTRACE_TRACEME. */ > ret = ptrace(PTRACE_TRACEME); > - if (!can_trace_child) { > + if (can_trace_child) { > + EXPECT_EQ(0, ret); > + } else { > EXPECT_EQ(-1, ret); > EXPECT_EQ(EPERM, errno); > - } else { > - EXPECT_EQ(0, ret); > } > > /* > @@ -396,17 +403,23 @@ TEST_F(hierarchy, trace) > EXPECT_EQ(ESRCH, errno); > } > > - /* Tests PTRACE_ATTACH and PTRACE_MODE_READ on the child. */ > + /* Tests PTRACE_MODE_READ on the child. */ > err_proc_read = test_ptrace_read(child); > + if (can_read_child) { > + EXPECT_EQ(0, err_proc_read); > + } else { > + EXPECT_EQ(EACCES, err_proc_read); > + } > + > + /* Tests PTRACE_ATTACH on the child. */ > ret = ptrace(PTRACE_ATTACH, child, NULL, 0); > if (!can_trace_child) { I forgot to inverse the !can_trace_child condition to make it more consistent with the rest. Please update it for your next patch. > EXPECT_EQ(-1, ret); > EXPECT_EQ(EPERM, errno); > - EXPECT_EQ(EACCES, err_proc_read); > } else { > EXPECT_EQ(0, ret); > - EXPECT_EQ(0, err_proc_read); > } > + > if (ret == 0) { > ASSERT_EQ(child, waitpid(child, &status, 0)); > ASSERT_EQ(1, WIFSTOPPED(status)); > @@ -419,11 +432,6 @@ TEST_F(hierarchy, trace) > if (WIFSIGNALED(status) || !WIFEXITED(status) || > WEXITSTATUS(status) != EXIT_SUCCESS) > _metadata->passed = 0; > - > - if (yama_ptrace_scope > 0) > - SKIP(return, > - "Incomplete tests due to Yama restrictions (scope %d)", > - yama_ptrace_scope); > } > > TEST_HARNESS_MAIN
Done. Thank you for double checking the read operation. The test case is more complete with that. -Jeff On Fri, Jan 13, 2023 at 10:12 AM Mickaël Salaün <mic@digikod.net> wrote: > > > On 13/01/2023 18:53, Mickaël Salaün wrote: > > I found some issues running this new ptrace_test. Indeed, Yama doesn't > > restrict PTRACE_MODE_READ whereas Landlock does. I also changed some > > comments and socpe conditions. In fact, the final SKIP() wasn't a good > > idea because it masks such potential errors; let's only use a log > > message instead. I also removed the first SKIP() because we want > > everything to be tested, and the MODE_READ does still matter anyway. > > > > Please review this patch and squash it in yours. > > > > Signed-off-by: Mickaël Salaün <mic@digikod.net> > > Link: https://lore.kernel.org/r/20230113175308.50900-1-mic@digikod.net > > --- > > .../testing/selftests/landlock/ptrace_test.c | 106 ++++++++++-------- > > 1 file changed, 57 insertions(+), 49 deletions(-) > > > > diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c > > index c9d9f3001d0f..3c3ba37bf8ec 100644 > > --- a/tools/testing/selftests/landlock/ptrace_test.c > > +++ b/tools/testing/selftests/landlock/ptrace_test.c > > @@ -19,7 +19,7 @@ > > > > #include "common.h" > > > > -/* copy from yama_lsm.c */ > > +/* Copied from security/yama/yama_lsm.c */ > > #define YAMA_SCOPE_DISABLED 0 > > #define YAMA_SCOPE_RELATIONAL 1 > > #define YAMA_SCOPE_CAPABILITY 2 > > @@ -70,7 +70,7 @@ static int get_yama_ptrace_scope(void) > > { > > int ret; > > char buf[2] = {}; > > - int fd = open("/proc/sys/kernel/yama/ptrace_scope", O_RDONLY); > > + const int fd = open("/proc/sys/kernel/yama/ptrace_scope", O_RDONLY); > > > > if (fd < 0) > > return 0; > > @@ -260,46 +260,48 @@ TEST_F(hierarchy, trace) > > int yama_ptrace_scope; > > char buf_parent; > > long ret; > > - bool can_trace_child, can_trace_parent; > > + bool can_read_child, can_trace_child, can_read_parent, can_trace_parent; > > > > yama_ptrace_scope = get_yama_ptrace_scope(); > > ASSERT_LE(0, yama_ptrace_scope); > > > > - if (yama_ptrace_scope >= YAMA_SCOPE_NO_ATTACH) > > - SKIP(return, "Yama forbids any ptrace use (scope %d)", > > - yama_ptrace_scope); > > + if (yama_ptrace_scope > YAMA_SCOPE_DISABLED) > > + TH_LOG("Incomplete tests due to Yama restrictions (scope %d)", > > + yama_ptrace_scope); > > > > /* > > - * can_trace_child: if a parent process can trace its child process. > > - * > > - * There are two conditions concerning landlock: > > - * 1> the parent and child processes are in the same landlock domain or > > - * one beneath it (case: domain_both = true). > > - * 2> yama allows tracing children (up to YAMA_SCOPE_RELATIONAL). > > - * Both 1 and 2 need to be met for can_trace_child to be true. > > - * > > - * If a parent process has its own domain not shared with the child > > - * process (case:domain_parent = true), then the parent can't trace the > > - * child. > > + * can_read_child is true if a parent process can read its child > > + * process, which is only the case when the parent process is not > > + * isolated from the child with a dedicated Landlock domain. > > */ > > - can_trace_child = !variant->domain_parent && > > - yama_ptrace_scope < YAMA_SCOPE_CAPABILITY; > > + can_read_child = !variant->domain_parent; > > > > /* > > - * can_trace_parent: if a child process can trace its parent process. > > - * > > - * There are two conditions concerning landlock: > > - * 1> the parent and child process are in the same landlock domain or > > - * one beneath it.(case: domain_both = true). > > - * 2> yama is disabled (YAMA_SCOPE_DISABLED). > > - * Both 1 and 2 need to be met for can_trace_parent to be true. > > - * > > - * If a child process has its own domain not shared with the parent > > - * process (case:domain_child = true, then the child can't trace the > > - * parent. > > + * can_trace_child is true if a parent process can trace its child > > + * process. This depends on two conditions: > > + * - The parent process is not isolated from the child with a dedicated > > + * Landlock domain. > > + * - Yama allows tracing children (up to YAMA_SCOPE_RELATIONAL). > > */ > > - can_trace_parent = !variant->domain_child && > > - yama_ptrace_scope < YAMA_SCOPE_RELATIONAL; > > + can_trace_child = can_read_child && > > + yama_ptrace_scope <= YAMA_SCOPE_RELATIONAL; > > + > > + /* > > + * can_read_parent is true if a child process can read its parent > > + * process, which is only the case when the child process is not > > + * isolated from the parent with a dedicated Landlock domain. > > + */ > > + can_read_parent = !variant->domain_child; > > + > > + /* > > + * can_trace_parent is true if a child process can trace its parent > > + * process. This depends on two conditions: > > + * - The child process is not isolated from the parent with a dedicated > > + * Landlock domain. > > + * - Yama is disabled (YAMA_SCOPE_DISABLED). > > + */ > > + can_trace_parent = can_read_parent && > > + yama_ptrace_scope <= YAMA_SCOPE_DISABLED; > > > > /* > > * Removes all effective and permitted capabilities to not interfere > > @@ -330,16 +332,21 @@ TEST_F(hierarchy, trace) > > /* Waits for the parent to be in a domain, if any. */ > > ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1)); > > > > - /* Tests PTRACE_ATTACH and PTRACE_MODE_READ on the parent. */ > > + /* Tests PTRACE_MODE_READ on the parent. */ > > err_proc_read = test_ptrace_read(parent); > > + if (can_read_parent) { > > + EXPECT_EQ(0, err_proc_read); > > + } else { > > + EXPECT_EQ(EACCES, err_proc_read); > > + } > > + > > + /* Tests PTRACE_ATTACH on the parent. */ > > ret = ptrace(PTRACE_ATTACH, parent, NULL, 0); > > - if (!can_trace_parent) { > > + if (can_trace_parent) { > > + EXPECT_EQ(0, ret); > > + } else { > > EXPECT_EQ(-1, ret); > > EXPECT_EQ(EPERM, errno); > > - EXPECT_EQ(EACCES, err_proc_read); > > - } else { > > - EXPECT_EQ(0, ret); > > - EXPECT_EQ(0, err_proc_read); > > } > > if (ret == 0) { > > ASSERT_EQ(parent, waitpid(parent, &status, 0)); > > @@ -349,11 +356,11 @@ TEST_F(hierarchy, trace) > > > > /* Tests child PTRACE_TRACEME. */ > > ret = ptrace(PTRACE_TRACEME); > > - if (!can_trace_child) { > > + if (can_trace_child) { > > + EXPECT_EQ(0, ret); > > + } else { > > EXPECT_EQ(-1, ret); > > EXPECT_EQ(EPERM, errno); > > - } else { > > - EXPECT_EQ(0, ret); > > } > > > > /* > > @@ -396,17 +403,23 @@ TEST_F(hierarchy, trace) > > EXPECT_EQ(ESRCH, errno); > > } > > > > - /* Tests PTRACE_ATTACH and PTRACE_MODE_READ on the child. */ > > + /* Tests PTRACE_MODE_READ on the child. */ > > err_proc_read = test_ptrace_read(child); > > + if (can_read_child) { > > + EXPECT_EQ(0, err_proc_read); > > + } else { > > + EXPECT_EQ(EACCES, err_proc_read); > > + } > > + > > + /* Tests PTRACE_ATTACH on the child. */ > > ret = ptrace(PTRACE_ATTACH, child, NULL, 0); > > if (!can_trace_child) { > > I forgot to inverse the !can_trace_child condition to make it more > consistent with the rest. Please update it for your next patch. > > > > EXPECT_EQ(-1, ret); > > EXPECT_EQ(EPERM, errno); > > - EXPECT_EQ(EACCES, err_proc_read); > > } else { > > EXPECT_EQ(0, ret); > > - EXPECT_EQ(0, err_proc_read); > > } > > + > > if (ret == 0) { > > ASSERT_EQ(child, waitpid(child, &status, 0)); > > ASSERT_EQ(1, WIFSTOPPED(status)); > > @@ -419,11 +432,6 @@ TEST_F(hierarchy, trace) > > if (WIFSIGNALED(status) || !WIFEXITED(status) || > > WEXITSTATUS(status) != EXIT_SUCCESS) > > _metadata->passed = 0; > > - > > - if (yama_ptrace_scope > 0) > > - SKIP(return, > > - "Incomplete tests due to Yama restrictions (scope %d)", > > - yama_ptrace_scope); > > } > > > > TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c index c9d9f3001d0f..3c3ba37bf8ec 100644 --- a/tools/testing/selftests/landlock/ptrace_test.c +++ b/tools/testing/selftests/landlock/ptrace_test.c @@ -19,7 +19,7 @@ #include "common.h" -/* copy from yama_lsm.c */ +/* Copied from security/yama/yama_lsm.c */ #define YAMA_SCOPE_DISABLED 0 #define YAMA_SCOPE_RELATIONAL 1 #define YAMA_SCOPE_CAPABILITY 2 @@ -70,7 +70,7 @@ static int get_yama_ptrace_scope(void) { int ret; char buf[2] = {}; - int fd = open("/proc/sys/kernel/yama/ptrace_scope", O_RDONLY); + const int fd = open("/proc/sys/kernel/yama/ptrace_scope", O_RDONLY); if (fd < 0) return 0; @@ -260,46 +260,48 @@ TEST_F(hierarchy, trace) int yama_ptrace_scope; char buf_parent; long ret; - bool can_trace_child, can_trace_parent; + bool can_read_child, can_trace_child, can_read_parent, can_trace_parent; yama_ptrace_scope = get_yama_ptrace_scope(); ASSERT_LE(0, yama_ptrace_scope); - if (yama_ptrace_scope >= YAMA_SCOPE_NO_ATTACH) - SKIP(return, "Yama forbids any ptrace use (scope %d)", - yama_ptrace_scope); + if (yama_ptrace_scope > YAMA_SCOPE_DISABLED) + TH_LOG("Incomplete tests due to Yama restrictions (scope %d)", + yama_ptrace_scope); /* - * can_trace_child: if a parent process can trace its child process. - * - * There are two conditions concerning landlock: - * 1> the parent and child processes are in the same landlock domain or - * one beneath it (case: domain_both = true). - * 2> yama allows tracing children (up to YAMA_SCOPE_RELATIONAL). - * Both 1 and 2 need to be met for can_trace_child to be true. - * - * If a parent process has its own domain not shared with the child - * process (case:domain_parent = true), then the parent can't trace the - * child. + * can_read_child is true if a parent process can read its child + * process, which is only the case when the parent process is not + * isolated from the child with a dedicated Landlock domain. */ - can_trace_child = !variant->domain_parent && - yama_ptrace_scope < YAMA_SCOPE_CAPABILITY; + can_read_child = !variant->domain_parent; /* - * can_trace_parent: if a child process can trace its parent process. - * - * There are two conditions concerning landlock: - * 1> the parent and child process are in the same landlock domain or - * one beneath it.(case: domain_both = true). - * 2> yama is disabled (YAMA_SCOPE_DISABLED). - * Both 1 and 2 need to be met for can_trace_parent to be true. - * - * If a child process has its own domain not shared with the parent - * process (case:domain_child = true, then the child can't trace the - * parent. + * can_trace_child is true if a parent process can trace its child + * process. This depends on two conditions: + * - The parent process is not isolated from the child with a dedicated + * Landlock domain. + * - Yama allows tracing children (up to YAMA_SCOPE_RELATIONAL). */ - can_trace_parent = !variant->domain_child && - yama_ptrace_scope < YAMA_SCOPE_RELATIONAL; + can_trace_child = can_read_child && + yama_ptrace_scope <= YAMA_SCOPE_RELATIONAL; + + /* + * can_read_parent is true if a child process can read its parent + * process, which is only the case when the child process is not + * isolated from the parent with a dedicated Landlock domain. + */ + can_read_parent = !variant->domain_child; + + /* + * can_trace_parent is true if a child process can trace its parent + * process. This depends on two conditions: + * - The child process is not isolated from the parent with a dedicated + * Landlock domain. + * - Yama is disabled (YAMA_SCOPE_DISABLED). + */ + can_trace_parent = can_read_parent && + yama_ptrace_scope <= YAMA_SCOPE_DISABLED; /* * Removes all effective and permitted capabilities to not interfere @@ -330,16 +332,21 @@ TEST_F(hierarchy, trace) /* Waits for the parent to be in a domain, if any. */ ASSERT_EQ(1, read(pipe_parent[0], &buf_child, 1)); - /* Tests PTRACE_ATTACH and PTRACE_MODE_READ on the parent. */ + /* Tests PTRACE_MODE_READ on the parent. */ err_proc_read = test_ptrace_read(parent); + if (can_read_parent) { + EXPECT_EQ(0, err_proc_read); + } else { + EXPECT_EQ(EACCES, err_proc_read); + } + + /* Tests PTRACE_ATTACH on the parent. */ ret = ptrace(PTRACE_ATTACH, parent, NULL, 0); - if (!can_trace_parent) { + if (can_trace_parent) { + EXPECT_EQ(0, ret); + } else { EXPECT_EQ(-1, ret); EXPECT_EQ(EPERM, errno); - EXPECT_EQ(EACCES, err_proc_read); - } else { - EXPECT_EQ(0, ret); - EXPECT_EQ(0, err_proc_read); } if (ret == 0) { ASSERT_EQ(parent, waitpid(parent, &status, 0)); @@ -349,11 +356,11 @@ TEST_F(hierarchy, trace) /* Tests child PTRACE_TRACEME. */ ret = ptrace(PTRACE_TRACEME); - if (!can_trace_child) { + if (can_trace_child) { + EXPECT_EQ(0, ret); + } else { EXPECT_EQ(-1, ret); EXPECT_EQ(EPERM, errno); - } else { - EXPECT_EQ(0, ret); } /* @@ -396,17 +403,23 @@ TEST_F(hierarchy, trace) EXPECT_EQ(ESRCH, errno); } - /* Tests PTRACE_ATTACH and PTRACE_MODE_READ on the child. */ + /* Tests PTRACE_MODE_READ on the child. */ err_proc_read = test_ptrace_read(child); + if (can_read_child) { + EXPECT_EQ(0, err_proc_read); + } else { + EXPECT_EQ(EACCES, err_proc_read); + } + + /* Tests PTRACE_ATTACH on the child. */ ret = ptrace(PTRACE_ATTACH, child, NULL, 0); if (!can_trace_child) { EXPECT_EQ(-1, ret); EXPECT_EQ(EPERM, errno); - EXPECT_EQ(EACCES, err_proc_read); } else { EXPECT_EQ(0, ret); - EXPECT_EQ(0, err_proc_read); } + if (ret == 0) { ASSERT_EQ(child, waitpid(child, &status, 0)); ASSERT_EQ(1, WIFSTOPPED(status)); @@ -419,11 +432,6 @@ TEST_F(hierarchy, trace) if (WIFSIGNALED(status) || !WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS) _metadata->passed = 0; - - if (yama_ptrace_scope > 0) - SKIP(return, - "Incomplete tests due to Yama restrictions (scope %d)", - yama_ptrace_scope); } TEST_HARNESS_MAIN
I found some issues running this new ptrace_test. Indeed, Yama doesn't restrict PTRACE_MODE_READ whereas Landlock does. I also changed some comments and socpe conditions. In fact, the final SKIP() wasn't a good idea because it masks such potential errors; let's only use a log message instead. I also removed the first SKIP() because we want everything to be tested, and the MODE_READ does still matter anyway. Please review this patch and squash it in yours. Signed-off-by: Mickaël Salaün <mic@digikod.net> Link: https://lore.kernel.org/r/20230113175308.50900-1-mic@digikod.net --- .../testing/selftests/landlock/ptrace_test.c | 106 ++++++++++-------- 1 file changed, 57 insertions(+), 49 deletions(-)