LAOS: Source port control in commons-net-tftp breaks LAOS

t-oster / LibLaserCut

a platform independant library to control Lasercutters. This is the base library for VisiCut

http://visicut.org

Other

59 stars 55 forks source link

LAOS: Source port control in commons-net-tftp breaks LAOS #131

Open dirkx opened 4 years ago

dirkx commented 4 years ago

We needed to make this https://github.com/MakerSpaceLeiden/commons-net/commit/5c3b9228135e07fe366afbdf5bde73982ad16eaf change in commons.net to keep visicut working with our LAOS laser cutters (https://wiki.makerspaceleiden.nl/mediawiki/index.php/Laser_Engraver).

Dw.

mgmax commented 4 years ago

As far as I understand the specification https://tools.ietf.org/html/rfc1350 , there's a good reason for checking the port (like a session ID), and it is specified like this:

client: random_client_port -> server: 69 : initial request
server: random_server_port <-> client: random_client_port: ACK / data

Do I understand correctly that the problem is that random_server_port == 69? I don't see where the spec strictly mandates that this is an error, although "69" isn't very random :-) So the question is if this error check has any use, or it can simply be removed.

As far as possible, this bug should be fixed upstream in either LAOS or commons-net. For the time being, we could revert to an earlier version of commons-net if it is available via maven.

t-oster commented 4 years ago

It seems as if this was implemented because of this bug report https://issues.apache.org/jira/browse/NET-414 which looks as if they added the check intentionally and I doubt they will remove it. So I think fixing upstream LAOS is the correct way. I added an issue for that (see below). Anyhow, maybe there is a quick workaround without forking commons-net? (Maybe subclass the TFTP Client or similar)

dirkx commented 4 years ago

Hmm - the issue seems to be that pre 1990 code and the world+dog in IoT uses the standard https://github.com/ARMmbed/lwip/blob/master/src/apps/tftp/tftp_server.c example or some variation thereof.

And re-reading the RFC - it does not actually forbid the use of a source TID-69 (there is not even a 'SHOULD' in rfc-parlance to avoid doing it).

mgmax commented 4 years ago

@dirkx That's also my interpretation of the RFC -- can you submit a pull request for commons-net?

mgmax commented 4 years ago

@dirkx Please try to submit a pull request for commons-net so that we don't need to fork it.

t-oster commented 4 years ago

Seems to be fixed in LAOS https://github.com/LaosLaser/Firmware/issues/63#issuecomment-603732692. @dirkx Can we close this?

mgmax commented 4 years ago

The issue is only partially solved. There is a LAOS firmware update, but of course VisiCut should also work with the older firmware. Unfortunately the original poster @dirkx fails to respond to any of my questions, though we almost have the solution at hand. Maybe someone else wants to try?