uuid.uuid1() is not suitable as an unguessable identifier/token
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| MAAS |
Fix Released
|
Critical
|
Blake Rouse | ||
| 1.2 |
Won't Fix
|
Critical
|
Unassigned | ||
| 1.5 |
Won't Fix
|
Critical
|
Unassigned | ||
| 1.6 |
Won't Fix
|
Critical
|
Unassigned | ||
| 1.7 |
Won't Fix
|
Undecided
|
Unassigned | ||
| 1.9 |
Fix Released
|
Critical
|
Blake Rouse | ||
Bug Description
uuid.uuid1() has been used for the FileStorage.key field. This field is
used to create unguessable URLs that we can hand out from MAAS to refer
back to that file. However, uuid1() is not random enough:
def uuid1(node=None, clock_seq=None):
"""Generate a UUID from a host ID, sequence number, and the
current time. If 'node' is not given, getnode() is used to obtain
the hardware address. If 'clock_seq' is given, it is used as the
sequence number; otherwise a random 14-bit sequence number is
chosen."""
The host ID is, afaik, based on the machine's MAC address, so this is
easily discoverable. The current time is assumed to be nanoseconds, but
in practice appears to be microseconds; check time.time() output.
An attacker, with the knowledge of the machine's MAC address, would need
to search 58982400000000 UUIDs to discover every file created during a 1
hour timespan (3600e6 microseconds * 2^14). That's a lot of UUIDs, but I
doubt it's even close to "secure".
I think uuid.uuid4() would be a better choice here, as it is entirely
made from random data. Or we could go direct and encode (e.g. base
16/32/64) 16 bytes straight from /dev/urandom.
This follows on from discussion in bug 1212205.
CVE References
| summary: |
- uuid.uuid1() is not suitable as a "private" identifier/token + uuid.uuid1() is not suitable as an unguessable identifier/token |
| description: | updated |
| Julian Edwards (julian-edwards) wrote | #1 |
| Changed in maas: | |
| status: | New → Triaged |
| importance: | Undecided → Critical |
| milestone: | none → next |
| Gavin Panella (allenap) wrote | #2 |
| Raphaël Badin (rvb) wrote | #3 |
| Julian Edwards (julian-edwards) wrote Re: [Bug 1379826] Re: uuid.uuid1() is not suitable as an unguessable identifier/token | #4 |
| Christian Reis (kiko) wrote | #5 |
| Julian Edwards (julian-edwards) wrote | #6 |
| Changed in maas: | |
| milestone: | next → 1.7.2 |
| Changed in maas: | |
| status: | Triaged → In Progress |
| Marc Deslauriers (mdeslaur) wrote | #7 |
| Marc Deslauriers (mdeslaur) wrote | #8 |
| Marc Deslauriers (mdeslaur) wrote | #9 |
| Changed in maas: | |
| status: | Fix Committed → Fix Released |
| information type: | Private Security → Public Security |
I am assuming this is a trivial fix s/uuid1/uuid4/ ?
It needs landing on all existing releases as well, so targeting appropriately.