Make PBSZ optional

[gczuczy]

We've found a broken client, which doesn't do PBSZ before PROT, and by RFC2228 proftpd is refusing the client because of this. I would like to ask to make PBSZ optional, because even in the source it's forced to 0, and to the best of my knowledge, it doesn't raise any security issues as well.

As per email discussion with castaglia, I've created a patch, and will create a pull request for it.

The path introduces a new TLSOptions option named "RequirePBSZ", and PBSZ will only be required if that flag is present. If the flag is not specified, then PBSZ is optional.

[Castaglia]

Thanks for the pull request! I may change the TLSOption name to something else later, e.g. TLSOption RFCConformance or somesuch, some name that might be slightly more human-readable, since some users/admins may not be aware of the PBSZ and its role, specifically. But the functionality will stay the same.

[gczuczy]

You are most welcome, I've fixed the things you've pointed out in the pull request, please take a look at it again.

I've chosen "RequirePBSZ", because it's specifically about that command, and the error message also mentions this command in reply to PROT, when forgotten. There are a rather large number of relevant RFCs, and even RFC2228 has multiple requirements, that's one of the reasons why I chosen a name for the option with "RFC" in it. Personally I think, that might be misleading and confusing on the long run.

[Castaglia]

In the case of FTP over SSL/TLS, there's only one particularly relevant RFC: RFC 4127. However, your point is taken.

Another approach might be to not provide such a TLSOption at all, and simply always treat PBSZ as optional. A quick look through the vsftpd and pure-ftpd source code bases show that those FTP daemons do not enforce a requirement of PBSZ, nor do they provide configurations for requiring them, which lends credence to the idea that the admins running those servers have no need to require a PBSZ command.

[gczuczy]

Honestly, I don't really see any reason why couldn't be optional by default, except if some sysadmin is feeling to start a holy crusade against broken clients, and wants to teach the world a lesson. Apart from that, I don't think it would hurt anyone.

Would you like me to change the pull request to make this the default behaviour?

[Castaglia]

Yes, please. It's easy enough to add such an option in, later, if/when we find a use case.

[gczuczy]

Done, please review. Instead of removing the error return, i've just disabled the codepath with a macro, if that's fine. Might come handy if there's a usecase.

[Castaglia]

PR merged to master; thanks!