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

ftp module: status success when AUTH TLS failed #276

Closed
lambdafu opened this issue Oct 8, 2020 · 2 comments · Fixed by #418
Closed

ftp module: status success when AUTH TLS failed #276

lambdafu opened this issue Oct 8, 2020 · 2 comments · Fixed by #418

Comments

@lambdafu
Copy link

lambdafu commented Oct 8, 2020

I noticed an inconsistency among the smtp, imap, pop3 and ftp modules: If explicit tls is selected, the smtp, imap and pop3 modules will not set .data.PROTO.status to "success" if the explicit TLS handshake fails. But the ftp module will happily do so. You have to filter for key_material or server_finished in the tls handshake log to get the actually successful connections. Just something odd I noticed, and wanted to let you know in case you want to keep it consistent (which I would suggest). This is with version 0.1.1, which I realize is quite old by now, but I am in the middle of something and can't update right now, sorry.

Thanks!

@lambdafu
Copy link
Author

There are other inconsistencies as well: For example, the ftp module uses application-error for TLS related failures that other modules put into unknown-error.

@developStorm
Copy link
Member

Hi @lambdafu, thanks for reporting this! For the status flag, it seems we fixed the issue earlier in #314. I know it has been quite a while since this was brought up, but could you verify if the current version sets status to a non-success value as expected?

For error type, I have a fix ready but we might need to discuss if we want to continue to use unknown or unifies everything to use a specific error type for TLS. I'll get back on this soon.

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

Successfully merging a pull request may close this issue.

2 participants