Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Skip to content
Commit b0bcbce7 authored by Eric Biggers's avatar Eric Biggers
Browse files

Lock down the ability to read from the locksettings database

Currently the getString(), getLong(), and getBoolean() methods of
ILockSettings don't require any permission by default.  There are some
specific database keys that they enforce ACCESS_KEYGUARD_SECURE_STORAGE
for, and some other keys that they enforce READ_CONTACTS for.  This is
much too lenient, since it means that anything new that gets added to
the database is automatically readable without any permission.

I've searched through the users of these methods as much as I can.
Virtually all their callers are from SystemUI / Keyguard or Settings, or
from system_server itself with a cleared calling identity, and therefore
would be fine with ACCESS_KEYGUARD_SECURE_STORAGE.  The only potential
exception I found is the public API android.provider.Settings, which
intentionally allows apps targeted to an old API level to read three
LOCK_PATTERN_* settings.  I've kept these three settings unprotected.

Regarding potential unsupported app usage, the non-SDK dashboards show
no hits for any of the following, which should cover all relevant
@UnsupportedAppUsage entry points:

    * ILockSettings.getBoolean()
    * ILockSettings.getLong()
    * ILockSettings.getString()
    * LockPatternUtils.getOwnerInfo()
    * LockPatternUtils.getString()
    * LockPatternUtils.isLockScreenDisabled()
    * LockPatternUtils.isVisiblePatternEnabled()

LOCK_SCREEN_OWNER_INFO and LOCK_SCREEN_OWNER_INFO_ENABLED are an
interesting case in that they were protected by READ_CONTACTS.  However,
looking at the change that added that code (commit 158fe19f,
http://ag/298629) and its associated bug, it seems that READ_CONTACTS
was just used because of the nature of the bug report: it was a security
vulnerability report that was presented as apps being able to get
personal user information without the READ_CONTACTS permission.

That doesn't mean that READ_CONTACTS is the right permission, however.
The "owner info" is somewhat misnamed in that it isn't really owner
info, but rather just the text configured in `Settings -> Display ->
Lock screen -> Add text on lock screen`.  So it can be any sort of
free-form text the user puts there.  It's only Settings and System UI /
Keyguard that access this text.  It's also totally separate from the
emergency information that can be configured via `Settings -> Safety &
emergency`.  Thus, switching to ACCESS_KEYGUARD_SECURE_STORAGE (a
"system-only" permission, instead of a permission that apps can get) for
the owner info seems like the right choice.

All in all, it's likely that this change is safe to make.  If an issue
arises, we can always relax the permission check for specific keys.

Bug: 156606120
Bug: 256170784
Test: Tested setting PIN, rebooting, unlocking, changing PIN
Test: Tested that "Add text on lock screen" still works
Test: atest com.android.server.locksettings
Change-Id: I574d6a3f29b6fc76d964ddd5e9e0f8e4c680084f
parent 7f44da66
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment