[libvirt] [PATCH 0/2] virt-aa-helper cleanups
As follow up to [1] some smaller extensions and fixups to virt-aa-helper and its tests. [1]: https://www.redhat.com/archives/libvir-list/2017-October/msg01161.html Christian Ehrhardt (2): virt-aa-helper: apparmor wildcards to forbidden chars virt-aa-helper-test: only fails go to stdout by default src/security/virt-aa-helper.c | 2 +- tests/virt-aa-helper-test | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) -- 2.7.4
Some globbing chars in the domain name could be used to break out of apparmor rules, so lets forbid these when in virt-aa-helper. Also adding a test to ensure all those cases were detected as bad char. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 2 +- tests/virt-aa-helper-test | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index ef1bf01..4f1c195 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -449,7 +449,7 @@ valid_name(const char *name) { /* just try to filter out any dangerous characters in the name that can be * used to subvert the profile */ - const char *bad = "/[]*"; + const char *bad = "/[]{}?^,\"*"; if (strlen(name) == 0) return -1; diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 0599046..e837668 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -211,6 +211,26 @@ testme "1" "-c with no os.type" "-c -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,hvm</type>,hvm_invalid</type>,g" "$template_xml" > "$test_xml" testme "1" "-c with invalid hvm" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-/,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char /" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-[,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char [" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-],g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ]" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-{,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char {" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-},g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char }" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-?,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ?" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-^,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ^" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-\,,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ," "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-\",g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char \"" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt-aa-helper-test,virt-aa-helper-test-*,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char *" "-c -u $valid_uuid" "$test_xml" echo "Expected pass:" >$output sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" "$template_xml" > "$test_xml" -- 2.7.4
On Thu, 2017-10-26 at 15:22 +0200, Christian Ehrhardt wrote:
Some globbing chars in the domain name could be used to break out of apparmor rules, so lets forbid these when in virt-aa-helper.
Also adding a test to ensure all those cases were detected as bad char.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 2 +- tests/virt-aa-helper-test | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa- helper.c index ef1bf01..4f1c195 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -449,7 +449,7 @@ valid_name(const char *name) { /* just try to filter out any dangerous characters in the name that can be * used to subvert the profile */ - const char *bad = "/[]*"; + const char *bad = "/[]{}?^,\"*";
if (strlen(name) == 0) return -1; diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index 0599046..e837668 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -211,6 +211,26 @@ testme "1" "-c with no os.type" "-c -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,hvm</type>,hvm_invalid</type>,g" "$template_xml" > "$test_xml" testme "1" "-c with invalid hvm" "-c -u $valid_uuid" "$test_xml"
+sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-/,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char /" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-[,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char [" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-],g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ]" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-{,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char {" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-},g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char }" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-?,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ?" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-^,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ^" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-\,,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char ," "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-\",g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char \"" "-c -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,virt- aa-helper-test,virt-aa-helper-test-*,g" "$template_xml" > "$test_xml" +testme "1" "-c with invalid domain name char *" "-c -u $valid_uuid" "$test_xml"
echo "Expected pass:" >$output sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" "$template_xml" > "$test_xml"
LGTM, thanks! -- Jamie Strandboge | http://www.canonical.com
By Default (without -d) the tests will only print Failures. So a log should follow general "no message is a good message" style. But the testfw checks always emit the skip info to stdout. Instead they should use the redirection that is controlled by -d. This avoids mesages like the following to clutter the log: Skipping FW AAVMF32 test. Could not find /usr/share/AAVMF/AAVMF32_CODE.fd Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- tests/virt-aa-helper-test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index e837668..d2a557e 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -155,7 +155,7 @@ testfw() { -e "s,</os>,<loader readonly='yes' type='pflash'>$fwpath</loader></os>,g" "$template_xml" > "$test_xml" testme "0" "$title" "-r -u $valid_uuid" "$test_xml" else - echo "Skipping FW $title test. Could not find $fwpath" + echo "Skipping FW $title test. Could not find $fwpath" >$output fi } -- 2.7.4
On Thu, 2017-10-26 at 15:22 +0200, Christian Ehrhardt wrote:
By Default (without -d) the tests will only print Failures. So a log should follow general "no message is a good message" style.
But the testfw checks always emit the skip info to stdout. Instead they should use the redirection that is controlled by -d.
This avoids mesages like the following to clutter the log: Skipping FW AAVMF32 test. Could not find /usr/share/AAVMF/AAVMF32_CODE.fd
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- tests/virt-aa-helper-test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index e837668..d2a557e 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -155,7 +155,7 @@ testfw() { -e "s,</os>,<loader readonly='yes' type='pflash'>$fwpath</loader></os>,g" "$template_xml" > "$test_xml" testme "0" "$title" "-r -u $valid_uuid" "$test_xml" else - echo "Skipping FW $title test. Could not find $fwpath" + echo "Skipping FW $title test. Could not find $fwpath"
$output fi }
LGTM. Thanks! -- Jamie Strandboge | http://www.canonical.com
On 10/26/2017 03:22 PM, Christian Ehrhardt wrote:
As follow up to [1] some smaller extensions and fixups to virt-aa-helper and its tests.
[1]: https://www.redhat.com/archives/libvir-list/2017-October/msg01161.html
Christian Ehrhardt (2): virt-aa-helper: apparmor wildcards to forbidden chars virt-aa-helper-test: only fails go to stdout by default
src/security/virt-aa-helper.c | 2 +- tests/virt-aa-helper-test | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-)
Thanks, pushed. Michal
participants (3)
-
Christian Ehrhardt -
Jamie Strandboge -
Michal Privoznik