diff mbox

nvmem: sunxi-sid: SID content is not a valid source of randomness

Message ID 1477144408-15896-1-git-send-email-clabbe.montjoie@gmail.com (mailing list archive)
State Rejected
Delegated to: Herbert Xu
Headers show

Commit Message

Corentin Labbe Oct. 22, 2016, 1:53 p.m. UTC 
Since SID's content is constant over reboot, it must not be used
as source of randomness.

This patch remove the use of SID content as source of randomness.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/nvmem/sunxi_sid.c | 21 ---------------------
 1 file changed, 21 deletions(-)

Comments

Maxime Ripard Oct. 24, 2016, 8:10 p.m. UTC | #1 
On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> Since SID's content is constant over reboot,

That's not true, at least not across all the Allwinner SoCs, and
especially not on the A10 and A20 that this driver supports.

> it must not be used as source of randomness.

And I don't think that's true either. A constant entropy provider will
not add any entropy, but will not remove any, would it?

Maxime
Corentin Labbe Oct. 25, 2016, 5:38 a.m. UTC | #2 
On Mon, Oct 24, 2016 at 10:10:20PM +0200, Maxime Ripard wrote:
> On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > Since SID's content is constant over reboot,
> 
> That's not true, at least not across all the Allwinner SoCs, and
> especially not on the A10 and A20 that this driver supports.
> 

On my cubieboard2 (A20)
hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000200
cubiedev ~ # reboot
cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000200

So clearly for me its constant.

> > it must not be used as source of randomness.
> 
> And I don't think that's true either. A constant entropy provider will
> not add any entropy, but will not remove any, would it?

I cced linux-crypto at the begining for confirmation on that.
But the problem is increased as a part of the content is predicatable over same type of device (at least the thirst bytes and all the zeros).
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jean-Francois Moine Oct. 25, 2016, 7:06 a.m. UTC | #3 
On Tue, 25 Oct 2016 07:38:55 +0200
LABBE Corentin <clabbe.montjoie@gmail.com> wrote:

> > On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > > Since SID's content is constant over reboot,
> > 
> > That's not true, at least not across all the Allwinner SoCs, and
> > especially not on the A10 and A20 that this driver supports.
> > 
> 
> On my cubieboard2 (A20)
> hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000200
> cubiedev ~ # reboot
> cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000200
> 
> So clearly for me its constant.

Even after power off/power on?
Maxime Ripard Oct. 25, 2016, 1:26 p.m. UTC | #4 
On Tue, Oct 25, 2016 at 07:38:55AM +0200, LABBE Corentin wrote:
> On Mon, Oct 24, 2016 at 10:10:20PM +0200, Maxime Ripard wrote:
> > On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > > Since SID's content is constant over reboot,
> > 
> > That's not true, at least not across all the Allwinner SoCs, and
> > especially not on the A10 and A20 that this driver supports.
> > 
> 
> On my cubieboard2 (A20)
> hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000200
> cubiedev ~ # reboot
> cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> *
> 00000200
> 
> So clearly for me its constant.

It's constant across reboots, but not across devices. Each device have
a different SID content, therefore it's a relevant source of entropy
in the system.

Maxime
Corentin Labbe Oct. 28, 2016, 12:37 p.m. UTC | #5 
On Tue, Oct 25, 2016 at 09:06:34AM +0200, Jean-Francois Moine wrote:
> On Tue, 25 Oct 2016 07:38:55 +0200
> LABBE Corentin <clabbe.montjoie@gmail.com> wrote:
> 
> > > On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > > > Since SID's content is constant over reboot,
> > > 
> > > That's not true, at least not across all the Allwinner SoCs, and
> > > especially not on the A10 and A20 that this driver supports.
> > > 
> > 
> > On my cubieboard2 (A20)
> > hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> > 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000200
> > cubiedev ~ # reboot
> > cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> > 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000200
> > 
> > So clearly for me its constant.
> 
> Even after power off/power on?

Yes, even after remove of any power supply.

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Corentin Labbe Nov. 10, 2016, 3:14 p.m. UTC | #6 
On Tue, Oct 25, 2016 at 03:26:48PM +0200, Maxime Ripard wrote:
> On Tue, Oct 25, 2016 at 07:38:55AM +0200, LABBE Corentin wrote:
> > On Mon, Oct 24, 2016 at 10:10:20PM +0200, Maxime Ripard wrote:
> > > On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > > > Since SID's content is constant over reboot,
> > > 
> > > That's not true, at least not across all the Allwinner SoCs, and
> > > especially not on the A10 and A20 that this driver supports.
> > > 
> > 
> > On my cubieboard2 (A20)
> > hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> > 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000200
> > cubiedev ~ # reboot
> > cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem 
> > 00000000  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000100  16 51 66 83 80 48 50 72  56 54 48 48 03 c2 75 72  |.Qf..HPrVTHH..ur|
> > 00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > *
> > 00000200
> > 
> > So clearly for me its constant.
> 
> It's constant across reboots, but not across devices. Each device have
> a different SID content, therefore it's a relevant source of entropy
> in the system.
> 

Not the 3 leading digit and not the tailing zeros which are the same accross device.
So only 50% of data are really different accross devices.
Perhaps a "random-range" property could be used ?

Herbert, does it is safe to add that 50% duplicate content via add_device_randomness() ?
Reading add_device_randomness doc, it seems finally it is safe, but if you could confirm it.

Regards

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index 1567ccc..c82d5d1 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -21,8 +21,6 @@ 
 #include <linux/nvmem-provider.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/slab.h>
-#include <linux/random.h>
 
 static struct nvmem_config econfig = {
 	.name = "sunxi-sid",
@@ -70,8 +68,6 @@  static int sunxi_sid_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct nvmem_device *nvmem;
 	struct sunxi_sid *sid;
-	int ret, i, size;
-	char *randomness;
 
 	sid = devm_kzalloc(dev, sizeof(*sid), GFP_KERNEL);
 	if (!sid)
@@ -82,7 +78,6 @@  static int sunxi_sid_probe(struct platform_device *pdev)
 	if (IS_ERR(sid->base))
 		return PTR_ERR(sid->base);
 
-	size = resource_size(res) - 1;
 	econfig.size = resource_size(res);
 	econfig.dev = dev;
 	econfig.reg_read = sunxi_sid_read;
@@ -91,25 +86,9 @@  static int sunxi_sid_probe(struct platform_device *pdev)
 	if (IS_ERR(nvmem))
 		return PTR_ERR(nvmem);
 
-	randomness = kzalloc(sizeof(u8) * (size), GFP_KERNEL);
-	if (!randomness) {
-		ret = -EINVAL;
-		goto err_unreg_nvmem;
-	}
-
-	for (i = 0; i < size; i++)
-		randomness[i] = sunxi_sid_read_byte(sid, i);
-
-	add_device_randomness(randomness, size);
-	kfree(randomness);
-
 	platform_set_drvdata(pdev, nvmem);
 
 	return 0;
-
-err_unreg_nvmem:
-	nvmem_unregister(nvmem);
-	return ret;
 }
 
 static int sunxi_sid_remove(struct platform_device *pdev)