Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect External Access Warning #5005

Closed
1 of 4 tasks
steve1515 opened this issue Apr 30, 2024 · 15 comments
Closed
1 of 4 tasks

Incorrect External Access Warning #5005

steve1515 opened this issue Apr 30, 2024 · 15 comments
Labels
bug Issue describes a bug bugfix release relevant Issue should be looked at for backporting into the next bugfix release done Done but not yet released
Milestone

Comments

@steve1515
Copy link

The problem

Ever since updating to 1.10.0, I'm not getting a Possible external access detected notification. My access is local only and has always been. It seems something in this detection has recently changed and is broken.

Did the issue persist even in safe mode?

Yes, it did persist

If you could not test in safe mode, please state why ("currently printing" is NOT an excuse!)

No response

Version of OctoPrint

1.10.0

Operating system running OctoPrint

Ubuntu Server 22.04

Printer model & used firmware incl. version

Not applicable

Browser and version of browser, operating system running browser

Firefox 125.0.2 (64-bit) on Windows 10 22H2 with latest Windows Updates

Checklist of files to include below

  • Systeminfo Bundle (always include!)
  • Contents of the JavaScript browser console (always include in cases of issues with the user interface)
  • Screenshots and/or videos showing the problem (always include in case of issues with the user interface)
  • GCODE file with which to reproduce (always include in case of issues with GCODE analysis or printing behaviour)

Additional information & file uploads

Note: I did not include the systeminfo bundle here as it contains some public IPv6 addresses and also a domain name that I do not want to be shown publicly in the github issues. (I can share privately if needed or I can censor them.)

@github-actions github-actions bot added the triage This issue needs triage label Apr 30, 2024
@cp2004
Copy link
Member

cp2004 commented Apr 30, 2024

Note: I did not include the systeminfo bundle here as it contains some public IPv6 addresses and also a domain name that I do not want to be shown publicly in the github issues. (I can share privately if needed or I can censor them.)

This may well be the cause of the warning. If your network is setup so that your access appears to be from a public IPv6 address, then OctoPrint will trigger the warning.

I'm not sure of what changed here in relation to OctoPrint 1.10.0, but there was also this report on the forum.

https://community.octoprint.org/t/how-is-external-access-checked/58170?u=charlie_powell

@steve1515
Copy link
Author

My systems (both client and OctoPrint server) have IPv6 addresses allocated from a public range as that's pretty standard with IPv6.

I would think OctoPrint could check it's hosts directly attached IP networks and compare it to the client. If the networks match, then it could consider it a local connection.

Another solution might be to have a place to add trusted networks. In this case, I would manually add my local network ranges and OctoPrint would consider them local if a client is connecting from them.

@jneilliii
Copy link
Contributor

Possibly validate against localNetworks in config.yaml and if there's a match don't prompt.

https://docs.octoprint.org/en/master/configuration/config_yaml.html#access-control

@steve1515
Copy link
Author

steve1515 commented Apr 30, 2024

I started to look through the code and I think there might be a bigger issue here.
It looks like the code does attempt to get a list of the directly attached network interfaces on the OctoPrint server, but this code seems to be failing.
My initial testing is showing that calls to subnets.append(to_ipnetwork(v4)) and subnets.append(to_ipnetwork(v6)) in the following locations are always throwing exceptions:

subnets.append(to_ipnetwork(v4))

subnets.append(to_ipnetwork(v6))

I'll keep looking, but I think this might be the cause of the issue.

@steve1515
Copy link
Author

The problem is specifically this line:

prefix = address["netmask"]

The element netmask does not exist but mask does. Changing to mask seems to make everything happy.

Here's the strange thing...
netifaces.ifaddresses() returns an object containing mask, but any documentation I can find shows that netmask is supposed to be correct.
One example here: https://pypi.org/project/netifaces/

Anyone have any thoughts on the correct way to proceed here?

@steve1515
Copy link
Author

