14.1: December ASB picks

Signed-off-by: Tad <tad@spotco.us>
This commit is contained in:
Tad 2023-12-07 00:07:00 -05:00
parent ee3e067016
commit d7996e8240
No known key found for this signature in database
GPG Key ID: B286E9F57A07424B
13 changed files with 774 additions and 1 deletions

View File

@ -0,0 +1,123 @@
From 0d5107c9fa5780919b18cd15c1a4a073ac6ed7cd Mon Sep 17 00:00:00 2001
From: Kweku Adams <kwekua@google.com>
Date: Fri, 23 Sep 2022 21:06:53 +0000
Subject: [PATCH] RESTRICT AUTOMERGE: Drop invalid data.
Drop invalid data when writing or reading from XML. PersistableBundle
does lazy unparcelling, so checking the values during unparcelling would
remove the benefit of the lazy unparcelling. Checking the validity when
writing to or reading from XML seems like the best alternative.
Bug: 246542285
Bug: 247513680
Test: install test app with invalid job config, start app to schedule job, then check logcat and jobscheduler persisted file
(cherry picked from commit 666e8ac60a31e2cc52b335b41004263f28a8db06)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:62b37ab21ce27746a79a2071deee98c61b23c8d9)
Merged-In: Ie817aa0993e9046cb313a750d2323cadc8c1ef15
Change-Id: Ie817aa0993e9046cb313a750d2323cadc8c1ef15
---
core/java/android/os/PersistableBundle.java | 42 +++++++++++++++++----
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java
index db9f4b14a345e..c3e4b759b103c 100644
--- a/core/java/android/os/PersistableBundle.java
+++ b/core/java/android/os/PersistableBundle.java
@@ -18,6 +18,7 @@
import android.annotation.Nullable;
import android.util.ArrayMap;
+import android.util.Slog;
import com.android.internal.util.XmlUtils;
@@ -36,6 +37,8 @@
*/
public final class PersistableBundle extends BaseBundle implements Cloneable, Parcelable,
XmlUtils.WriteMapCallback {
+ private static final String TAG = "PersistableBundle";
+
private static final String TAG_PERSISTABLEMAP = "pbundle_as_map";
public static final PersistableBundle EMPTY;
@@ -95,7 +98,11 @@ public PersistableBundle(PersistableBundle b) {
* @hide
*/
public PersistableBundle(Bundle b) {
- this(b.getMap());
+ this(b, true);
+ }
+
+ private PersistableBundle(Bundle b, boolean throwException) {
+ this(b.getMap(), throwException);
}
/**
@@ -104,7 +111,7 @@ public PersistableBundle(Bundle b) {
* @param map a Map containing only those items that can be persisted.
* @throws IllegalArgumentException if any element of #map cannot be persisted.
*/
- private PersistableBundle(ArrayMap<String, Object> map) {
+ private PersistableBundle(ArrayMap<String, Object> map, boolean throwException) {
super();
mFlags = FLAG_DEFUSABLE;
@@ -113,16 +120,23 @@ private PersistableBundle(ArrayMap<String, Object> map) {
// Now verify each item throwing an exception if there is a violation.
final int N = mMap.size();
- for (int i=0; i<N; i++) {
+ for (int i = N - 1; i >= 0; --i) {
Object value = mMap.valueAt(i);
if (value instanceof ArrayMap) {
// Fix up any Maps by replacing them with PersistableBundles.
- mMap.setValueAt(i, new PersistableBundle((ArrayMap<String, Object>) value));
+ mMap.setValueAt(i,
+ new PersistableBundle((ArrayMap<String, Object>) value, throwException));
} else if (value instanceof Bundle) {
- mMap.setValueAt(i, new PersistableBundle(((Bundle) value)));
+ mMap.setValueAt(i, new PersistableBundle((Bundle) value, throwException));
} else if (!isValidType(value)) {
- throw new IllegalArgumentException("Bad value in PersistableBundle key="
- + mMap.keyAt(i) + " value=" + value);
+ final String errorMsg = "Bad value in PersistableBundle key="
+ + mMap.keyAt(i) + " value=" + value;
+ if (throwException) {
+ throw new IllegalArgumentException(errorMsg);
+ } else {
+ Slog.wtfStack(TAG, errorMsg);
+ mMap.removeAt(i);
+ }
}
}
}
@@ -217,6 +231,15 @@ public void writeUnknownObject(Object v, String name, XmlSerializer out)
/** @hide */
public void saveToXml(XmlSerializer out) throws IOException, XmlPullParserException {
unparcel();
+ // Explicitly drop invalid types an attacker may have added before persisting.
+ for (int i = mMap.size() - 1; i >= 0; --i) {
+ final Object value = mMap.valueAt(i);
+ if (!isValidType(value)) {
+ Slog.e(TAG, "Dropping bad data before persisting: "
+ + mMap.keyAt(i) + "=" + value);
+ mMap.removeAt(i);
+ }
+ }
XmlUtils.writeMapXml(mMap, out, this);
}
@@ -265,9 +288,12 @@ public static PersistableBundle restoreFromXml(XmlPullParser in) throws IOExcept
while (((event = in.next()) != XmlPullParser.END_DOCUMENT) &&
(event != XmlPullParser.END_TAG || in.getDepth() < outerDepth)) {
if (event == XmlPullParser.START_TAG) {
+ // Don't throw an exception when restoring from XML since an attacker could try to
+ // input invalid data in the persisted file.
return new PersistableBundle((ArrayMap<String, Object>)
XmlUtils.readThisArrayMapXml(in, startTag, tagName,
- new MyReadMapCallback()));
+ new MyReadMapCallback()),
+ /* throwException */ false);
}
}
return EMPTY;

View File

@ -0,0 +1,29 @@
From a341bc7b7ae161f4a87e996d8a5e8b1ad005fb6b Mon Sep 17 00:00:00 2001
From: Pinyao Ting <pinyaoting@google.com>
Date: Mon, 24 Jul 2023 14:58:56 -0700
Subject: [PATCH] Validate userId when publishing shortcuts
Bug: 288110451
Test: manual
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:01bfd04ff445db6290ae430d44ea1bf1a115fe3c)
Merged-In: Idbde676f871db83825155730e3714f3727e25762
Change-Id: Idbde676f871db83825155730e3714f3727e25762
---
services/core/java/com/android/server/pm/ShortcutService.java | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java
index 944f75345df6f..2cfc3461c6977 100644
--- a/services/core/java/com/android/server/pm/ShortcutService.java
+++ b/services/core/java/com/android/server/pm/ShortcutService.java
@@ -1528,6 +1528,10 @@ private void verifyShortcutInfoPackage(String callerPackage, ShortcutInfo si) {
android.util.EventLog.writeEvent(0x534e4554, "109824443", -1, "");
throw new SecurityException("Shortcut package name mismatch");
}
+ final int callingUid = injectBinderCallingUid();
+ if (UserHandle.getUserId(callingUid) != si.getUserId()) {
+ throw new SecurityException("User-ID in shortcut doesn't match the caller");
+ }
}
private void verifyShortcutInfoPackages(

View File

@ -0,0 +1,33 @@
From cb95e01ba40c3b8c70f5810efb7abfd85bfd0b1f Mon Sep 17 00:00:00 2001
From: Kunal Malhotra <malhk@google.com>
Date: Thu, 2 Feb 2023 23:48:27 +0000
Subject: [PATCH] Adding in verification of calling UID in onShellCommand
Test: manual testing on device
Bug: b/261709193
(cherry picked from commit b651d295b44eb82d664861b77f33dbde1bce9453)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3ef3f18ba3094c4cc4f954ba23d1da421f9ca8b0)
Merged-In: I68903ebd6d3d85f4bc820b745e3233a448b62273
Change-Id: I68903ebd6d3d85f4bc820b745e3233a448b62273
---
.../java/com/android/server/am/ActivityManagerService.java | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 4e48f422a2fe3..7cda7571df70d 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -14349,6 +14349,13 @@ public int getMemoryTrimLevel() {
@Override
public void onShellCommand(FileDescriptor in, FileDescriptor out,
FileDescriptor err, String[] args, ResultReceiver resultReceiver) {
+ final int callingUid = Binder.getCallingUid();
+ if (callingUid != Process.ROOT_UID && callingUid != Process.SHELL_UID) {
+ if (resultReceiver != null) {
+ resultReceiver.send(-1, null);
+ }
+ throw new SecurityException("Shell commands are only callable by root or shell");
+ }
(new ActivityManagerShellCommand(this, false)).exec(
this, in, out, err, args, resultReceiver);
}

View File

@ -0,0 +1,38 @@
From fd90a5342af9b9ead4026a70a694d3f31908d5ea Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Thu, 5 Oct 2023 00:01:03 +0000
Subject: [PATCH] Fix UAF in ~CallbackEnv
com_android_bluetooth_btservice_AdapterService does not null its local
JNI environment variable after detaching the thread (which frees the
environment context), allowing UAF under certain conditions.
Null the variable in this case.
Testing here was done through a custom unit test; see patchsets 4-6 for
contents. However, unit testing of the JNI layer is problematic in
production, so that part of the patch is omitted for final merge.
Bug: 291500341
Test: atest bluetooth_test_gd_unit, atest net_test_stack_btm
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5f543d919c4067f2f4925580fd8a690ba3440e80)
Merged-In: I3e5e3c51412640aa19f0981caaa809313d6ad030
Change-Id: I3e5e3c51412640aa19f0981caaa809313d6ad030
---
jni/com_android_bluetooth_btservice_AdapterService.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/jni/com_android_bluetooth_btservice_AdapterService.cpp b/jni/com_android_bluetooth_btservice_AdapterService.cpp
index 1768ef51b..4a579e0ca 100644
--- a/jni/com_android_bluetooth_btservice_AdapterService.cpp
+++ b/jni/com_android_bluetooth_btservice_AdapterService.cpp
@@ -465,6 +465,7 @@ static void callback_thread_event(bt_cb_thread_evt event) {
return;
}
vm->DetachCurrentThread();
+ callbackEnv = NULL;
}
}

