-
Notifications
You must be signed in to change notification settings - Fork 771
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
[bug] skip tls verify in ftps #710
Comments
Like a lot of things in Filestash, there's a compromise between "ease of use" and "security" and you might have a different opinion about where the bar is in various plugins: hence the plugin based approach for all the storage that are supported so you can put your own opinion in if you want to, pick the thing you want and discard the rest because most companies will not see kindly a software that can speak all protocols and all flavors of those protocols. The plugin you cite is made for the layman who have no idea what a certificate is and won't have the level of understanding to know why his FTP doesn't work properly when it does work fine somewhere else and will think that's a bug in Filestash. The goal of plg_backend_ftp is to support the most FTP server I could find in the wild and this has a hidden cost, InsecureSkipVerify is the prime example of such hidden cost as tons of ftps server are either missconfigured or are link to root certificates that aren't known to any of us (very typical in enterprise settings and intranets). If I were to blindly apply the suggested change, tons of people will lose access to their FTP and that's not ok. The real question is how do we solve this? The most realistic answer is to have a "hardened ftps" version that instead of favoring compatibility with all possible FTP server would provide a hardened version of it that work without any of the drawback of the existing implementation and at the cost of compatibility. I'm happy to accept a PR to whoever make one up as it would make for a great addition. On my end I'm way to busy working on this |
We are very sorry for the inconvenience caused to you, but you did not edit the security bulletin page on GitHub. Our issue has been raised for more than 3 weeks and we have not received a response before we chose to make CVE public. We have now closed the bulletin page and contacted the CVE team to revoke the public release |
Description of the bug
In the
Init
function ofindex.go
located ingithub.com/mickael-kerjean/filestash/server/plugin/plg_backend_ftp
, the FTPS (FTPs over TLS) connections are being established with theInsecureSkipVerify
flag set totrue
. This configuration instructs the client to bypass the validation of the server's TLS certificate, which is inherently insecure and can expose the connection to man-in-the-middle attacks.Step by step instructions to reproduce the bug
server/plugin/plg_backend_ftp/index.go
file.2 Within the
Init
function, locate the conditional statement that configures thetls.Config
whenwithTLS
is true.InsecureSkipVerify
totrue
.Observed behavior
The FTPS connection is established without validating the server's TLS certificate, which means the connection is susceptible to interception and tampering by third parties.
Expected behavior
The TLS configuration should include a secure validation of the server's certificate. At minimum, there should be an option to enable or disable certificate verification based on the user's preference or security requirements. This would ensure that the FTPS connections are properly secured and not vulnerable to security threats due to misconfigured TLS settings.
The text was updated successfully, but these errors were encountered: