| Message ID | 20200921163021.v1.1.Id3160295d33d44a59fa3f2a444d74f40d132ea5c@changeid |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v1] Bluetooth: Enforce key size of 16 bytes on FIPS level | expand |
Hi Archie, On Mon, Sep 21, 2020 at 1:31 AM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: > Device in security mode 4 level 4 shall enforce: > 128-bit equivalent strength for link and encryption keys required > using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, > and P-192 not allowed; encryption key not shortened) > > This patch rejects connection with key size below 16 for FIPS level > services. > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > Reviewed-by: Alain Michaud <alainm@chromium.org> > > --- > > net/bluetooth/l2cap_core.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index ade83e224567..306616ec26e6 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) > * that have no key size requirements. Ensure that the link is > * actually encrypted before enforcing a key size. > */ > + int min_key_size = hcon->hdev->min_enc_key_size; > + > + if (hcon->sec_level == BT_SECURITY_FIPS) > + min_key_size = 16; > + > return (!test_bit(HCI_CONN_ENCRYPT, &hcon->flags) || > - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); > + hcon->enc_key_size >= min_key_size); While this looks fine to me, it looks like this should be placed elsewhere since it takes an hci_conn and it is not L2CAP specific. > } > > static void l2cap_do_start(struct l2cap_chan *chan) > -- > 2.28.0.681.g6f77f65b4e-goog >
Hi Luiz, On Tue, 22 Sep 2020 at 01:13, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > On Mon, Sep 21, 2020 at 1:31 AM Archie Pusaka <apusaka@google.com> wrote: > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: > > Device in security mode 4 level 4 shall enforce: > > 128-bit equivalent strength for link and encryption keys required > > using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, > > and P-192 not allowed; encryption key not shortened) > > > > This patch rejects connection with key size below 16 for FIPS level > > services. > > > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > --- > > > > net/bluetooth/l2cap_core.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index ade83e224567..306616ec26e6 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) > > * that have no key size requirements. Ensure that the link is > > * actually encrypted before enforcing a key size. > > */ > > + int min_key_size = hcon->hdev->min_enc_key_size; > > + > > + if (hcon->sec_level == BT_SECURITY_FIPS) > > + min_key_size = 16; > > + > > return (!test_bit(HCI_CONN_ENCRYPT, &hcon->flags) || > > - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); > > + hcon->enc_key_size >= min_key_size); > > While this looks fine to me, it looks like this should be placed > elsewhere since it takes an hci_conn and it is not L2CAP specific. From what I understood, it is permissible to use AES-CCM P-256 encryption with key length < 16 when encrypting the link, but such a connection does not satisfy security level 4, and therefore must not be given access to level 4 services. However, I think it is permissible to give them access to level 3 services or below. Should I use l2cap chan->sec_level for this purpose? I'm kind of lost on the difference between hcon->sec_level and chan->sec_level. > > > } > > > > static void l2cap_do_start(struct l2cap_chan *chan) > > -- > > 2.28.0.681.g6f77f65b4e-goog > > > > > -- > Luiz Augusto von Dentz
Hi Archie, On Tue, Sep 22, 2020 at 12:37 AM Archie Pusaka <apusaka@google.com> wrote: > > Hi Luiz, > > On Tue, 22 Sep 2020 at 01:13, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Archie, > > > > On Mon, Sep 21, 2020 at 1:31 AM Archie Pusaka <apusaka@google.com> wrote: > > > > > > From: Archie Pusaka <apusaka@chromium.org> > > > > > > According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: > > > Device in security mode 4 level 4 shall enforce: > > > 128-bit equivalent strength for link and encryption keys required > > > using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, > > > and P-192 not allowed; encryption key not shortened) > > > > > > This patch rejects connection with key size below 16 for FIPS level > > > services. > > > > > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > > > Reviewed-by: Alain Michaud <alainm@chromium.org> > > > > > > --- > > > > > > net/bluetooth/l2cap_core.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > > index ade83e224567..306616ec26e6 100644 > > > --- a/net/bluetooth/l2cap_core.c > > > +++ b/net/bluetooth/l2cap_core.c > > > @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) > > > * that have no key size requirements. Ensure that the link is > > > * actually encrypted before enforcing a key size. > > > */ > > > + int min_key_size = hcon->hdev->min_enc_key_size; > > > + > > > + if (hcon->sec_level == BT_SECURITY_FIPS) > > > + min_key_size = 16; > > > + > > > return (!test_bit(HCI_CONN_ENCRYPT, &hcon->flags) || > > > - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); > > > + hcon->enc_key_size >= min_key_size); > > > > While this looks fine to me, it looks like this should be placed > > elsewhere since it takes an hci_conn and it is not L2CAP specific. > > From what I understood, it is permissible to use AES-CCM P-256 > encryption with key length < 16 when encrypting the link, but such a > connection does not satisfy security level 4, and therefore must not > be given access to level 4 services. However, I think it is > permissible to give them access to level 3 services or below. > > Should I use l2cap chan->sec_level for this purpose? I'm kind of lost > on the difference between hcon->sec_level and chan->sec_level. The chan->sec_level is L2CAP channel required sec_level while hcon->sec_level is the current secure level in effect, at some point I guess we assign the hcon->sec_level with chan->sec_level but Im not sure if that has already happened here or not. > > > > > } > > > > > > static void l2cap_do_start(struct l2cap_chan *chan) > > > -- > > > 2.28.0.681.g6f77f65b4e-goog > > > > > > > > > -- > > Luiz Augusto von Dentz
Hi Archie, > According to the spec Ver 5.2, Vol 3, Part C, Sec 5.2.2.8: > Device in security mode 4 level 4 shall enforce: > 128-bit equivalent strength for link and encryption keys required > using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, > and P-192 not allowed; encryption key not shortened) > > This patch rejects connection with key size below 16 for FIPS level > services. > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > Reviewed-by: Alain Michaud <alainm@chromium.org> > > --- > > net/bluetooth/l2cap_core.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index ade83e224567..306616ec26e6 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) > * that have no key size requirements. Ensure that the link is > * actually encrypted before enforcing a key size. > */ > + int min_key_size = hcon->hdev->min_enc_key_size; > + > + if (hcon->sec_level == BT_SECURITY_FIPS) > + min_key_size = 16; > + > return (!test_bit(HCI_CONN_ENCRYPT, &hcon->flags) || > - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); > + hcon->enc_key_size >= min_key_size); > } I think this is fine at this position. It is a L2CAP socket requirement to be in FIPS mode since you will set it via socket option. However I would extend the comment above to describe what is going on. And generally the variable declaration might be better placed before the comment. Regards Marcel
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index ade83e224567..306616ec26e6 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1515,8 +1515,13 @@ static bool l2cap_check_enc_key_size(struct hci_conn *hcon) * that have no key size requirements. Ensure that the link is * actually encrypted before enforcing a key size. */ + int min_key_size = hcon->hdev->min_enc_key_size; + + if (hcon->sec_level == BT_SECURITY_FIPS) + min_key_size = 16; + return (!test_bit(HCI_CONN_ENCRYPT, &hcon->flags) || - hcon->enc_key_size >= hcon->hdev->min_enc_key_size); + hcon->enc_key_size >= min_key_size); } static void l2cap_do_start(struct l2cap_chan *chan)