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

[FEATURE] Allow disabling TLS entirely #261

Open
Thalhammer opened this issue Jan 17, 2025 · 3 comments
Open

[FEATURE] Allow disabling TLS entirely #261

Thalhammer opened this issue Jan 17, 2025 · 3 comments

Comments

@Thalhammer
Copy link

Version: 90f9c2d (current master)
Android: 12 (Samsung Galaxy S10+, OneUI 4.1)

Expected Behaviour:
There is an option to disable TLS support or the TLS support works out of the Box.

Observed Behaviour:
It doesn't.
Connecting with Filezilla to the plaintext socket fails with a GnuTLS-Error -15. I suspect the reason for this is that I don't have a certificate configured in swiftp, but I don't think I should have to explicitly configure one to get a basic ftp server. I don't need TLS anyway (I use the phone hotspot for connecting my digital camera, so noone has access anyway).

Fix:
Option 1: Introduce a setting to allow turning off AUTH TLS/SSL and make it return the default 502 reply, causing clients to do plaintext auth.
Option 2: Autogenerate a certificate on first startup.

Imho both should be implemented with Option 2 + TLS on being the default. But it is still a value add if TLS can be toggled off because a client might have a faulty TLS implementation but connect fine in plaintext.

@Xavron
Copy link

Xavron commented Jan 19, 2025

TLDR only version. Instead of making this really long, replaced with a short concise version, as its probably better. Yes, this isn't very short lol.

  • FTPS Explicit uses plain connection by design and starts off as plain. This is how FTPS is designed.

  • Even a "plain only connection loop" in the eyes of the user here, will allow a client set to FTPS Explicit to attempt AUTH command. There's really nothing different as FTPS Explicit starts off as a plain connection, besides possibly where it fails at.

  • There's really no way to block earlier as the user suggests because FTPS explicit starts off as plain. The only way to block is only about the same way as it blocks now, once a command like AUTH is received.

  • Plain only connections will break from wrong choices in Advanced Settings. This is what I see as happening here.

  • Its expected that users will face more problems as FTPS, and increased settings are complicated. There's ultimately nothing that can be done about this. Users must figure out many problems on their own. No client or server can overcome this.

  • Due to FTPS Explicit starting off as plain, there's no security enhancement from attempting a "plain only loop". It already is a plain loop. FTP Explicit must spin up from a plain only loop after asking for the command to do so. It will maybe just fail on something else and that is it. I don't know how to make this more clear.

  • It is not possible to block FTPS Explicit in the way the user is thinking there as it starts off as plain.

  • TLS cannot ever be used without certs, as it requires the TLS handshake, and that handshake cannot be done without the certs. So, no, TLS is not being used here.

  • The error is correct. TLS cannot be used because its not setup and because the user cannot use it. The error is a result of things working correctly, and if the user wanted it to work, is a result of the user doing things incorrectly. That is not something that can be dealt with except by the user.

  • "Allow disabling TLS entirely" does not actually do anything. It is disabled. It cannot be used here in this issue. It starts off as plain only. Wrong settings will break plain only connections.

I could go on with more but, this will suffice. From here, it will only get more complicated, difficult for users to understand.

Ppareit will need to decide anything further such as if deciding to deal with settings in a way that users have less chance of causing conflicts. I see this as partially futile, as its still going to happen regardless. I am done with FTPS unless I see a bug or problem. I do not see any non-user issue here. To me, this is a can't do anything about situation, as there's nothing to do. Except to attempt to explain how it actually works.

You can search the internet on FTPS Explicit and see that it works this way.

Its also going to be a learning curve for people who are not used to FTPS being included.

The only real way to do what is suggested here is to remove FTPS from the app. That way its removed from the user's view. Otherwise, the user has to learn. Maybe, this helps with the learning. But, I have no idea if it does.

@Thalhammer
Copy link
Author

Thalhammer commented Jan 20, 2025

The User (i.e. me) knows how FTPS Explicit works and is aware of the fact that adding a cert would fix the issue, but the issue the user is complaining about isn't that. Its that swiftp falsely claims to support TLS/SSL to the client (by replying with a non error code to AUTH TLS) when it fact it doesn't (cause of the missing cert). Instead it should either disable TLS when no cert is provided or allow the user to generate a cert right in the app (and probably do so automatically).

While I can easily generate a jks/bks file on my pc and copy it over to the phone (ironically not using ftp though cause that doesn't work unless you add a cert), the majority of non IT users probably can't and have no idea what a jks file is, let alone how generate one.

I have no idea what you are referencing with a "plain only connection loop", but I highly doubt that's needed, cause all thats needed is for AUTH TLS to return a error (and proceed as if it was never issued by the client) if no certs are provided. Effectively behaving as if TLS wasn't supported.

I can provide an PR with the changes to allow this, if there's any interest in fixing it.

EDIT: Since I feel like theres a clear lack of knowledge about how the AUTH command works I linked the relevant RFC. You might wanna read it, particularly page 6.

@Xavron
Copy link

Xavron commented Jan 20, 2025

Thank you for the clarification of what you're saying.

#242

Things not done

  • Further implement the new FTP commands as they're currently done only to a working minimum.

  • Anything I missed from the rfc docs.

I don't speak for ppareit but can tell you what I've always seen.

I'm sure ppareit would accept a PR if you'd like to do so. Has always been open to all. I think its great. To me, you're fully welcome to (not my project anyway, and I'd love for someone else to), and ppareit is always very nice.

If you do anything, I look forward to seeing it.

All the best :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants