MAC address for node's IPMI is reversed looked up to yield IP address using case sensitive comparison

Bug #1367455 reported by Alan Mimms
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MAAS
Fix Released
High
Julian Edwards
maas (Ubuntu)
New
Undecided
Unassigned

Bug Description

The comparison used in provisioningserver/utils/__init__.py: find_ip_via_arp(mac) compares MAC addresses to the output of an "arp -n" command using case sensitive comparison. This MAC address may be entered by a user in a web UI and often is NOT cut/pasted from ARP but rather from the IPMI vendor's display, which may display the MAC address in uppercase.

For a fix, I strongly recommend doing these things to make this more friendly:

1. Trim all whitespace from the supplied MAC address before comparison.

2. Trim all delimiters too - both on the supplied MAC address and from the output column of the "arp -n" it's being compared with.

3. Translate both the supplied MAC address and the "arp -n" column data to lowercase or uppercase for comparison or use a case-insensitive match in the "for" loop.

- DIFF -

diff /usr/lib/python2.7/dist-packages/provisioningserver/utils/__init__.py __init__.py
46a47
> import re
824a826,827
> mac = mac.lower()
> mac = re.sub('[^0-9a-f]', '', mac)
829c832,833
< if len(columns) == 5 and columns[2] == mac:
---
> col = re.sub('[^0-9a-f]', '', columns[2])
> if len(columns) == 5 and col == mac:

Tags: power trivial

Related branches

Revision history for this message
Alan Mimms (a-mimms) wrote :

OK the diff was useless, and I'm sorry. But I'm not a python weenie, nor do I do diffs everyday for bug reports. The code, which is trivial to someone who writes python all the time, is easy in any case. Thank you for playing.

Revision history for this message
Alan Mimms (a-mimms) wrote :

Oh and the proposed code has a bug too - don't look at columns[2] without checking for len(columns) == 5. Sheesh.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Heh, thank you for the report. It's an easy change to make so I think we can drop this in quickly. The code is fundamentally flawed anyway because it depends on the arp cache being primed, so there's a bit more work to do yet to make it bulletproof.

Changed in maas:
status: New → Triaged
importance: Undecided → High
tags: added: power trivial
Changed in maas:
milestone: none → 1.7.0
Changed in maas:
status: Triaged → Fix Committed
Christian Reis (kiko)
Changed in maas:
assignee: nobody → Julian Edwards (julian-edwards)
Changed in maas:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.