View File

@ -0,0 +1,35 @@
From e54bad92576d31fc959342e4c35d73abaf29d926 Mon Sep 17 00:00:00 2001
From: balakrishna <quic_kunthumu@quicinc.com>
Date: Tue, 7 Mar 2023 16:53:46 +0530
Subject: [PATCH] BT: Fixing the rfc_slot_id overflow
Root cause:
overflow causing leak in slot fds.
As slot id 0 not valid, we are not able to release these fds later.
Fix:
Changes are made to avoid overflow while allocate rfc slots.
CRs-Fixed: 3417458
Change-Id: I5d7efa34bfb97a6dd8e9d68615d29120a0ae51f0
---
btif/src/btif_sock_rfc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/btif/src/btif_sock_rfc.c b/btif/src/btif_sock_rfc.c
index d5522b739eb..d77c3f44253 100644
--- a/btif/src/btif_sock_rfc.c
+++ b/btif/src/btif_sock_rfc.c
@@ -225,8 +225,11 @@ static rfc_slot_t *alloc_rfc_slot(const bt_bdaddr_t *addr, const char *name, con
}
// Increment slot id and make sure we don't use id=0.
- if (++rfc_slot_id == 0)
+ if (UINT32_MAX == rfc_slot_id) {
rfc_slot_id = 1;
+ } else {
+ ++rfc_slot_id;
+ }
slot->fd = fds[0];
slot->app_fd = fds[1];

