From 21a960507b6250c6ecc84c20b8183f89c32a1e2e Mon Sep 17 00:00:00 2001 From: Khaled Abdelmohsen Date: Wed, 6 Nov 2019 12:23:22 +0000 Subject: [PATCH] Implement XML parsing for open formula Parse a rule's open formula. Bug: 143697198 Test: atest FrameworksServicesTests:RuleXmlParserTest Change-Id: I8e406f9df551afc9171e8065560b15a3ea86f7e4 --- .../integrity/parser/RuleParseException.java | 32 ++ .../server/integrity/parser/RuleParser.java | 4 +- .../integrity/parser/RuleXmlParser.java | 145 +++++---- .../integrity/parser/RuleXmlParserTest.java | 289 +++++++++++++++--- 4 files changed, 369 insertions(+), 101 deletions(-) create mode 100644 services/core/java/com/android/server/integrity/parser/RuleParseException.java diff --git a/services/core/java/com/android/server/integrity/parser/RuleParseException.java b/services/core/java/com/android/server/integrity/parser/RuleParseException.java new file mode 100644 index 0000000000000..c0f36a66528a8 --- /dev/null +++ b/services/core/java/com/android/server/integrity/parser/RuleParseException.java @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.integrity.parser; + +import android.annotation.NonNull; + +/** + * Thrown when rule parsing fails. + */ +public class RuleParseException extends Exception { + public RuleParseException(@NonNull String message) { + super(message); + } + + public RuleParseException(@NonNull String message, @NonNull Throwable cause) { + super(message, cause); + } +} diff --git a/services/core/java/com/android/server/integrity/parser/RuleParser.java b/services/core/java/com/android/server/integrity/parser/RuleParser.java index bfffc70bb12cf..08e0a5db8c204 100644 --- a/services/core/java/com/android/server/integrity/parser/RuleParser.java +++ b/services/core/java/com/android/server/integrity/parser/RuleParser.java @@ -25,8 +25,8 @@ import java.util.List; public interface RuleParser { /** Parse rules from a string. */ - List parse(String ruleText); + List parse(String ruleText) throws RuleParseException; /** Parse rules from an input stream. */ - List parse(InputStream inputStream); + List parse(InputStream inputStream) throws RuleParseException; } diff --git a/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java b/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java index 5e24d21eb09ad..ef5bb6ee1e798 100644 --- a/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java +++ b/services/core/java/com/android/server/integrity/parser/RuleXmlParser.java @@ -16,11 +16,11 @@ package com.android.server.integrity.parser; -import android.util.Slog; import android.util.Xml; import com.android.server.integrity.model.AtomicFormula; import com.android.server.integrity.model.Formula; +import com.android.server.integrity.model.OpenFormula; import com.android.server.integrity.model.Rule; import org.xmlpull.v1.XmlPullParser; @@ -49,29 +49,28 @@ public final class RuleXmlParser implements RuleParser { private static final String KEY_TAG = "Key"; private static final String OPERATOR_TAG = "Operator"; private static final String VALUE_TAG = "Value"; + private static final String CONNECTOR_TAG = "Connector"; @Override - public List parse(String ruleText) { + public List parse(String ruleText) throws RuleParseException { try { XmlPullParser xmlPullParser = Xml.newPullParser(); xmlPullParser.setInput(new StringReader(ruleText)); return parseRules(xmlPullParser); - } catch (XmlPullParserException | IOException e) { - Slog.e(TAG, String.format("Unable to read rules from string: %s", ruleText), e); + } catch (Exception e) { + throw new RuleParseException(e.getMessage(), e); } - return null; } @Override - public List parse(InputStream inputStream) { + public List parse(InputStream inputStream) throws RuleParseException { try { XmlPullParser xmlPullParser = Xml.newPullParser(); xmlPullParser.setInput(inputStream, StandardCharsets.UTF_8.name()); return parseRules(xmlPullParser); - } catch (XmlPullParserException | IOException e) { - Slog.e(TAG, "Unable to read rules from stream", e); + } catch (Exception e) { + throw new RuleParseException(e.getMessage(), e); } - return null; } private static List parseRules(XmlPullParser parser) @@ -99,59 +98,83 @@ public final class RuleXmlParser implements RuleParser { if (eventType != XmlPullParser.START_TAG || !nodeName.equals(RULE_TAG)) { continue; } - Rule parsedRule = parseRule(parser); - if (parsedRule != null) { - rules.add(parsedRule); - } + rules.add(parseRule(parser)); } return rules; } - private static Rule parseRule(XmlPullParser parser) { - try { - Formula formula = null; - @Rule.Effect int effect = 0; + private static Rule parseRule(XmlPullParser parser) throws IOException, XmlPullParserException { + Formula formula = null; + @Rule.Effect int effect = 0; - boolean isValid = true; - int eventType; - while ((eventType = parser.next()) != XmlPullParser.END_DOCUMENT) { - String nodeName = parser.getName(); + int eventType; + while ((eventType = parser.next()) != XmlPullParser.END_DOCUMENT) { + String nodeName = parser.getName(); - if (eventType == XmlPullParser.END_TAG && parser.getName().equals(RULE_TAG)) { - break; - } - - if (eventType == XmlPullParser.START_TAG) { - switch (nodeName) { - case OPEN_FORMULA_TAG: - formula = parseOpenFormula(parser); - break; - case ATOMIC_FORMULA_TAG: - formula = parseAtomicFormula(parser); - break; - case EFFECT_TAG: - effect = Integer.parseInt(extractValue(parser)); - break; - default: - isValid = false; - } - } else { - isValid = false; - } + if (eventType == XmlPullParser.END_TAG && parser.getName().equals(RULE_TAG)) { + break; } - return isValid ? new Rule(formula, effect) : null; - } catch (Exception e) { - // In case of any exceptions arising from constructing the rule, it will be skipped. - // Rules are assumed to be validated on the server. - return null; + if (eventType == XmlPullParser.START_TAG) { + switch (nodeName) { + case OPEN_FORMULA_TAG: + formula = parseOpenFormula(parser); + break; + case ATOMIC_FORMULA_TAG: + formula = parseAtomicFormula(parser); + break; + case EFFECT_TAG: + effect = Integer.parseInt(extractValue(parser)); + break; + default: + throw new RuntimeException( + String.format("Found unexpected tag: %s", nodeName)); + } + } else { + throw new RuntimeException( + String.format("Found unexpected event type: %d", eventType)); + } } + + return new Rule(formula, effect); } - private static Formula parseOpenFormula(XmlPullParser parser) { - // TODO: Implement open formula parser. - return null; + private static Formula parseOpenFormula(XmlPullParser parser) + throws IOException, XmlPullParserException { + @OpenFormula.Connector int connector = 0; + List formulas = new ArrayList<>(); + + int eventType; + while ((eventType = parser.next()) != XmlPullParser.END_DOCUMENT) { + String nodeName = parser.getName(); + + if (eventType == XmlPullParser.END_TAG && parser.getName().equals(OPEN_FORMULA_TAG)) { + break; + } + + if (eventType == XmlPullParser.START_TAG) { + switch (nodeName) { + case CONNECTOR_TAG: + connector = Integer.parseInt(extractValue(parser)); + break; + case ATOMIC_FORMULA_TAG: + formulas.add(parseAtomicFormula(parser)); + break; + case OPEN_FORMULA_TAG: + formulas.add(parseOpenFormula(parser)); + break; + default: + throw new RuntimeException( + String.format("Found unexpected tag: %s", nodeName)); + } + } else { + throw new RuntimeException( + String.format("Found unexpected event type: %d", eventType)); + } + } + + return new OpenFormula(connector, formulas); } private static Formula parseAtomicFormula(XmlPullParser parser) @@ -160,7 +183,6 @@ public final class RuleXmlParser implements RuleParser { @AtomicFormula.Operator int operator = 0; String value = null; - boolean isValid = true; int eventType; while ((eventType = parser.next()) != XmlPullParser.END_DOCUMENT) { String nodeName = parser.getName(); @@ -181,11 +203,15 @@ public final class RuleXmlParser implements RuleParser { value = extractValue(parser); break; default: - isValid = false; + throw new RuntimeException( + String.format("Found unexpected tag: %s", nodeName)); } + } else { + throw new RuntimeException( + String.format("Found unexpected event type: %d", eventType)); } } - return isValid ? constructAtomicFormulaBasedOnKey(key, operator, value) : null; + return constructAtomicFormulaBasedOnKey(key, operator, value); } private static Formula constructAtomicFormulaBasedOnKey(@AtomicFormula.Key int key, @@ -201,16 +227,21 @@ public final class RuleXmlParser implements RuleParser { case AtomicFormula.VERSION_CODE: return new AtomicFormula.IntAtomicFormula(key, operator, Integer.parseInt(value)); default: - return null; + throw new RuntimeException(String.format("Found unexpected key: %d", key)); } } private static String extractValue(XmlPullParser parser) throws IOException, XmlPullParserException { - String value = null; - if (parser.next() == XmlPullParser.TEXT) { + String value; + int eventType = parser.next(); + if (eventType == XmlPullParser.TEXT) { value = parser.getText(); + eventType = parser.next(); + if (eventType == XmlPullParser.END_TAG) { + return value; + } } - return parser.next() == XmlPullParser.END_TAG ? value : null; + throw new RuntimeException(String.format("Found unexpected event type: %d", eventType)); } } diff --git a/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java b/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java index 1aa9046bb1a1c..3b1c3c2b5e8b1 100644 --- a/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java +++ b/services/tests/servicestests/src/com/android/server/integrity/parser/RuleXmlParserTest.java @@ -30,47 +30,250 @@ import org.junit.runners.JUnit4; import java.io.ByteArrayInputStream; import java.io.InputStream; +import java.util.Arrays; import java.util.Collections; import java.util.List; @RunWith(JUnit4.class) public class RuleXmlParserTest { - private static final String VALID_RULE_XML = "" - + "" - + "" - + "" + OpenFormula.NOT + "" - + "" - + "" + AtomicFormula.PACKAGE_NAME + "" - + "" + AtomicFormula.EQ + "" - + "com.app.test" - + "" - + "" - + "" + Rule.DENY + "" - + "" - + ""; - @Test - public void testXmlString_validRule() { + public void testXmlStream_validOpenFormula() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; RuleParser xmlParser = new RuleXmlParser(); - - List rules = xmlParser.parse(VALID_RULE_XML); - - assertThat(rules).isEmpty(); - } - - @Test - public void testXmlStream_validRule() { - RuleParser xmlParser = new RuleXmlParser(); - InputStream inputStream = new ByteArrayInputStream(VALID_RULE_XML.getBytes()); + InputStream inputStream = new ByteArrayInputStream(ruleXmlOpenFormula.getBytes()); + Rule expectedRule = new Rule(new OpenFormula(OpenFormula.NOT, Collections.singletonList( + new AtomicFormula.StringAtomicFormula(AtomicFormula.PACKAGE_NAME, "com.app.test"))), + Rule.DENY); List rules = xmlParser.parse(inputStream); - assertThat(rules).isEmpty(); + assertThat(rules).isEqualTo(Collections.singletonList(expectedRule)); } @Test - public void testXmlString_validAtomicFormula_stringValue() { + public void testXmlString_validOpenFormula_notConnector() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + Rule expectedRule = new Rule(new OpenFormula(OpenFormula.NOT, Collections.singletonList( + new AtomicFormula.StringAtomicFormula(AtomicFormula.PACKAGE_NAME, "com.app.test"))), + Rule.DENY); + + List rules = xmlParser.parse(ruleXmlOpenFormula); + + assertThat(rules).isEqualTo(Collections.singletonList(expectedRule)); + } + + @Test + public void testXmlString_validOpenFormula_andConnector() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.AND + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "" + AtomicFormula.APP_CERTIFICATE + "" + + "" + AtomicFormula.EQ + "" + + "test_cert" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + Rule expectedRule = new Rule(new OpenFormula(OpenFormula.AND, Arrays.asList( + new AtomicFormula.StringAtomicFormula(AtomicFormula.PACKAGE_NAME, "com.app.test"), + new AtomicFormula.StringAtomicFormula(AtomicFormula.APP_CERTIFICATE, "test_cert"))), + Rule.DENY); + + List rules = xmlParser.parse(ruleXmlOpenFormula); + + assertThat(rules).isEqualTo(Collections.singletonList(expectedRule)); + } + + @Test + public void testXmlString_validOpenFormula_orConnector() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.OR + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "" + AtomicFormula.APP_CERTIFICATE + "" + + "" + AtomicFormula.EQ + "" + + "test_cert" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + Rule expectedRule = new Rule(new OpenFormula(OpenFormula.OR, Arrays.asList( + new AtomicFormula.StringAtomicFormula(AtomicFormula.PACKAGE_NAME, "com.app.test"), + new AtomicFormula.StringAtomicFormula(AtomicFormula.APP_CERTIFICATE, "test_cert"))), + Rule.DENY); + + List rules = xmlParser.parse(ruleXmlOpenFormula); + + assertThat(rules).isEqualTo(Collections.singletonList(expectedRule)); + } + + @Test + public void testXmlString_validOpenFormula_differentTagOrder() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + Rule expectedRule = new Rule(new OpenFormula(OpenFormula.NOT, Collections.singletonList( + new AtomicFormula.StringAtomicFormula(AtomicFormula.PACKAGE_NAME, "com.app.test"))), + Rule.DENY); + + List rules = xmlParser.parse(ruleXmlOpenFormula); + + assertThat(rules).isEqualTo(Collections.singletonList(expectedRule)); + } + + @Test + public void testXmlString_invalidOpenFormula_invalidNumberOfFormulas() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "" + AtomicFormula.VERSION_CODE + "" + + "" + AtomicFormula.EQ + "" + + "1" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "Connector NOT must have 1 formula only", + () -> xmlParser.parse(ruleXmlOpenFormula)); + } + + @Test + public void testXmlString_invalidOpenFormula_invalidOperator() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "INVALID_OPERATOR" + + "com.app.test" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "For input string: \"INVALID_OPERATOR\"", + () -> xmlParser.parse(ruleXmlOpenFormula)); + } + + @Test + public void testXmlString_invalidOpenFormula_invalidEffect() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "INVALID_EFFECT" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "For input string: \"INVALID_EFFECT\"", + () -> xmlParser.parse(ruleXmlOpenFormula)); + } + + @Test + public void testXmlString_invalidOpenFormula_invalidTags() throws Exception { + String ruleXmlOpenFormula = "" + + "" + + "" + + "" + OpenFormula.NOT + "" + + "" + + "" + AtomicFormula.PACKAGE_NAME + "" + + "" + AtomicFormula.EQ + "" + + "com.app.test" + + "" + + "" + + "" + Rule.DENY + "" + + "" + + ""; + RuleParser xmlParser = new RuleXmlParser(); + + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "Found unexpected tag: InvalidConnector", + () -> xmlParser.parse(ruleXmlOpenFormula)); + } + + @Test + public void testXmlString_validAtomicFormula_stringValue() throws Exception { String ruleXmlAtomicFormula = "" + "" + "" @@ -92,7 +295,7 @@ public class RuleXmlParserTest { } @Test - public void testXmlString_validAtomicFormula_integerValue() { + public void testXmlString_validAtomicFormula_integerValue() throws Exception { String ruleXmlAtomicFormula = "" + "" + "" @@ -114,7 +317,7 @@ public class RuleXmlParserTest { } @Test - public void testXmlString_validAtomicFormula_booleanValue() { + public void testXmlString_validAtomicFormula_booleanValue() throws Exception { String ruleXmlAtomicFormula = "" + "" + "" @@ -136,7 +339,7 @@ public class RuleXmlParserTest { } @Test - public void testXmlString_validAtomicFormula_differentTagOrder() { + public void testXmlString_validAtomicFormula_differentTagOrder() throws Exception { String ruleXmlAtomicFormula = "" + "" + "" @@ -158,7 +361,7 @@ public class RuleXmlParserTest { } @Test - public void testXmlString_invalidAtomicFormula_invalidTags() { + public void testXmlString_invalidAtomicFormula_invalidTags() throws Exception { String ruleXmlAtomicFormula = "" + "" + "" @@ -171,13 +374,14 @@ public class RuleXmlParserTest { + ""; RuleParser xmlParser = new RuleXmlParser(); - List rules = xmlParser.parse(ruleXmlAtomicFormula); - - assertThat(rules).isEmpty(); + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "Found unexpected tag: BadKey", + () -> xmlParser.parse(ruleXmlAtomicFormula)); } @Test - public void testXmlString_invalidAtomicFormula() { + public void testXmlString_invalidAtomicFormula() throws Exception { String ruleXmlAtomicFormula = "" + "" + "" @@ -190,16 +394,17 @@ public class RuleXmlParserTest { + ""; RuleParser xmlParser = new RuleXmlParser(); - List rules = xmlParser.parse(ruleXmlAtomicFormula); - - assertThat(rules).isEmpty(); + assertExpectException( + RuleParseException.class, + /* expectedExceptionMessageRegex */ "For input string: \"com.app.test\"", + () -> xmlParser.parse(ruleXmlAtomicFormula)); } @Test public void testXmlString_withNoRuleList() { String ruleXmlWithNoRuleList = "" + "" - + "NOT" + + "" + OpenFormula.NOT + "" + "" + "" + AtomicFormula.PACKAGE_NAME + "" + "" + AtomicFormula.EQ + "" @@ -211,7 +416,7 @@ public class RuleXmlParserTest { RuleParser xmlParser = new RuleXmlParser(); assertExpectException( - RuntimeException.class, + RuleParseException.class, /* expectedExceptionMessageRegex */ "Rules must start with tag.", () -> xmlParser.parse(ruleXmlWithNoRuleList)); } @@ -220,7 +425,7 @@ public class RuleXmlParserTest { public void testXmlStream_withNoRuleList() { String ruleXmlWithNoRuleList = "" + "" - + "NOT" + + "" + OpenFormula.NOT + "" + "" + "" + AtomicFormula.PACKAGE_NAME + "" + "" + AtomicFormula.EQ + "" @@ -233,7 +438,7 @@ public class RuleXmlParserTest { RuleParser xmlParser = new RuleXmlParser(); assertExpectException( - RuntimeException.class, + RuleParseException.class, /* expectedExceptionMessageRegex */ "Rules must start with tag.", () -> xmlParser.parse(inputStream)); }