From 4a2989dbba94e8871a2d4f2dbce3646012313555 Mon Sep 17 00:00:00 2001 From: Kholoud Mohamed Date: Wed, 10 May 2023 16:51:09 +0000 Subject: [PATCH] Handle errors during policy loading from file Bug: 280605194 Test: manual Change-Id: Id47f482e9fced82b42090eabed6297f2ebba223e Merged-In: Id47f482e9fced82b42090eabed6297f2ebba223e --- core/java/android/app/admin/PolicyKey.java | 9 ++++++ .../devicepolicy/DevicePolicyEngine.java | 21 +++++++++---- .../server/devicepolicy/EnforcingAdmin.java | 17 ++++++++++ .../server/devicepolicy/OwnersData.java | 1 + .../server/devicepolicy/PolicyDefinition.java | 22 +++++++++++++ .../server/devicepolicy/PolicyState.java | 31 ++++++++++++++++--- 6 files changed, 91 insertions(+), 10 deletions(-) diff --git a/core/java/android/app/admin/PolicyKey.java b/core/java/android/app/admin/PolicyKey.java index 3544c19fe869e..9b12e5969c2e4 100644 --- a/core/java/android/app/admin/PolicyKey.java +++ b/core/java/android/app/admin/PolicyKey.java @@ -25,6 +25,7 @@ import android.os.Parcelable; import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; +import android.util.Log; import org.xmlpull.v1.XmlPullParserException; @@ -41,6 +42,9 @@ import java.util.Objects; @SuppressLint({"ParcelNotFinal", "ParcelCreator"}) @SystemApi public abstract class PolicyKey implements Parcelable { + + static final String TAG = "PolicyKey"; + /** * @hide */ @@ -76,9 +80,14 @@ public abstract class PolicyKey implements Parcelable { /** * @hide */ + @Nullable public static PolicyKey readGenericPolicyKeyFromXml(TypedXmlPullParser parser) { String identifier = parser.getAttributeValue( /* namespace= */ null, ATTR_POLICY_IDENTIFIER); + if (identifier == null) { + Log.wtf(TAG, "Error parsing generic policy key, identifier is null."); + return null; + } return new NoArgsPolicyKey(identifier); } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java index 1392d0232262a..1322225d8e930 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java @@ -56,6 +56,7 @@ import android.util.Xml; import com.android.internal.util.XmlUtils; import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; +import com.android.server.utils.Slogf; import libcore.io.IoUtils; @@ -1511,7 +1512,7 @@ final class DevicePolicyEngine { readInner(parser); } catch (XmlPullParserException | IOException | ClassNotFoundException e) { - Log.e(TAG, "Error parsing resources file", e); + Slogf.wtf(TAG, "Error parsing resources file", e); } finally { IoUtils.closeQuietly(input); } @@ -1533,7 +1534,7 @@ final class DevicePolicyEngine { readEnforcingAdminsInner(parser); break; default: - Log.e(TAG, "Unknown tag " + tag); + Slogf.wtf(TAG, "Unknown tag " + tag); } } } @@ -1554,7 +1555,7 @@ final class DevicePolicyEngine { policyState = PolicyState.readFromXml(parser); break; default: - Log.e(TAG, "Unknown tag for local policy entry" + tag); + Slogf.wtf(TAG, "Unknown tag for local policy entry" + tag); } } @@ -1564,7 +1565,9 @@ final class DevicePolicyEngine { } mLocalPolicies.get(userId).put(policyKey, policyState); } else { - Log.e(TAG, "Error parsing local policy"); + Slogf.wtf(TAG, "Error parsing local policy, policyKey is " + + (policyKey == null ? "null" : policyKey) + ", and policyState is " + + (policyState == null ? "null" : policyState) + "."); } } @@ -1583,20 +1586,26 @@ final class DevicePolicyEngine { policyState = PolicyState.readFromXml(parser); break; default: - Log.e(TAG, "Unknown tag for local policy entry" + tag); + Slogf.wtf(TAG, "Unknown tag for local policy entry" + tag); } } if (policyKey != null && policyState != null) { mGlobalPolicies.put(policyKey, policyState); } else { - Log.e(TAG, "Error parsing global policy"); + Slogf.wtf(TAG, "Error parsing global policy, policyKey is " + + (policyKey == null ? "null" : policyKey) + ", and policyState is " + + (policyState == null ? "null" : policyState) + "."); } } private void readEnforcingAdminsInner(TypedXmlPullParser parser) throws XmlPullParserException { EnforcingAdmin admin = EnforcingAdmin.readFromXml(parser); + if (admin == null) { + Slogf.wtf(TAG, "Error parsing enforcingAdmins, EnforcingAdmin is null."); + return; + } if (!mEnforcingAdmins.contains(admin.getUserId())) { mEnforcingAdmins.put(admin.getUserId(), new HashSet<>()); } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java b/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java index 3ed2d34be6dfc..5243d14af1ab6 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java @@ -30,6 +30,7 @@ import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; import com.android.role.RoleManagerLocal; import com.android.server.LocalManagerRegistry; +import com.android.server.utils.Slogf; import org.xmlpull.v1.XmlPullParserException; @@ -51,6 +52,9 @@ import java.util.Set; * */ final class EnforcingAdmin { + + static final String TAG = "EnforcingAdmin"; + static final String ROLE_AUTHORITY_PREFIX = "role:"; static final String DPC_AUTHORITY = "enterprise"; static final String DEVICE_ADMIN_AUTHORITY = "device_admin"; @@ -286,6 +290,7 @@ final class EnforcingAdmin { } } + @Nullable static EnforcingAdmin readFromXml(TypedXmlPullParser parser) throws XmlPullParserException { String packageName = parser.getAttributeValue(/* namespace= */ null, ATTR_PACKAGE_NAME); @@ -294,13 +299,25 @@ final class EnforcingAdmin { int userId = parser.getAttributeInt(/* namespace= */ null, ATTR_USER_ID); if (isRoleAuthority) { + if (packageName == null) { + Slogf.wtf(TAG, "Error parsing EnforcingAdmin with RoleAuthority, packageName is " + + "null."); + return null; + } // TODO(b/281697976): load active admin return new EnforcingAdmin(packageName, userId, null); } else { + if (packageName == null || authoritiesStr == null) { + Slogf.wtf(TAG, "Error parsing EnforcingAdmin, packageName is " + + (packageName == null ? "null" : packageName) + ", and authorities is " + + (authoritiesStr == null ? "null" : authoritiesStr) + "."); + return null; + } String className = parser.getAttributeValue(/* namespace= */ null, ATTR_CLASS_NAME); ComponentName componentName = className == null ? null : new ComponentName(packageName, className); Set authorities = Set.of(authoritiesStr.split(ATTR_AUTHORITIES_SEPARATOR)); + // TODO(b/281697976): load active admin return new EnforcingAdmin(packageName, componentName, authorities, userId, null); } } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/OwnersData.java b/services/devicepolicy/java/com/android/server/devicepolicy/OwnersData.java index 63b250d4acfcd..37d4f95cac29a 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/OwnersData.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/OwnersData.java @@ -457,6 +457,7 @@ class OwnersData { case TAG_POLICY_ENGINE_MIGRATION: mMigratedToPolicyEngine = parser.getAttributeBoolean( null, ATTR_MIGRATED_TO_POLICY_ENGINE, false); + break; default: Slog.e(TAG, "Unexpected tag: " + tag); return false; diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java index 7e5bb0bb40ba0..7e48407fc9117 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java @@ -42,6 +42,7 @@ import android.os.UserManager; import com.android.internal.util.function.QuadFunction; import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; +import com.android.server.utils.Slogf; import org.xmlpull.v1.XmlPullParserException; @@ -53,6 +54,9 @@ import java.util.Map; import java.util.Set; final class PolicyDefinition { + + static final String TAG = "PolicyDefinition"; + private static final int POLICY_FLAG_NONE = 0; // Only use this flag if a policy can not be applied locally. @@ -596,22 +600,40 @@ final class PolicyDefinition { mPolicyKey.saveToXml(serializer); } + @Nullable static PolicyDefinition readFromXml(TypedXmlPullParser parser) throws XmlPullParserException, IOException { // TODO: can we avoid casting? PolicyKey policyKey = readPolicyKeyFromXml(parser); + if (policyKey == null) { + Slogf.wtf(TAG, "Error parsing PolicyDefinition, PolicyKey is null."); + return null; + } PolicyDefinition genericPolicyDefinition = (PolicyDefinition) POLICY_DEFINITIONS.get(policyKey.getIdentifier()); + if (genericPolicyDefinition == null) { + Slogf.wtf(TAG, "Unknown generic policy key: " + policyKey); + return null; + } return genericPolicyDefinition.createPolicyDefinition(policyKey); } + @Nullable static PolicyKey readPolicyKeyFromXml(TypedXmlPullParser parser) throws XmlPullParserException, IOException { // TODO: can we avoid casting? PolicyKey policyKey = PolicyKey.readGenericPolicyKeyFromXml(parser); + if (policyKey == null) { + Slogf.wtf(TAG, "Error parsing PolicyKey, GenericPolicyKey is null"); + return null; + } PolicyDefinition> genericPolicyDefinition = (PolicyDefinition>) POLICY_DEFINITIONS.get( policyKey.getIdentifier()); + if (genericPolicyDefinition == null) { + Slogf.wtf(TAG, "Error parsing PolicyKey, Unknown generic policy key: " + policyKey); + return null; + } return genericPolicyDefinition.mPolicyKey.readFromXml(parser); } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyState.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyState.java index dd4c6afdcfb68..599c4a7441c57 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyState.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyState.java @@ -19,11 +19,11 @@ package com.android.server.devicepolicy; import android.annotation.NonNull; import android.annotation.Nullable; import android.app.admin.PolicyValue; -import android.util.Log; import com.android.internal.util.XmlUtils; import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; +import com.android.server.utils.Slogf; import org.xmlpull.v1.XmlPullParserException; @@ -224,6 +224,7 @@ final class PolicyState { } } + @Nullable static PolicyState readFromXml(TypedXmlPullParser parser) throws IOException, XmlPullParserException { @@ -245,33 +246,55 @@ final class PolicyState { switch (adminPolicyTag) { case TAG_ENFORCING_ADMIN_ENTRY: admin = EnforcingAdmin.readFromXml(parser); + if (admin == null) { + Slogf.wtf(TAG, "Error Parsing TAG_ENFORCING_ADMIN_ENTRY, " + + "EnforcingAdmin is null"); + } break; case TAG_POLICY_VALUE_ENTRY: value = policyDefinition.readPolicyValueFromXml(parser); + if (value == null) { + Slogf.wtf(TAG, "Error Parsing TAG_POLICY_VALUE_ENTRY, " + + "PolicyValue is null"); + } break; } } if (admin != null) { policiesSetByAdmins.put(admin, value); } else { - Log.e(TAG, "Error Parsing TAG_ADMIN_POLICY_ENTRY"); + Slogf.wtf(TAG, "Error Parsing TAG_ADMIN_POLICY_ENTRY, EnforcingAdmin " + + "is null"); } break; case TAG_POLICY_DEFINITION_ENTRY: policyDefinition = PolicyDefinition.readFromXml(parser); + if (policyDefinition == null) { + Slogf.wtf(TAG, "Error Parsing TAG_POLICY_DEFINITION_ENTRY, " + + "PolicyDefinition is null"); + } break; case TAG_RESOLVED_VALUE_ENTRY: + if (policyDefinition == null) { + Slogf.wtf(TAG, "Error Parsing TAG_RESOLVED_VALUE_ENTRY, " + + "policyDefinition is null"); + break; + } currentResolvedPolicy = policyDefinition.readPolicyValueFromXml(parser); + if (currentResolvedPolicy == null) { + Slogf.wtf(TAG, "Error Parsing TAG_RESOLVED_VALUE_ENTRY, " + + "currentResolvedPolicy is null"); + } break; default: - Log.e(TAG, "Unknown tag: " + tag); + Slogf.wtf(TAG, "Unknown tag: " + tag); } } if (policyDefinition != null) { return new PolicyState(policyDefinition, policiesSetByAdmins, currentResolvedPolicy); } else { - Log.e("PolicyState", "Error parsing policyState"); + Slogf.wtf(TAG, "Error parsing policyState, policyDefinition is null"); return null; } }