View File

@ -0,0 +1,31 @@
From e446be9bf42b2559add74d48b91bae52b828e24f Mon Sep 17 00:00:00 2001
From: balakrishna <quic_kunthumu@quicinc.com>
Date: Wed, 24 May 2023 13:28:21 +0530
Subject: [PATCH] Fix OOB Write in pin_reply in bluetooth.cc
Root cause:
if the length of "pin_code" is greater than 16,
an OOBW will be triggered due to a missing bounds check.
Fix:
Check is added to avoid Out of Bound Write.
CRs-Fixed: 3507292
Change-Id: I15a1eae59b17f633e29180a01676c260189b8353
---
btif/src/bluetooth.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/btif/src/bluetooth.c b/btif/src/bluetooth.c
index d2f81733da4..5fc6c880db0 100644
--- a/btif/src/bluetooth.c
+++ b/btif/src/bluetooth.c
@@ -345,6 +345,8 @@ static int pin_reply(const bt_bdaddr_t *bd_addr, uint8_t accept,
/* sanity check */
if (interface_ready() == FALSE)
return BT_STATUS_NOT_READY;
+ if (pin_code == NULL || pin_len > PIN_CODE_LEN)
+ return BT_STATUS_FAIL;
return btif_dm_pin_reply(bd_addr, accept, pin_len, pin_code);
}

View File

