From 945490c12e32b1c13b9097c00702558260b2011f Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Thu, 27 Mar 2014 16:37:28 +0000 Subject: [PATCH] Don't double prompt on booting encrypted device vold will store password securely until KeyGuard requests it and hands it on to KeyStore. This is a revision of https://googleplex-android-review.git.corp.google.com/#/c/418123/ which was reverted. It had two bugs in LockSettingsService.checkVoldPassword. 1) We were not checking password for null, which caused an exception 2) checkPattern/checkPassword return true if there is no saved pattern or password. This leads to situations where we get true returned even when the password doesn't match. Call the correct one based on what is there, not what vold thinks ought to be there. Bug: 12990752 Change-Id: I05315753387b1e508de5aa79b5a68ad7315791d4 --- .../android/os/storage/IMountService.java | 60 ++++++++++++++++++- .../internal/widget/ILockSettings.aidl | 1 + .../internal/widget/LockPatternUtils.java | 14 +++++ .../keyguard/KeyguardViewMediator.java | 7 +++ .../android/server/LockSettingsService.java | 59 ++++++++++++++++++ .../java/com/android/server/MountService.java | 53 +++++++++++++++- 6 files changed, 192 insertions(+), 2 deletions(-) diff --git a/core/java/android/os/storage/IMountService.java b/core/java/android/os/storage/IMountService.java index b97734e83ed37..41808603356cb 100644 --- a/core/java/android/os/storage/IMountService.java +++ b/core/java/android/os/storage/IMountService.java @@ -694,6 +694,36 @@ public interface IMountService extends IInterface { return _result; } + public String getPassword() throws RemoteException { + Parcel _data = Parcel.obtain(); + Parcel _reply = Parcel.obtain(); + String _result; + try { + _data.writeInterfaceToken(DESCRIPTOR); + mRemote.transact(Stub.TRANSACTION_getPassword, _data, _reply, 0); + _reply.readException(); + _result = _reply.readString(); + } finally { + _reply.recycle(); + _data.recycle(); + } + return _result; + } + + public void clearPassword() throws RemoteException { + Parcel _data = Parcel.obtain(); + Parcel _reply = Parcel.obtain(); + String _result; + try { + _data.writeInterfaceToken(DESCRIPTOR); + mRemote.transact(Stub.TRANSACTION_clearPassword, _data, _reply, 0); + _reply.readException(); + } finally { + _reply.recycle(); + _data.recycle(); + } + } + public StorageVolume[] getVolumeList() throws RemoteException { Parcel _data = Parcel.obtain(); Parcel _reply = Parcel.obtain(); @@ -846,7 +876,11 @@ public interface IMountService extends IInterface { static final int TRANSACTION_mkdirs = IBinder.FIRST_CALL_TRANSACTION + 34; - static final int TRANSACTION_getPasswordType = IBinder.FIRST_CALL_TRANSACTION + 36; + static final int TRANSACTION_getPasswordType = IBinder.FIRST_CALL_TRANSACTION + 35; + + static final int TRANSACTION_getPassword = IBinder.FIRST_CALL_TRANSACTION + 36; + + static final int TRANSACTION_clearPassword = IBinder.FIRST_CALL_TRANSACTION + 37; /** * Cast an IBinder object into an IMountService interface, generating a @@ -1208,6 +1242,19 @@ public interface IMountService extends IInterface { reply.writeInt(result); return true; } + case TRANSACTION_getPassword: { + data.enforceInterface(DESCRIPTOR); + String result = getPassword(); + reply.writeNoException(); + reply.writeString(result); + return true; + } + case TRANSACTION_clearPassword: { + data.enforceInterface(DESCRIPTOR); + clearPassword(); + reply.writeNoException(); + return true; + } } return super.onTransact(code, data, reply, flags); } @@ -1446,4 +1493,15 @@ public interface IMountService extends IInterface { * @return PasswordType */ public int getPasswordType() throws RemoteException; + + /** + * Get password from vold + * @return password or empty string + */ + public String getPassword() throws RemoteException; + + /** + * Securely clear password from vold + */ + public void clearPassword() throws RemoteException; } diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl index 91056f1609101..9501f92fc75f2 100644 --- a/core/java/com/android/internal/widget/ILockSettings.aidl +++ b/core/java/com/android/internal/widget/ILockSettings.aidl @@ -28,6 +28,7 @@ interface ILockSettings { boolean checkPattern(in String pattern, int userId); void setLockPassword(in String password, int userId); boolean checkPassword(in String password, int userId); + boolean checkVoldPassword(int userId); boolean havePattern(int userId); boolean havePassword(int userId); void removeUser(int userId); diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 2d7949140ca1f..e5aaf7ef714bc 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -312,6 +312,20 @@ public class LockPatternUtils { } } + /** + * Check to see if vold already has the password. + * Note that this also clears vold's copy of the password. + * @return Whether the vold password matches or not. + */ + public boolean checkVoldPassword() { + final int userId = getCurrentOrCallingUserId(); + try { + return getLockSettings().checkVoldPassword(userId); + } catch (RemoteException re) { + return false; + } + } + /** * Check to see if a password matches any of the passwords stored in the * password history. diff --git a/packages/Keyguard/src/com/android/keyguard/KeyguardViewMediator.java b/packages/Keyguard/src/com/android/keyguard/KeyguardViewMediator.java index 31e806c2a1d81..3fc562cb41102 100644 --- a/packages/Keyguard/src/com/android/keyguard/KeyguardViewMediator.java +++ b/packages/Keyguard/src/com/android/keyguard/KeyguardViewMediator.java @@ -979,6 +979,13 @@ public class KeyguardViewMediator { return; } + if (mLockPatternUtils.checkVoldPassword()) { + if (DEBUG) Log.d(TAG, "Not showing lock screen since just decrypted"); + // Without this, settings is not enabled until the lock screen first appears + hideLocked(); + return; + } + if (DEBUG) Log.d(TAG, "doKeyguard: showing the lock screen"); showLocked(options); } diff --git a/services/core/java/com/android/server/LockSettingsService.java b/services/core/java/com/android/server/LockSettingsService.java index fe814fc3be302..0d2cee831bca2 100644 --- a/services/core/java/com/android/server/LockSettingsService.java +++ b/services/core/java/com/android/server/LockSettingsService.java @@ -30,7 +30,11 @@ import android.database.sqlite.SQLiteOpenHelper; import android.database.sqlite.SQLiteStatement; import android.os.Binder; import android.os.Environment; +import android.os.IBinder; import android.os.RemoteException; +import android.os.storage.IMountService; +import android.os.ServiceManager; +import android.os.storage.StorageManager; import android.os.SystemProperties; import android.os.UserHandle; import android.os.UserManager; @@ -79,6 +83,7 @@ public class LockSettingsService extends ILockSettings.Stub { private final Context mContext; private LockPatternUtils mLockPatternUtils; + private boolean mFirstCallToVold; public LockSettingsService(Context context) { mContext = context; @@ -86,6 +91,7 @@ public class LockSettingsService extends ILockSettings.Stub { mOpenHelper = new DatabaseHelper(mContext); mLockPatternUtils = new LockPatternUtils(context); + mFirstCallToVold = true; } public void systemReady() { @@ -346,6 +352,51 @@ public class LockSettingsService extends ILockSettings.Stub { return true; } + @Override + public boolean checkVoldPassword(int userId) throws RemoteException { + if (!mFirstCallToVold) { + return false; + } + mFirstCallToVold = false; + + checkPasswordReadPermission(userId); + + // There's no guarantee that this will safely connect, but if it fails + // we will simply show the lock screen when we shouldn't, so relatively + // benign. There is an outside chance something nasty would happen if + // this service restarted before vold stales out the password in this + // case. The nastiness is limited to not showing the lock screen when + // we should, within the first minute of decrypting the phone if this + // service can't connect to vold, it restarts, and then the new instance + // does successfully connect. + final IMountService service = getMountService(); + String password = service.getPassword(); + service.clearPassword(); + if (password == null) { + return false; + } + + try { + if (mLockPatternUtils.isLockPatternEnabled()) { + if (checkPattern(password, userId)) { + return true; + } + } + } catch (Exception e) { + } + + try { + if (mLockPatternUtils.isLockPasswordEnabled()) { + if (checkPassword(password, userId)) { + return true; + } + } + } catch (Exception e) { + } + + return false; + } + @Override public void removeUser(int userId) { checkWritePermission(userId); @@ -524,4 +575,12 @@ public class LockSettingsService extends ILockSettings.Stub { Secure.LOCK_SCREEN_OWNER_INFO_ENABLED, Secure.LOCK_SCREEN_OWNER_INFO }; + + private IMountService getMountService() { + final IBinder service = ServiceManager.getService("mount"); + if (service != null) { + return IMountService.Stub.asInterface(service); + } + return null; + } } diff --git a/services/core/java/com/android/server/MountService.java b/services/core/java/com/android/server/MountService.java index cd74fed4d1b35..c1d9fbd3f15f9 100644 --- a/services/core/java/com/android/server/MountService.java +++ b/services/core/java/com/android/server/MountService.java @@ -74,6 +74,7 @@ import com.google.android.collect.Lists; import com.google.android.collect.Maps; import org.apache.commons.codec.binary.Hex; +import org.apache.commons.codec.DecoderException; import org.xmlpull.v1.XmlPullParserException; import java.io.File; @@ -573,6 +574,14 @@ class MountService extends IMountService.Stub } } + private boolean isReady() { + try { + return mConnectedSignal.await(0, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + return false; + } + } + private void handleSystemReady() { // Snapshot current volume states since it's not safe to call into vold // while holding locks. @@ -2075,12 +2084,25 @@ class MountService extends IMountService.Stub private String toHex(String password) { if (password == null) { - return null; + return new String(); } byte[] bytes = password.getBytes(StandardCharsets.UTF_8); return new String(Hex.encodeHex(bytes)); } + private String fromHex(String hexPassword) { + if (hexPassword == null) { + return null; + } + + try { + byte[] bytes = Hex.decodeHex(hexPassword.toCharArray()); + return new String(bytes, StandardCharsets.UTF_8); + } catch (DecoderException e) { + return null; + } + } + @Override public int decryptStorage(String password) { if (TextUtils.isEmpty(password)) { @@ -2229,6 +2251,35 @@ class MountService extends IMountService.Stub } } + @Override + public String getPassword() throws RemoteException { + if (!isReady()) { + return new String(); + } + + final NativeDaemonEvent event; + try { + event = mConnector.execute("cryptfs", "getpw"); + return fromHex(event.getMessage()); + } catch (NativeDaemonConnectorException e) { + throw e.rethrowAsParcelableException(); + } + } + + @Override + public void clearPassword() throws RemoteException { + if (!isReady()) { + return; + } + + final NativeDaemonEvent event; + try { + event = mConnector.execute("cryptfs", "clearpw"); + } catch (NativeDaemonConnectorException e) { + throw e.rethrowAsParcelableException(); + } + } + @Override public int mkdirs(String callingPkg, String appPath) { final int userId = UserHandle.getUserId(Binder.getCallingUid());