ZAP handler communication errors should be handled consistently

[sigiesec]

The current implementations of the mechanisms are not consistent with regard to ZAP handler communication errors, most importantly

1. there is no ZAP handler at all
2. the ZAP handler has not yet started / connected to the ZAP socket
3. the ZAP handler has disconnected from the ZAP socket before the session tries to bind to the ZAP socket
4. the ZAP handler disconnects from the ZAP socket after the session has bound to the ZAP socket
5. the ZAP handler does not read the ZAP request
6. the ZAP handler does not send a ZAP response

The first two error conditions cannot be distinguished. The third could in principle be distinguished from the first and second, but this is not currently implemented, and seems rather arbitrary.

The NULL and CURVE mechanisms simply ignore errors 1-3, and accept the connection attempt. The CURVE mechanism claims to support the Stonehouse pattern by this, however this attributes to error condition 1 only, which appears rather insecure.

The PLAIN mechanism rejects any connection attempt in case of error conditions 1-3.

Condition 4 will likely result in some server-side error. Error conditions 5 and 6 will probably cause a client-side timeout, which might be acceptable.

I suggest that it can be explicitly configured whether a ZAP handler is required by a dedicated socket option. Inconsistent configurations (e.g. a ZAP domain is defined, but ZAP handler is not configured as required) should result in an error.

Error condition 2 can be avoided by the client code (like it is done in testutil_security.hpp), but it is rather easy to get this wrong (it was also wrong in the tests until some days ago), and hard to notice the effect. Error condition 3 is also easily triggered by errors in the ZAP handler code, and may remain unnoticed, and allow unauthenticated clients.

Should these cases use server assertions (and abort the process), or should they simply emit monitor events? They are not libzmq-internal errors, but still severe process-internal errors, which cannot be signalled synchronously to the libzmq-client code.

[bluca]

I think anything that can compromise security should be an assert, rather than an event.

[sigiesec]

@bluca I agree. But if we block any connections in case of a misconfiguration, security will not be compromised as well. Depending on the role of that ZeroMQ socket in the application, this might be the better solution (imagine this is only created on demand, not at startup). Using assertions, we take away the choice from the application. And I think we cannot decide this on the library level. If we use events, the application can listen to them and kill the process itself if it feels that is appropriate.

Events are a bit easier to test, if we use assertions we need death tests. And this should definitely be tested.

[bluca]

Yeah that can work too, as long as we make it 100% sure that no connection happens.

[sigiesec]

I mentioned above that I would like to explicitly enable/disable ZAP handling. But maybe we can just use the ZMQ_ZAP_DOMAIN for that. I think ZAP is enabled if and only if it is non-empty.

RFC 27 says:

The domain, which SHALL contain a string.

It does not explicitly say "non-empty string" but I think it can be concluded that this is meant, since at other places it says "MAY contain a string". If empty strings were allowed, SHALL and MAY would have the same meaning.

Any objections to this interpretation? Should this be added as a clarification to the RFC?

[bluca]

I agree that interpretation makes sense. Having a clarification in the RFC would probably be useful to avoid any possible confusion