diff mbox series

Bluetooth: smp: Fix biased random passkey generation

Message ID 20201207174957.408-1-encrypted.def@gmail.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series Bluetooth: smp: Fix biased random passkey generation | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 0 this patch: 2
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 10 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Mincheol Son Dec. 7, 2020, 5:49 p.m. UTC 
Since u32 range size is not a multiple of 1,000,000, current passkey generation logic is biased.

Fixed this by adding a routine that selects passkey again if passkey is 4,200,000,000 or more.

Signed-off-by: Mincheol Son <encrypted.def@gmail.com>
---
 net/bluetooth/smp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

kernel test robot Dec. 7, 2020, 9:27 p.m. UTC | #1 
Hi Mincheol,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on net-next/master net/master bluetooth/master sparc-next/master v5.10-rc7 next-20201207]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mincheol-Son/Bluetooth-smp-Fix-biased-random-passkey-generation/20201208-015207
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: parisc-randconfig-r035-20201207 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/80c9c180f997bc9d9e1df4426fc7957839caee56
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mincheol-Son/Bluetooth-smp-Fix-biased-random-passkey-generation/20201208-015207
        git checkout 80c9c180f997bc9d9e1df4426fc7957839caee56
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/bluetooth/smp.c: In function 'tk_request':
>> net/bluetooth/smp.c:927:3: warning: this decimal constant is unsigned only in ISO C90
     927 |   } while (passkey >= (u32)4200000000);
         |   ^

vim +927 net/bluetooth/smp.c

   849	
   850	static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
   851							u8 local_io, u8 remote_io)
   852	{
   853		struct hci_conn *hcon = conn->hcon;
   854		struct l2cap_chan *chan = conn->smp;
   855		struct smp_chan *smp = chan->data;
   856		u32 passkey = 0;
   857		int ret;
   858	
   859		/* Initialize key for JUST WORKS */
   860		memset(smp->tk, 0, sizeof(smp->tk));
   861		clear_bit(SMP_FLAG_TK_VALID, &smp->flags);
   862	
   863		BT_DBG("tk_request: auth:%d lcl:%d rem:%d", auth, local_io, remote_io);
   864	
   865		/* If neither side wants MITM, either "just" confirm an incoming
   866		 * request or use just-works for outgoing ones. The JUST_CFM
   867		 * will be converted to JUST_WORKS if necessary later in this
   868		 * function. If either side has MITM look up the method from the
   869		 * table.
   870		 */
   871		if (!(auth & SMP_AUTH_MITM))
   872			smp->method = JUST_CFM;
   873		else
   874			smp->method = get_auth_method(smp, local_io, remote_io);
   875	
   876		/* Don't confirm locally initiated pairing attempts */
   877		if (smp->method == JUST_CFM && test_bit(SMP_FLAG_INITIATOR,
   878							&smp->flags))
   879			smp->method = JUST_WORKS;
   880	
   881		/* Don't bother user space with no IO capabilities */
   882		if (smp->method == JUST_CFM &&
   883		    hcon->io_capability == HCI_IO_NO_INPUT_OUTPUT)
   884			smp->method = JUST_WORKS;
   885	
   886		/* If Just Works, Continue with Zero TK and ask user-space for
   887		 * confirmation */
   888		if (smp->method == JUST_WORKS) {
   889			ret = mgmt_user_confirm_request(hcon->hdev, &hcon->dst,
   890							hcon->type,
   891							hcon->dst_type,
   892							passkey, 1);
   893			if (ret)
   894				return ret;
   895			set_bit(SMP_FLAG_WAIT_USER, &smp->flags);
   896			return 0;
   897		}
   898	
   899		/* If this function is used for SC -> legacy fallback we
   900		 * can only recover the just-works case.
   901		 */
   902		if (test_bit(SMP_FLAG_SC, &smp->flags))
   903			return -EINVAL;
   904	
   905		/* Not Just Works/Confirm results in MITM Authentication */
   906		if (smp->method != JUST_CFM) {
   907			set_bit(SMP_FLAG_MITM_AUTH, &smp->flags);
   908			if (hcon->pending_sec_level < BT_SECURITY_HIGH)
   909				hcon->pending_sec_level = BT_SECURITY_HIGH;
   910		}
   911	
   912		/* If both devices have Keyoard-Display I/O, the master
   913		 * Confirms and the slave Enters the passkey.
   914		 */
   915		if (smp->method == OVERLAP) {
   916			if (hcon->role == HCI_ROLE_MASTER)
   917				smp->method = CFM_PASSKEY;
   918			else
   919				smp->method = REQ_PASSKEY;
   920		}
   921	
   922		/* Generate random passkey. */
   923		if (smp->method == CFM_PASSKEY) {
   924			memset(smp->tk, 0, sizeof(smp->tk));
   925			do {
   926				get_random_bytes(&passkey, sizeof(passkey));
 > 927			} while (passkey >= (u32)4200000000);
   928			passkey %= 1000000;
   929			put_unaligned_le32(passkey, smp->tk);
   930			BT_DBG("PassKey: %d", passkey);
   931			set_bit(SMP_FLAG_TK_VALID, &smp->flags);
   932		}
   933	
   934		if (smp->method == REQ_PASSKEY)
   935			ret = mgmt_user_passkey_request(hcon->hdev, &hcon->dst,
   936							hcon->type, hcon->dst_type);
   937		else if (smp->method == JUST_CFM)
   938			ret = mgmt_user_confirm_request(hcon->hdev, &hcon->dst,
   939							hcon->type, hcon->dst_type,
   940							passkey, 1);
   941		else
   942			ret = mgmt_user_passkey_notify(hcon->hdev, &hcon->dst,
   943							hcon->type, hcon->dst_type,
   944							passkey, 0);
   945	
   946		return ret;
   947	}
   948	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index c659c464f7ca..26ed83e0db34 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -922,7 +922,9 @@  static int tk_request(struct l2cap_conn *conn, u8 remote_oob, u8 auth,
 	/* Generate random passkey. */
 	if (smp->method == CFM_PASSKEY) {
 		memset(smp->tk, 0, sizeof(smp->tk));
-		get_random_bytes(&passkey, sizeof(passkey));
+		do {
+			get_random_bytes(&passkey, sizeof(passkey));
+		} while (passkey >= (u32)4200000000);
 		passkey %= 1000000;
 		put_unaligned_le32(passkey, smp->tk);
 		BT_DBG("PassKey: %d", passkey);