Closed sistemicorp closed 2 years ago
davesteele
commented
2 years ago Comitup has a recent change that likely exposed this problem. It makes the null visible to dbus (e.g. see test).
I've been assuming that hidden SSIDs would not be reported by "iw scan". Filtering out invalid unicode strings in iwscan sounds like a good idea.
davesteele
commented
2 years ago I'm getting the ValueError exception from the eval() in decode_x() from strings with encoded nulls ("source code string cannot contain null bytes").
sistemicorp
commented
2 years ago I would be glad to test any code snippet... Or do you want me to propose a change?
decode_x() returns a string, so if the change is made there, it would have to return something else, as presumably if the string is all nulls (hidden) we don't want to add this SSID to the choice list.
So the fix would be in devaps()...?
def devaps(dev: str, dump: str = "") -> List[Dict[str, str]]:
"""Get a list of Access Points (as dicts) for a device"""
if not dump:
log.debug("Getting AP list from 'iw' dev %s" % dev)
dump = docmd("iw dev %s scan" % dev)
aps = []
for blk in re.split("\nBSS ", dump[4:]):
try:
ap = blk2dict(blk)
ap["power"] = dbm2pct(float(ap["signal"].split()[0]))
if ap["SSID"]: <----- Change to make sure string is not null bytes...
ap["SSID"] = decode_x(ap["SSID"])
aps.append(ap)
except KeyError:
pass
return apsNot quite. The string going into decode_x is not null bytes - it is escaped null bytes.
Try changing "except KeyError: to "except (KeyError, ValueError):"
sistemicorp
commented
2 years ago Right, I was just editing my response. A valid SSID can contain any Null characters anywhere in the string. Seems SSID can also be data, or so that option exists, so one can't assume it is a string of printable characters, see this
davesteele
commented
2 years ago Yikes.
OTOH, if one makes an unprintable SSID, they should not be surprised when it is not printed,
sistemicorp
commented
2 years ago Exactly.
Consider this, a hidden SSID is a common thing, probably way more common than embedding non-printable characters in SSID. And consider that a with a hidden SSID, having that as an option to the user to select doesn't make sense, comitup will not be able to connect to that SSID even if the user enters a password. And a hidden SSID is handled differently by the web form.
So. Can we detect the hidden SSID (all null bytes) and skip adding that to the scanned list?
AND, I we add your except (KeyError, ValueError): to handle any other case of embedded null...
I am just getting setup to test that change... Thoughts?
sistemicorp
commented
2 years ago Looking up SSID stuff, seems its max length is 32. The hidden SSID near me is 18 Null bytes. So, scan for all nulls,
def devaps(dev: str, dump: str = "") -> List[Dict[str, str]]:
"""Get a list of Access Points (as dicts) for a device"""
if not dump:
log.debug("Getting AP list from 'iw' dev %s" % dev)
dump = docmd("iw dev %s scan" % dev)
aps = []
for blk in re.split("\nBSS ", dump[4:]):
try:
ap = blk2dict(blk)
ap["power"] = dbm2pct(float(ap["signal"].split()[0]))
# scan SSID, if only nulls, its hidden, 0-31, skip adding it to scanned
hidden_ssid = True
for c in ap["SSID"]:
if ord(c): # any non-null
hidden_ssid = False
break
if ap["SSID"] and not hidden_ssid:
ap["SSID"] = decode_x(ap["SSID"])
aps.append(ap)
except KeyError:
pass
return apsFrom what I tested, the "except" edit I proposed above should be enough.
sistemicorp
commented
2 years ago Right, I agree.
I am arguing to remove all Null strings from the AP list that is shown to the user. Because it is hidden. I can send a pic from what it looks like on the Hotspot access page, one sees an AP entry that is "\x00\x00..."
davesteele
commented
2 years ago Sure. I suspect, though, that any SSID including nulls wants to be more hidden. The "except" should achieve that.
Note that connection requests are identified by SSID, so you would need to separate the sanitized printed vs. the actual SSID.
sistemicorp
commented
2 years ago except (KeyError, ValueError): didn't fix anything here because the error doesn't occur there in my setup.
There are two errors, one thrown by dbus,
Jan 25 04:33:51 p1125-3ef8 comitup[910]: ERROR:dbus.service:Unable to append ([{'ssid': 'darkwood', 'strength': '94.0', 'security': 'encrypted'}, {'ssid': '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', 'strength': '68.0', 'security': 'encrypted'}, {'ssid': '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00', 'strength': '68.0', 'security': 'encrypted'}],) to message with signature aa{ss}: <class 'ValueError'>: embedded null byteand another thrown by comitup-web, see log snippet in first post.
My attempt to filter SSIDs that are all nulls is not working and has me very perplexed. The string is hex escaped, and in my test program, each escaped hex is a char, but in the RPi, the string is acting like its not escaped hex... I will eventually figure that out...
Once hidden SSIDs are filtered from the AP list, I am hoping this fixes both errors. It might not be the right fix though... it depends what we think about SSID names and the rules you want to put in place...
I was hoping that the iw scan info included a field that indicates that the SSID is hidden.... but it doesnt.
davesteele
commented
2 years ago Ok. I don't understand how dbus is still seeing the Exception.
sistemicorp
commented
2 years ago Here I added some logging to comitup,
2022-01-25 23:17:27,658 - comitup - INFO - Activating hotspot
2022-01-25 23:17:36,222 - comitup - ERROR - MGAG: type <class 'str'>, darkwood
2022-01-25 23:17:36,224 - comitup - ERROR - MGAG added SSID
2022-01-25 23:17:36,226 - comitup - ERROR - MGAG: type <class 'str'>, darkwood
2022-01-25 23:17:36,227 - comitup - ERROR - MGAG added SSID
2022-01-25 23:17:36,228 - comitup - ERROR - MGAG: type <class 'str'>, \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00
2022-01-25 23:17:36,228 - comitup - ERROR - MGAG added SSID
2022-01-25 23:17:36,229 - comitup - ERROR - MGAG: type <class 'str'>, darkwood
2022-01-25 23:17:36,229 - comitup - ERROR - MGAG added SSID
2022-01-25 23:17:36,230 - comitup - ERROR - MGAG: type <class 'str'>, \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00
2022-01-25 23:17:36,230 - comitup - ERROR - MGAG added SSID
2022-01-25 23:17:36,231 - comitup - ERROR - MGAG: type <class 'str'>, \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00
2022-01-25 23:17:36,231 - comitup - ERROR - MGAG added SSID
2022-01-25 23:17:47,155 - comitup - ERROR - MGAG: type <class 'str'>, darkwooddarkwood is my known SSID. There are two others that are hidden... I live in the country and don't have any neighbours... I don't know what these hidden SSIDs are... I think one is a furnance, and the other is a cellular modem antenna sitting on the roof.
Whats interesting is that the two hidden SSIDs have difference lengths of null, and neither fills the total of 32.
Note that the two null SSIDs are being printed out, yet \x00 is supposed to be null. When I create a python shell and use that as a string, I see a null. But in-system, it comes out as a string. So I was printing its type and... well, still trying to understand whats going on there with the string...
dbus: I think it sees the problem because the null APs are added to the list...
But I am still researching
davesteele
commented
2 years ago The string is an str, returned from the "iw" cli command. It includes non-ASCII characters using the "\x" escape, again, as a str.
decode_x() turns that back into a byte string, then works on the escapes.
sistemicorp
commented
2 years ago Yeah... I figured something out at least...
In the shell if I use mystring = r"\x00\x00" then I get whats happening in-system. Without the "r" mystring acts like a hex escaped string. But in-system its not acting that way.
sistemicorp
commented
2 years ago This solved the issue(s) for my environment. To be clear, I am not proposing this is the final solution. Consider this research.
def devaps(dev: str, dump: str = "") -> List[Dict[str, str]]:
"""Get a list of Access Points (as dicts) for a device"""
if not dump:
log.debug("Getting AP list from 'iw' dev %s" % dev)
dump = docmd("iw dev %s scan" % dev)
aps = []
for blk in re.split("\nBSS ", dump[4:]):
try:
ap = blk2dict(blk)
ap["power"] = dbm2pct(float(ap["signal"].split()[0]))
if ap["SSID"].count(r"\x00"): # null chars indicate hidden SSID, skip
continue
if ap["SSID"]:
ap["SSID"] = decode_x(ap["SSID"])
aps.append(ap)
except KeyError:
pass
return apsIf there was a way to make an SSID that was a mix of chars and had one or more nulls, then it would be tough to save that string onto the file system, without encoding. So I submit that if there are any \x00 strings, then the SSID is skipped. And note that the SSID string in system has a "python" null terminator, meaning its there, and \x00 has nothing to do with normal string termination... in this context.
davesteele
commented
2 years ago I just commented out the "eval" in decode_x(). No exception, and it prints with the nulls stripped out. Not sure why I put that in there.
sistemicorp
commented
2 years ago Indeed, that worked. No exception. The hidden SSIDs show up like this,
davesteele
commented
2 years ago That's an improvement.
sistemicorp
commented
2 years ago I think at some point in the system, you may want to remove the hidden SSIDs, cause I don't think they make sense to display in that above form. As I said above, you already have a pathway for entering a hidden SSIDs.
davesteele
commented
2 years ago 1.33 filters out the nulls, which appears to fix the problems I was seeing. They should be eliminated at some point.
sistemicorp
commented
2 years ago FYI I updated to 1.33 and tested and its an improvement, the comitup service no longer throws the null byte exception.
However, comitup-web still does throw the exception, see the very top first post for the log. I resolve it by throwing away SSIDs that contain (any) nulls,
def devaps(dev: str, dump: str = "") -> List[Dict[str, str]]:
"""Get a list of Access Points (as dicts) for a device"""
if not dump:
log.debug("Getting AP list from 'iw' dev %s" % dev)
dump = docmd("iw dev %s scan" % dev)
aps = []
for blk in re.split("\nBSS ", dump[4:]):
try:
ap = blk2dict(blk)
ap["power"] = dbm2pct(float(ap["signal"].split()[0]))
if ap["SSID"].count(r"\x00"): # null chars indicate hidden SSID, skip
continue
if ap["SSID"]:
ap["SSID"] = decode_x(ap["SSID"])
aps.append(ap)
except KeyError:
pass
return aps
Environment:
Installing the latest comitup (via davesteele-comitup-apt-source_latest.deb) and getting this error, seen in
journalctl -u comitup-web,The hotspot is advertising, but cannot access the web page for selecting the network.
Looking at
systemctl status comitup.service,My guess is that code isn't handling the null (hidden) SSID, and I think its a Python 3.9 issue? Because I know this worked before.
Q: What is the strategy re a hidden SSID in terms of displaying that SSID in the hotspot connection form? I have always seen networks that were named as a string of
\00in the form, but since other SSIDs I expected were present I just ignored these. Given the SSID is hidden, should those networks be removed from the connection list? Would filtering out hidden SSIDs be done iniwscan.py? (I will be able to work on this tonight, so if there is any diagnostic work I can do to help...)