Even a little more confusing is it seems the source code for netifaces does actually use netmask. See here: https://github.com/al45tair/netifaces/blob/53fcdb6e5dccc84f6734939cfee1a95d3f470d7b/netifaces.c#L996

I'm really not sure why I'm seeing mask instead of netmask... 😕

@steve1515
Copy link
Author

Even more findings...
If I create a simple script like the one below, when I run it with python3 in OctoPrint's venv I get objects with mask, but if I run it outside of the venv as my regular linux user I get the expected netmask. I checkd that netifaces is version 0.11.0 in both locations using pip list | grep netifaces.

This seems strange to me. Is OctoPrint using some kind of custom modified build of netifaces?

import os
import socket
import sys
import netifaces

for interface in netifaces.interfaces():
    addrs = netifaces.ifaddresses(interface)
    for v4 in addrs.get(socket.AF_INET, ()):
        try:
            print(f'v4 subnet = {v4}')
        except Exception:
            print("FAILED v4")

    for v6 in addrs.get(socket.AF_INET6, ()):
        try:
            print(f'v6 subnet = {v6}')
        except Exception:
            print("FAILED v6")

@cp2004
Copy link
Member

cp2004 commented Apr 30, 2024

OctoPrint switched to netifaces2 in 1.10.0

"netifaces2>=0.0.21,<0.1",

Good catch!

@cp2004
Copy link
Member

cp2004 commented Apr 30, 2024

@ManuelMcLure ran into this issue as well - netifaces2 is returning mask instead of netmask.

https://discord.com/channels/704958479194128507/708230829050036236/1232770439416381463

Should at least be a simple fix in OctoPrint, and can be part of a bugfix release for 1.10 when that is required.

@GitIssueBot
Copy link

This issue has been mentioned on OctoPrint Community Forum. There might be relevant details there:

https://community.octoprint.org/t/how-is-external-access-checked/58170/5

@steve1515
Copy link
Author

Aha!!! That means we should change the code to use mask I believe.

I could try to do a PR some time this week.

@ManuelMcLure
Copy link
Contributor

What I did in my plugin was to try mask first and then tried netmask as a fallback, since the plugin might still be running on 1.9.x. For OP core it doesn't make sense to do it that way, I think.

@foosel foosel added bug Issue describes a bug bugfix release relevant Issue should be looked at for backporting into the next bugfix release and removed triage This issue needs triage labels May 1, 2024
@foosel foosel added this to the 1.10.x milestone May 1, 2024
@steve1515
Copy link
Author

I'm not as familiar with python as I am with other languages. Is there any potential issue with both netifaces and netifaces2 being installed in the venv? How do we ensure that netifaces2 is loaded when both are found?

@jneilliii
Copy link
Contributor

I think you could patch locally via SSH to the pi and edit the file directly.

nano ~/oprint/lib/python3.9/site-packages/octoprint/util/net.py

go to line 60 and change it from address["netmask"] to address["mask"]

image

ctrl+s to save ctrl+x to exit and then restart OctoPrint.

jneilliii added a commit that referenced this issue May 1, 2024
in netifaces2 netmask was replaced with mask, #5005
@jneilliii jneilliii mentioned this issue May 1, 2024
10 tasks
@foosel
Copy link
Member

foosel commented May 6, 2024

Fixed by the above commit, ready for 1.10.1.

As explained in #5006, I've decide to go with an either/or approach here with regards what is fetched from the address dict. netifaces2 might at some point change this back from mask to netmask to be fully compatible, or we might run into old netifaces installations out there at some point that end up getting loaded instead of netifaces2 (python can be a bit weird here), so better support both.

@foosel foosel added the done Done but not yet released label May 6, 2024
@foosel foosel closed this as completed in 7d80170 May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue describes a bug bugfix release relevant Issue should be looked at for backporting into the next bugfix release done Done but not yet released
Projects
Status: Done
Development

No branches or pull requests

6 participants