Restoring bond clarification

Author: philips77

dfu/src/main/java/no/nordicsemi/android/dfu/BaseCustomDfuImpl.java

@@ -464,7 +464,8 @@ void finalize(final Intent intent, final boolean forceRefresh) {
 
 			if (restoreBond && (mFileType & DfuBaseService.TYPE_APPLICATION) > 0) {
 				// Restore pairing when application was updated.
-				createBond();
+				if (!createBond())
+					logw("Creating bond failed");
 				alreadyWaited = false;
 			}
 		}

dfu/src/main/java/no/nordicsemi/android/dfu/BaseDfuImpl.java

@@ -447,7 +447,10 @@ void enableCCCD(@NonNull final BluetoothGattCharacteristic characteristic, final
 				while ((!cccdEnabled && mConnected && mError == 0) || mPaused) {
 					mLock.wait();
 					// Check the value of the CCCD
-					cccdEnabled = descriptor.getValue() != null && descriptor.getValue().length == 2 && descriptor.getValue()[0] > 0 && descriptor.getValue()[1] == 0;
+					cccdEnabled = descriptor.getValue() != null
+							   && descriptor.getValue().length == 2
+							   && descriptor.getValue()[0] > 0
+							   && descriptor.getValue()[1] == 0;
 				}
 			}
 		} catch (final InterruptedException e) {
@@ -570,14 +573,13 @@ void writeOpCode(@NonNull final BluetoothGattCharacteristic characteristic, @Non
 
 	/**
 	 * Creates bond to the device. Works on all APIs since 18th (Android 4.3).
+	 * This method will only be called in this library after bond information was removed.
 	 *
-	 * @return true if it's already bonded or the bonding has started
+	 * @return true if bonding has started, false otherwise.
 	 */
 	@SuppressWarnings("UnusedReturnValue")
 	boolean createBond() {
 		final BluetoothDevice device = mGatt.getDevice();
-		if (device.getBondState() == BluetoothDevice.BOND_BONDED)
-			return true;
 
 		boolean result;
 		mRequestCompleted = false;
@@ -593,7 +595,7 @@ boolean createBond() {
 		// We have to wait until device is bounded
 		try {
 			synchronized (mLock) {
-				while (!mRequestCompleted && !mAborted)
+				while (result && !mRequestCompleted && !mAborted)
 					mLock.wait();
 			}
 		} catch (final InterruptedException e) {
@@ -610,12 +612,14 @@ boolean createBond() {
 	 */
 	private boolean createBondApi18(@NonNull final BluetoothDevice device) {
 		/*
-		 * There is a createBond() method in BluetoothDevice class but for now it's hidden. We will call it using reflections. It has been revealed in KitKat (Api19)
+		 * There is a createBond() method in BluetoothDevice class but for now it's hidden.
+		 * We will call it using reflections. It has been revealed in KitKat (Api19)
 		 */
 		try {
 			final Method createBond = device.getClass().getMethod("createBond");
             mService.sendLogBroadcast(DfuBaseService.LOG_LEVEL_DEBUG, "gatt.getDevice().createBond() (hidden)");
-            return (Boolean) createBond.invoke(device);
+			//noinspection ConstantConditions
+			return (Boolean) createBond.invoke(device);
 		} catch (final Exception e) {
 			Log.w(TAG, "An exception occurred while creating bond", e);
 		}
@@ -636,14 +640,16 @@ boolean removeBond() {
 		mService.sendLogBroadcast(DfuBaseService.LOG_LEVEL_VERBOSE, "Removing bond information...");
 		boolean result = false;
 		/*
-		 * There is a removeBond() method in BluetoothDevice class but for now it's hidden. We will call it using reflections.
+		 * There is a removeBond() method in BluetoothDevice class but for now it's hidden.
+		 * We will call it using reflections.
 		 */
 		try {
             //noinspection JavaReflectionMemberAccess
             final Method removeBond = device.getClass().getMethod("removeBond");
             mRequestCompleted = false;
             mService.sendLogBroadcast(DfuBaseService.LOG_LEVEL_DEBUG, "gatt.getDevice().removeBond() (hidden)");
-            result = (Boolean) removeBond.invoke(device);
+			//noinspection ConstantConditions
+			result = (Boolean) removeBond.invoke(device);
 
             // We have to wait until device is unbounded
             try {

dfu/src/main/java/no/nordicsemi/android/dfu/DfuBaseService.java

@@ -233,20 +233,20 @@ public abstract class DfuBaseService extends IntentService implements DfuProgres
 	 */
 	public static final String EXTRA_CURRENT_MTU = "no.nordicsemi.android.dfu.extra.EXTRA_CURRENT_MTU";
 	/**
-	 * Set this flag to true to enable experimental buttonless feature in Secure DFU. When the
-	 * experimental Buttonless DFU Service is found on a device, the service will use it to
+	 * Set this flag to true to enable experimental buttonless feature in Secure DFU from SDK 12.
+	 * When the experimental Buttonless DFU Service is found on a device, the service will use it to
 	 * switch the device to the bootloader mode, connect to it in that mode and proceed with DFU.
 	 * <p>
 	 * <b>Please, read the information below before setting it to true.</b>
 	 * <p>
 	 * In the SDK 12.x the Buttonless DFU feature for Secure DFU was experimental.
 	 * It is NOT recommended to use it: it was not properly tested, had implementation bugs
 	 * (e.g. https://devzone.nordicsemi.com/question/100609/sdk-12-bootloader-erased-after-programming/)
-	 * and does not required encryption and therefore may lead to DOS attack (anyone can use it
+	 * and does not require encryption and therefore may lead to DOS attack (anyone can use it
 	 * to switch the device to bootloader mode). However, as there is no other way to trigger
 	 * bootloader mode on devices without a button, this DFU Library supports this service,
 	 * but the feature must be explicitly enabled here.
-	 * Be aware, that setting this flag to false will no protect your devices from this kind of
+	 * Be aware, that setting this flag to false will not protect your devices from this kind of
 	 * attacks, as an attacker may use another app for that purpose. To be sure your device is
 	 * secure remove this experimental service from your device.
 	 * <p>
@@ -260,9 +260,7 @@ public abstract class DfuBaseService extends IntentService implements DfuProgres
 	 * 0x01 - Success<br>
 	 * The device should disconnect and restart in DFU mode after sending the notification.
 	 * <p>
-	 * In SDK 13 this issue will be fixed by a proper implementation (bonding required,
-	 * passing bond information to the bootloader, encryption, well tested).
-	 * It is recommended to use this new service when SDK 13 (or later) is out.
+	 * In SDK 14 this issue was fixed by Buttonless Service With Bonds.
 	 */
 	public static final String EXTRA_UNSAFE_EXPERIMENTAL_BUTTONLESS_DFU = "no.nordicsemi.android.dfu.extra.EXTRA_UNSAFE_EXPERIMENTAL_BUTTONLESS_DFU";
 	/**
@@ -877,16 +875,21 @@ public void onConnectionStateChange(final BluetoothGatt gatt, final int status,
 					mConnectionState = STATE_CONNECTED;
 
 					/*
-					 *  The onConnectionStateChange callback is called just after establishing connection and before sending Encryption Request BLE event in case of a paired device.
-					 *  In that case and when the Service Changed CCCD is enabled we will get the indication after initializing the encryption, about 1600 milliseconds later.
-					 *  If we discover services right after connecting, the onServicesDiscovered callback will be called immediately, before receiving the indication and the following
-					 *  service discovery and we may end up with old, application's services instead.
+					 * The onConnectionStateChange callback is called just after establishing connection and before sending Encryption Request BLE event in case of a paired device.
+					 * In that case and when the Service Changed CCCD is enabled we will get the indication after initializing the encryption, about 1600 milliseconds later.
+					 * If we discover services right after connecting, the onServicesDiscovered callback will be called immediately, before receiving the indication and the following
+					 * service discovery and we may end up with old, application's services instead.
 					 *
-					 *  This is to support the buttonless switch from application to bootloader mode where the DFU bootloader notifies the master about service change.
-					 *  Tested on Nexus 4 (Android 4.4.4 and 5), Nexus 5 (Android 5), Samsung Note 2 (Android 4.4.2). The time after connection to end of service discovery is about 1.6s
-					 *  on Samsung Note 2.
+					 * This is to support the buttonless switch from application to bootloader mode where the DFU bootloader notifies the master about service change.
+					 * Tested on Nexus 4 (Android 4.4.4 and 5), Nexus 5 (Android 5), Samsung Note 2 (Android 4.4.2). The time after connection to end of service discovery is about 1.6s
+					 * on Samsung Note 2.
 					 *
-					 *  NOTE: We are doing this to avoid the hack with calling the hidden gatt.refresh() method, at least for bonded devices.
+					 * NOTE: We are doing this to avoid the hack with calling the hidden gatt.refresh()
+					 * method, at least for bonded devices.
+					 *
+					 * IMPORTANT: BluetoothDevice.getBondState() returns true if the bond information
+					 * is present on Android, not necessarily when the link is established or even
+					 * encrypted. This is a security issue, but in here it does not matter.
 					 */
 					if (gatt.getDevice().getBondState() == BluetoothDevice.BOND_BONDED) {
 						logi("Waiting 1600 ms for a possible Service Changed indication...");
@@ -1627,19 +1630,23 @@ protected void close(@NonNull final BluetoothGatt gatt) {
 	 */
 	protected void refreshDeviceCache(@NonNull final BluetoothGatt gatt, final boolean force) {
 		/*
-		 * If the device is bonded this is up to the Service Changed characteristic to notify Android that the services has changed.
-		 * There is no need for this trick in that case.
-		 * If not bonded, the Android should not keep the services cached when the Service Changed characteristic is present in the target device database.
-		 * However, due to the Android bug (still exists in Android 5.0.1), it is keeping them anyway and the only way to clear services is by using this hidden refresh method.
+		 * If the device is bonded this is up to the Service Changed characteristic to notify Android
+		 * that the services has changed. There is no need for this trick in that case.
+		 * If not bonded, the Android should not keep the services cached when the Service Changed
+		 * characteristic is present in the target device database.
+		 * However, due to the Android bug, it is keeping them anyway and the only way to clear
+		 * services is by using this hidden refresh method.
 		 */
 		if (force || gatt.getDevice().getBondState() == BluetoothDevice.BOND_NONE) {
 			sendLogBroadcast(LOG_LEVEL_DEBUG, "gatt.refresh() (hidden)");
 			/*
-			 * There is a refresh() method in BluetoothGatt class but for now it's hidden. We will call it using reflections.
+			 * There is a refresh() method in BluetoothGatt class but for now it's hidden.
+			 * We will call it using reflections.
 			 */
 			try {
 				//noinspection JavaReflectionMemberAccess
 				final Method refresh = gatt.getClass().getMethod("refresh");
+				//noinspection ConstantConditions
 				final boolean success = (Boolean) refresh.invoke(gatt);
 				logi("Refreshing result: " + success);
 			} catch (final Exception e) {