@ -0,0 +1,103 @@
From 277464e315d43f3d66e2444744dfa9e42977c64c Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Sat, 2 Sep 2023 04:20:10 +0000
Subject: [PATCH] Reject access to secure service authenticated from a temp
bonding [1]
Rejecct access to services running on l2cap
Backport of
Idef4ea28eb3d17b0807ab7dc6849433ddc5581b3
Bug: 294854926
Test: m com.android.btservices
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a36757e967ab6d956127cac298134f28ce8f0d6d)
Merged-In: Idef4ea28eb3d17b0807ab7dc6849433ddc5581b3
Change-Id: Idef4ea28eb3d17b0807ab7dc6849433ddc5581b3
---
stack/btm/btm_sec.c | 41 +++++++++++++++++++++++++++++++++++++----
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
index b27b7e071c7..23b334ce27f 100644
--- a/stack/btm/btm_sec.c
+++ b/stack/btm/btm_sec.c
@@ -106,7 +106,7 @@ static BOOLEAN btm_sec_set_security_level ( CONNECTION_TYPE conn_type, char *p_
UINT16 sec_level, UINT16 psm, UINT32 mx_proto_id,
UINT32 mx_chan_id);
-static BOOLEAN btm_dev_authenticated(tBTM_SEC_DEV_REC *p_dev_rec);
+static BOOLEAN btm_dev_authenticated(const tBTM_SEC_DEV_REC* p_dev_rec);
static BOOLEAN btm_dev_encrypted(tBTM_SEC_DEV_REC *p_dev_rec);
static BOOLEAN btm_dev_authorized(tBTM_SEC_DEV_REC *p_dev_rec);
static BOOLEAN btm_serv_trusted(tBTM_SEC_DEV_REC *p_dev_rec, tBTM_SEC_SERV_REC *p_serv_rec);
@@ -145,7 +145,7 @@ static const BOOLEAN btm_sec_io_map [BTM_IO_CAP_MAX][BTM_IO_CAP_MAX] =
** Returns BOOLEAN TRUE or FALSE
**
*******************************************************************************/
-static BOOLEAN btm_dev_authenticated (tBTM_SEC_DEV_REC *p_dev_rec)
+static BOOLEAN btm_dev_authenticated(const tBTM_SEC_DEV_REC* p_dev_rec)
{
if(p_dev_rec->sec_flags & BTM_SEC_AUTHENTICATED)
{
@@ -229,6 +229,26 @@ static BOOLEAN btm_serv_trusted(tBTM_SEC_DEV_REC *p_dev_rec, tBTM_SEC_SERV_REC *
return(FALSE);
}
+/*******************************************************************************
+**
+** Function access_secure_service_from_temp_bond
+**
+** Description a utility function to test whether an access to
+** secure service from temp bonding is happening
+**
+** Returns true if the aforementioned condition holds,
+** false otherwise
+**
+*******************************************************************************/
+static BOOLEAN access_secure_service_from_temp_bond(const tBTM_SEC_DEV_REC* p_dev_rec,
+ bool locally_initiated,
+ uint16_t security_req)
+{
+ return !locally_initiated && (security_req & BTM_SEC_IN_AUTHENTICATE) &&
+ btm_dev_authenticated(p_dev_rec) &&
+ p_dev_rec->bond_type == BOND_TYPE_TEMPORARY;
+}
+
/*******************************************************************************
**
** Function BTM_SecRegister
@@ -2215,10 +2235,15 @@ tBTM_STATUS btm_sec_l2cap_access_req (BD_ADDR bd_addr, UINT16 psm, UINT16 handle
if (rc == BTM_SUCCESS)
{
+ if (access_secure_service_from_temp_bond(p_dev_rec, is_originator, security_required))
+ {
+ LOG_ERROR(LOG_TAG, "Trying to access a secure service from a temp bonding, rejecting");
+ rc = BTM_FAILED_ON_SECURITY;
+ }
if (p_callback)
- (*p_callback) (bd_addr, transport, (void *)p_ref_data, BTM_SUCCESS);
+ (*p_callback)(&bd_addr, transport, (void*)p_ref_data, rc);
- return(BTM_SUCCESS);
+ return (rc);
}
}
else
@@ -5587,6 +5612,14 @@ extern tBTM_STATUS btm_sec_execute_procedure (tBTM_SEC_DEV_REC *p_dev_rec)
}
}
+ if (access_secure_service_from_temp_bond(p_dev_rec,
+ p_dev_rec->is_originator,
+ p_dev_rec->security_required))
+ {
+ LOG_ERROR(LOG_TAG, "Trying to access a secure service from a temp bonding, rejecting");
+ return (BTM_FAILED_ON_SECURITY);
+ }
+
/* All required security procedures already established */
p_dev_rec->security_required &= ~(BTM_SEC_OUT_AUTHORIZE | BTM_SEC_IN_AUTHORIZE |
BTM_SEC_OUT_AUTHENTICATE | BTM_SEC_IN_AUTHENTICATE |

View File

@ -0,0 +1,40 @@
From 477c17c9d164ea056d9e32e215b8d54d757c9374 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Sat, 2 Sep 2023 04:27:29 +0000
Subject: [PATCH] Reject access to secure services authenticated from temp
bonding [2]
Reject access to service running on rfcomm
this is a backport of
I10fcc2dcd78fc22ffbe3c425669fc9889b94a166
Bug: 294854926
Test: m com.android.btservices
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5e0e907ec4948f06b3a35ecf08725c020d533ccb)
Merged-In: I10fcc2dcd78fc22ffbe3c425669fc9889b94a166
Change-Id: I10fcc2dcd78fc22ffbe3c425669fc9889b94a166
---
stack/btm/btm_sec.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
index 23b334ce27..21af82f8eb 100644
--- a/stack/btm/btm_sec.c
+++ b/stack/btm/btm_sec.c
@@ -2648,8 +2648,13 @@ tBTM_STATUS btm_sec_mx_access_request (BD_ADDR bd_addr, UINT16 psm, BOOLEAN is_o
}
else /* rc == BTM_SUCCESS */
{
+ if (access_secure_service_from_temp_bond(p_dev_rec,
+ is_originator, security_required)) {
+ LOG_ERROR(LOG_TAG, "Trying to access a secure rfcomm service from a temp bonding, reject");
+ rc = BTM_FAILED_ON_SECURITY;
+ }
/* access granted */
- if (p_callback)
+ if (p_callback)
{
(*p_callback) (bd_addr, transport, p_ref_data, (UINT8)rc);
}

View File

@ -0,0 +1,47 @@
From b04b44ae7eacd4fab4c8308ce15c2186f254180e Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Tue, 12 Sep 2023 23:47:48 +0000
Subject: [PATCH] Reject access to secure service authenticated from a temp
bonding [3]
Allow access to rfcomm PSM by default
Original bug
Bug: 294854926
Nearby regressions:
Bug: 298539299
Test: m com.android.btservices
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ab986fe4165aae74c5915f57ad2e78bf80f1d3ec)
Merged-In: If1f7c9278a9e877f64ae78b6f067c597fb5d0e66
Change-Id: If1f7c9278a9e877f64ae78b6f067c597fb5d0e66
---
stack/btm/btm_sec.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
index 21af82f8eb..ab97de8564 100644
--- a/stack/btm/btm_sec.c
+++ b/stack/btm/btm_sec.c
@@ -2356,15 +2356,15 @@ tBTM_STATUS btm_sec_l2cap_access_req (BD_ADDR bd_addr, UINT16 psm, UINT16 handle
{
if (is_originator)
{
- /* SM4 to SM4 -> always authenticate & encrypt */
- security_required |= (BTM_SEC_OUT_AUTHENTICATE | BTM_SEC_OUT_ENCRYPT);
+ /* SM4 to SM4 -> always encrypt */
+ security_required |= BTM_SEC_OUT_ENCRYPT;
}
else /* acceptor */
{
/* SM4 to SM4: the acceptor needs to make sure the authentication is already done */
chk_acp_auth_done = TRUE;
- /* SM4 to SM4 -> always authenticate & encrypt */
- security_required |= (BTM_SEC_IN_AUTHENTICATE | BTM_SEC_IN_ENCRYPT);
+ /* SM4 to SM4 -> always encrypt */
+ security_required |= BTM_SEC_IN_ENCRYPT;
}
}
else if (!(BTM_SM4_KNOWN & p_dev_rec->sm4))

View File

@ -0,0 +1,145 @@
From bf351819da52628f46d4fe11d7e58d4460e7d546 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Tue, 12 Sep 2023 23:54:08 +0000
Subject: [PATCH] Reorganize the code for checking auth requirement
Original bug
Bug: 294854926
regressions:
Bug: 299570702
Test: Test: m com.android.btservices
Test: QA validation
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0c488b2420befe0f8038957861072a8e63702f91)
Merged-In: I976a5a6d7bb819fd6accdc71eb1501b9606f3ae4
Change-Id: I976a5a6d7bb819fd6accdc71eb1501b9606f3ae4
---
stack/btm/btm_sec.c | 99 ++++++++++++++++++++++++++++++---------------
1 file changed, 66 insertions(+), 33 deletions(-)
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
index ab97de8564..39b9486f41 100644
--- a/stack/btm/btm_sec.c
+++ b/stack/btm/btm_sec.c
@@ -5519,53 +5519,86 @@ extern tBTM_STATUS btm_sec_execute_procedure (tBTM_SEC_DEV_REC *p_dev_rec)
/* If connection is not authenticated and authentication is required */
/* start authentication and return PENDING to the caller */
- if ((((!(p_dev_rec->sec_flags & BTM_SEC_AUTHENTICATED))
- && (( p_dev_rec->is_originator && (p_dev_rec->security_required & BTM_SEC_OUT_AUTHENTICATE))
- || (!p_dev_rec->is_originator && (p_dev_rec->security_required & BTM_SEC_IN_AUTHENTICATE))))
- || (!(p_dev_rec->sec_flags & BTM_SEC_16_DIGIT_PIN_AUTHED)
- && (!p_dev_rec->is_originator
- && (p_dev_rec->security_required & BTM_SEC_IN_MIN_16_DIGIT_PIN))))
- && (p_dev_rec->hci_handle != BTM_SEC_INVALID_HANDLE))
+ if (p_dev_rec->hci_handle != HCI_INVALID_HANDLE)
{
+ bool start_auth = false;
+
+ // Check link status of BR/EDR
+ if (!(p_dev_rec->sec_flags & BTM_SEC_AUTHENTICATED))
+ {
+ if (p_dev_rec->is_originator)
+ {
+ if (p_dev_rec->security_required & BTM_SEC_OUT_AUTHENTICATE)
+ {
+ LOG_DEBUG(LOG_TAG, "Outgoing authentication Required");
+ start_auth = true;
+ }
+ }
+ else
+ {
+ if (p_dev_rec->security_required & BTM_SEC_IN_AUTHENTICATE)
+ {
+ LOG_DEBUG(LOG_TAG, "Incoming authentication Required");
+ start_auth = true;
+ }
+ }
+ }
+
+ if (!(p_dev_rec->sec_flags & BTM_SEC_16_DIGIT_PIN_AUTHED))
+ {
/*
* We rely on BTM_SEC_16_DIGIT_PIN_AUTHED being set if MITM is in use,
* as 16 DIGIT is only needed if MITM is not used. Unfortunately, the
* BTM_SEC_AUTHENTICATED is used for both MITM and non-MITM
* authenticated connections, hence we cannot distinguish here.
*/
+ if (!p_dev_rec->is_originator)
+ {
+ if (p_dev_rec->security_required & BTM_SEC_IN_MIN_16_DIGIT_PIN)
+ {
+ LOG_DEBUG(LOG_TAG, "BTM_SEC_IN_MIN_16_DIGIT_PIN Required");
+ start_auth = true;
+ }
+ }
+ }
+
+ if (start_auth) {
#if (L2CAP_UCD_INCLUDED == TRUE)
- /* if incoming UCD packet, discard it */
- if ( !p_dev_rec->is_originator && (p_dev_rec->is_ucd == TRUE ))
- return(BTM_FAILED_ON_SECURITY);
+ /* if incoming UCD packet, discard it */
+ if ( !p_dev_rec->is_originator && (p_dev_rec->is_ucd == TRUE ))
+ return(BTM_FAILED_ON_SECURITY);
#endif
- BTM_TRACE_EVENT ("Security Manager: Start authentication");
+ LOG_DEBUG(LOG_TAG, "Security Manager: Start authentication");
- /*
- * If we do have a link-key, but we end up here because we need an
- * upgrade, then clear the link-key known and authenticated flag before
- * restarting authentication.
- * WARNING: If the controller has link-key, it is optional and
- * recommended for the controller to send a Link_Key_Request.
- * In case we need an upgrade, the only alternative would be to delete
- * the existing link-key. That could lead to very bad user experience
- * or even IOP issues, if a reconnect causes a new connection that
- * requires an upgrade.
- */
- if ((p_dev_rec->sec_flags & BTM_SEC_LINK_KEY_KNOWN)
- && (!(p_dev_rec->sec_flags & BTM_SEC_16_DIGIT_PIN_AUTHED)
- && (!p_dev_rec->is_originator && (p_dev_rec->security_required
- & BTM_SEC_IN_MIN_16_DIGIT_PIN)))) {
- p_dev_rec->sec_flags &= ~(BTM_SEC_LINK_KEY_KNOWN | BTM_SEC_LINK_KEY_AUTHED
- | BTM_SEC_AUTHENTICATED);
- }
+ /*
+ * If we do have a link-key, but we end up here because we need an
+ * upgrade, then clear the link-key known and authenticated flag before
+ * restarting authentication.
+ * WARNING: If the controller has link-key, it is optional and
+ * recommended for the controller to send a Link_Key_Request.
+ * In case we need an upgrade, the only alternative would be to delete
+ * the existing link-key. That could lead to very bad user experience
+ * or even IOP issues, if a reconnect causes a new connection that
+ * requires an upgrade.
+ */
+ if ((p_dev_rec->sec_flags & BTM_SEC_LINK_KEY_KNOWN) &&
+ (!(p_dev_rec->sec_flags & BTM_SEC_16_DIGIT_PIN_AUTHED) &&
+ (!p_dev_rec->is_originator &&
+ (p_dev_rec->security_required & BTM_SEC_IN_MIN_16_DIGIT_PIN))))
+ {
+ p_dev_rec->sec_flags &=
+ ~(BTM_SEC_LINK_KEY_KNOWN | BTM_SEC_LINK_KEY_AUTHED |
+ BTM_SEC_AUTHENTICATED);
+ }
- if (!btm_sec_start_authentication (p_dev_rec))
- {
- return(BTM_NO_RESOURCES);
+ if (!btm_sec_start_authentication (p_dev_rec))
+ {
+ return(BTM_NO_RESOURCES);
+ }
+ return(BTM_CMD_STARTED);
}
- return(BTM_CMD_STARTED);
}
/* If connection is not encrypted and encryption is required */

View File

@ -0,0 +1,50 @@
From eb5838d3c34231efb1ab700d91b596247f4a4898 Mon Sep 17 00:00:00 2001
From: Hui Peng <phui@google.com>
Date: Wed, 13 Sep 2023 00:00:44 +0000
Subject: [PATCH] Enforce authentication if encryption is required
Original bug
Bug: 294854926
regressions:
Bug: 299570702
Bug: 299561281
Test: Test: m com.android.btservices
Test: QA validation
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:778d3fb3fb520e54425ecefe9a28453002053553)
Merged-In: I0370ed2e3166d56f708e1981c2126526e1db9eaa
Change-Id: I0370ed2e3166d56f708e1981c2126526e1db9eaa
---
stack/btm/btm_sec.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/stack/btm/btm_sec.c b/stack/btm/btm_sec.c
index 39b9486f41..a43fdcafeb 100644
--- a/stack/btm/btm_sec.c
+++ b/stack/btm/btm_sec.c
@@ -5528,17 +5528,19 @@ extern tBTM_STATUS btm_sec_execute_procedure (tBTM_SEC_DEV_REC *p_dev_rec)
{
if (p_dev_rec->is_originator)
{
- if (p_dev_rec->security_required & BTM_SEC_OUT_AUTHENTICATE)
+ if (p_dev_rec->security_required &
+ (BTM_SEC_OUT_AUTHENTICATE | BTM_SEC_OUT_ENCRYPT))
{
- LOG_DEBUG(LOG_TAG, "Outgoing authentication Required");
+ LOG_DEBUG(LOG_TAG, "Outgoing authentication/encryption Required");
start_auth = true;
}
}
else
{
- if (p_dev_rec->security_required & BTM_SEC_IN_AUTHENTICATE)
+ if (p_dev_rec->security_required &
+ (BTM_SEC_IN_AUTHENTICATE | BTM_SEC_IN_ENCRYPT))
{
- LOG_DEBUG(LOG_TAG, "Incoming authentication Required");
+ LOG_DEBUG(LOG_TAG, "Incoming authentication/encryption Required");
start_auth = true;
}
}

View File

@ -0,0 +1,87 @@
From 892310be92b62dc2bd3447546dd385534ce8a695 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Tue, 3 Oct 2023 21:27:49 +0000
Subject: [PATCH] Fix timing attack in BTM_BleVerifySignature
BTM_BleVerifySignature uses a stock memcmp, allowing signature contents
to be deduced through a side-channel attack.
Change to CRYPTO_memcmp, which is hardened against this attack, to
eliminate this attack.
Bug: 274478807
Test: atest bluetooth_test_gd_unit
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:fcd1c44f7c4bf431dd6a6902d74c045174bd00ce)
Merged-In: I41a9b586d663d2ad4694222ae451d2d30a428a3c
Change-Id: I41a9b586d663d2ad4694222ae451d2d30a428a3c
---
main/Android.mk | 2 ++
stack/Android.mk | 5 +++--
stack/btm/btm_ble.c | 3 ++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/main/Android.mk b/main/Android.mk
index 2286997e8d3..985851add33 100644
--- a/main/Android.mk
+++ b/main/Android.mk
@@ -52,12 +52,14 @@ LOCAL_C_INCLUDES+= . \
$(LOCAL_PATH)/../audio_a2dp_hw \
$(LOCAL_PATH)/../utils/include \
$(bluetooth_C_INCLUDES) \
+ external/boringssl \
external/tinyxml2 \
external/zlib \
$(call include-path-for, audio-utils)
LOCAL_SHARED_LIBRARIES := \
libcutils \
+ libcrypto \
libdl \
liblog \
libz \
diff --git a/stack/Android.mk b/stack/Android.mk
index 4c77e8dd7f4..49f43fbc5df 100644
--- a/stack/Android.mk
+++ b/stack/Android.mk
@@ -33,7 +33,8 @@ LOCAL_C_INCLUDES:= \
$(LOCAL_PATH)/../bta/sys \
$(LOCAL_PATH)/../utils/include \
$(LOCAL_PATH)/../ \
- $(bluetooth_C_INCLUDES)
+ $(bluetooth_C_INCLUDES) \
+ external/boringssl
LOCAL_SRC_FILES:= \
./a2dp/a2d_api.c \
@@ -154,7 +155,7 @@ LOCAL_SRC_FILES:= \
LOCAL_MODULE := libbt-stack
LOCAL_STATIC_LIBRARIES := libbt-hci
-LOCAL_SHARED_LIBRARIES := libcutils
+LOCAL_SHARED_LIBRARIES := libcutils libcrypto
LOCAL_CFLAGS += $(bluetooth_CFLAGS)
diff --git a/stack/btm/btm_ble.c b/stack/btm/btm_ble.c
index 51fd748c074..9e248e96580 100644
--- a/stack/btm/btm_ble.c
+++ b/stack/btm/btm_ble.c
@@ -41,6 +41,7 @@
#include "hcimsgs.h"
#include "log/log.h"
#include "l2c_int.h"
+#include "openssl/mem.h"
#include "osi/include/log.h"
#include "smp_api.h"
@@ -2280,7 +2281,7 @@ BOOLEAN BTM_BleVerifySignature (BD_ADDR bd_addr, UINT8 *p_orig, UINT16 len, UINT
if (aes_cipher_msg_auth_code(p_rec->ble.keys.pcsrk, p_orig, len, BTM_CMAC_TLEN_SIZE, p_mac))
{
- if (memcmp(p_mac, p_comp, BTM_CMAC_TLEN_SIZE) == 0)
+ if (CRYPTO_memcmp(p_mac, p_comp, BTM_CMAC_TLEN_SIZE) == 0)
{
btm_ble_increment_sign_ctr(bd_addr, FALSE);
verified = TRUE;

View File

@ -76,7 +76,7 @@ sed -i '50i$(my_res_package): PRIVATE_AAPT_FLAGS += --auto-add-overlay' core/aap
sed -i '296iLOCAL_AAPT_FLAGS += --auto-add-overlay' core/package_internal.mk;
awk -i inplace '!/Email/' target/product/core.mk; #Remove Email
awk -i inplace '!/Exchange2/' target/product/core.mk;
sed -i 's/2021-06-05/2023-11-05/' core/version_defaults.mk; #Bump Security String #n-asb-2023-11 #XXX
sed -i 's/2021-06-05/2023-12-05/' core/version_defaults.mk; #Bump Security String #n-asb-2023-12 #XXX
fi;
if enterAndClear "device/qcom/sepolicy"; then
@ -239,6 +239,9 @@ applyPatch "$DOS_PATCHES/android_frameworks_base/367637.patch"; #n-asb-2023-10 D
applyPatch "$DOS_PATCHES/android_frameworks_base/367638.patch"; #n-asb-2023-10 Fix KCM key mapping cloning
applyPatch "$DOS_PATCHES/android_frameworks_base/373033.patch"; #n-asb-2023-11 [SettingsProvider] verify ringtone URI before setting
applyPatch "$DOS_PATCHES/android_frameworks_base/373034.patch"; #n-asb-2023-11 Use type safe API of readParcelableArray
applyPatch "$DOS_PATCHES/android_frameworks_base/376458.patch"; #n-asb-2023-12 Drop invalid data.
applyPatch "$DOS_PATCHES/android_frameworks_base/376459.patch"; #n-asb-2023-12 Validate userId when publishing shortcuts
applyPatch "$DOS_PATCHES/android_frameworks_base/376460.patch"; #n-asb-2023-12 Adding in verification of calling UID in onShellCommand
git revert --no-edit 0326bb5e41219cf502727c3aa44ebf2daa19a5b3; #Re-enable doze on devices without gms
applyPatch "$DOS_PATCHES/android_frameworks_base/248599.patch"; #Make SET_TIME_ZONE permission match SET_TIME (AOSP)
applyPatch "$DOS_PATCHES/android_frameworks_base/0001-Reduced_Resolution.patch"; #Allow reducing resolution to save power TODO: Add 800x480 (DivestOS)
@ -358,6 +361,7 @@ applyPatch "$DOS_PATCHES/android_packages_apps_Bluetooth/332451.patch"; #n-asb-2
applyPatch "$DOS_PATCHES/android_packages_apps_Bluetooth/332452.patch"; #n-asb-2022-06 Removes app access to BluetoothAdapter#setDiscoverableTimeout by requiring BLUETOOTH_PRIVILEGED permission.
applyPatch "$DOS_PATCHES/android_packages_apps_Bluetooth/345525.patch"; #n-asb-2022-12 Fix URI check in BluetoothOppUtility.java
applyPatch "$DOS_PATCHES/android_packages_apps_Bluetooth/348652.patch"; #n-asb-2023-02 Fix OPP comparison
applyPatch "$DOS_PATCHES/android_packages_apps_Bluetooth/376469.patch"; #n-asb-2023-12 Fix UAF in ~CallbackEnv
fi;
if enterAndClear "packages/apps/Contacts"; then
@ -518,6 +522,14 @@ applyPatch "$DOS_PATCHES/android_system_bt/365694.patch"; #n-asb-2023-09 Fix int
applyPatch "$DOS_PATCHES/android_system_bt/365695.patch"; #n-asb-2023-09 Fix reliable write.
applyPatch "$DOS_PATCHES/android_system_bt/365696.patch"; #n-asb-2023-09 Fix UAF in gatt_cl.cc
applyPatch "$DOS_PATCHES/android_system_bt/365697.patch"; #n-asb-2023-09 Fix an integer overflow bug in avdt_msg_asmbl
applyPatch "$DOS_PATCHES/android_system_bt/376461.patch"; #n-asb-2023-12 BT: Fixing the rfc_slot_id overflow
applyPatch "$DOS_PATCHES/android_system_bt/376462.patch"; #n-asb-2023-12 Fix OOB Write in pin_reply in bluetooth.cc
applyPatch "$DOS_PATCHES/android_system_bt/376463.patch"; #n-asb-2023-12 Reject access to secure service authenticated from a temp bonding [1]
applyPatch "$DOS_PATCHES/android_system_bt/376464.patch"; #n-asb-2023-12 Reject access to secure services authenticated from temp bonding [2]
applyPatch "$DOS_PATCHES/android_system_bt/376465.patch"; #n-asb-2023-12 Reject access to secure service authenticated from a temp bonding [3]
applyPatch "$DOS_PATCHES/android_system_bt/376466.patch"; #n-asb-2023-12 Reorganize the code for checking auth requirement
applyPatch "$DOS_PATCHES/android_system_bt/376467.patch"; #n-asb-2023-12 Enforce authentication if encryption is required
applyPatch "$DOS_PATCHES/android_system_bt/376468.patch"; #n-asb-2023-12 Fix timing attack in BTM_BleVerifySignature
applyPatch "$DOS_PATCHES/android_system_bt/229574.patch"; #bt-sbc-hd-dualchannel-nougat: Increase maximum Bluetooth SBC codec bitrate for SBC HD (ValdikSS)
applyPatch "$DOS_PATCHES/android_system_bt/229575.patch"; #bt-sbc-hd-dualchannel-nougat: Explicit SBC Dual Channel (SBC HD) support (ValdikSS)
applyPatch "$DOS_PATCHES/android_system_bt/242134.patch"; #avrc_bld_get_attrs_rsp - fix attribute length position off by one (cprhokie)