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

SSLOptions not working at all, cups 2.2.5 breaks CBC TLS suites even without DenyCBC #5145

Closed
hardfalcon opened this issue Oct 16, 2017 · 5 comments
Assignees

Comments

@hardfalcon
Copy link
Contributor

hardfalcon commented Oct 16, 2017

The DenyCBC parameter introduced in CUPS 2.2.5 for the SSLOptions directive doesn't work at all, it is simply always on because the corresponding logic in cups/tls-gnutls.c is broken in at least two ways.

First, the if() clause on line 1523 triggers when the _HTTP_TLS_DENY_CBC bit in the tls_options variable is not set, which is the exact opposite of what it is supposed to do.

Second, the tls_options variable is always zero on my machine (Archlinux) regardless of what parameters I pass to the SSLOptions directive. Unfortunately, I haven't been able to find the culprit yet for this.

In combination, this leads to the DenyCBC behaviour being always applied regardless of the actual configuration in cupsd.conf, and instead produces a priority_string of NORMAL:+VERS-TLS-ALL:-VERS-SSL3.0:-ARCFOUR-128:!ANON-DH:!AES-128-CBC:!AES-256-CBC:!CAMELLIA-128-CBC:!CAMELLIA-256-CBC:!3DES-CBC.

This effectively kicks out all ciphersuites that could be supported by non-TLS1.2 clients (only AES-GCM, AES-CCM and CHACHA20-POLY1305 suites remain). The result is that Windows 7 clients cannot print anymore. For some bizarre reason, the Windows printing system only uses TLS 1.0 and SSL 3.0 by default, and you have to fiddle with the registry to get support for TLS 1.1 and TLS 1.2.

Aside from these client-side problems, it also affects the CUPS server itself connecting to network printers which often only support outdated ciphersuites (like TLS-RSA-WITH-AES-128-CBC-SHA or TLS-RSA-WITH-AES-128-CBC-SHA). It would probably make sense to provide an option to specify the ciphersuites or at least the minimum TLS version seperately for every "upstream" printer and for "downstream" clients in general.

Besides, it would be tremendously helpful if CUPS logged the actual contents of tls_options and priority_string. Right now, the code for that seems kinda broken as well.

@michaelrsweet
Copy link
Collaborator

We will fix the CBC issue for GNU TLS.

As for separate options for different clients, printers, or servers, we are not prepared to provide that level of control as a) we haven't had anyone else ask for that (in fact, all requests have been for controls to lock things down globally with no exceptions) and b) I would be concerned about the number of support requests we'd get for that functionality.

As for logging, I'm not sure where we could be logging this information... There is no public CUPS API exposed for this, mainly because each of the TLS libraries we use is so different and the information we could supply would be less useful than what Wireshark will show you...

@hardfalcon
Copy link
Contributor Author

hardfalcon commented Oct 19, 2017

I've been able to find the culprit in the meantime. I have already written a patch for this, but I'll give it a little polishing before turning it into a PR. _httpTLSSetOptions() gets called a second time from cups/usersys.c after it has been called from cups/tls-gnutls.c, thus overriding the initial configuration.

We'll need to distinguish between the two cases of SSLOptions None and "SSLOptions is not set at all" in the context of cups/usersys.c.

As for logging, I'm not sure where we could be logging this information... There is no public CUPS API exposed for this, mainly because each of the TLS libraries we use is so different and the information we could supply would be less useful than what Wireshark will show you...

I have been wondering what all the DEBUG_printf() calls are supposed to do - at least I haven't been able to see their output anywhere. Wireshark is not very helpful when debugging server-side issues with TLS since you only see the single ciphersuite that the server selects after the client has sent its whole list of supported suites. I've used sslyze, but I really don't see why stuff like this shouldn't be logged. Besides, logging the actual ciphersuite that was negotiated for every single connection would also be helpful to enable admins to assess what ciphersuites are still needed and what suites can be thrown out.

@michaelrsweet
Copy link
Collaborator

DEBUG_printf logging is there for development, not production (and there is a pretty good performance hit when you enable it, even if logging isn't turned on...) Details are in the INSTALL.md file.

Wireshark should be able to show you the list of cipher suites that the Client is sending in the ClientHello message to the Server as well.

As for logging the cipher suite, we don't always have easy access to this information - Windows, in particular, will only give us an ID but not a string describing it, and the ID is not a constant we can just map ourselves... Even if we did try to log the cipher suite information, we don't provide fine-grained control over cipher suites and TLS versions so there typically isn't anything the admin can do with the information aside from use Wireshark to see the offered TLS versions and cipher suites, compare them against what the printer supports, and maybe decide to use one of the higher-level SSLOptions to enable what needs to be enabled for a given printer.

The only useful thing to add for logging would be to state what change needs to be made to allow the negotiation to succeed, but I don't know whether we'd have enough information all of the time to make the effort worth-while. Our strategy has been to set the defaults to what is most likely to work without posing a major security concern.

@hardfalcon
Copy link
Contributor Author

I've created a pull request for this.

I have virtually no experience at all with C, so please double-check everything. I've tested the code successfully on Archlinux with gnutls 3.5.15, but I don't have a macOS or Windows box around for testing.

I guess the patch itself should be trivial enough to not require me signing the CLA.

michaelrsweet added a commit that referenced this issue Oct 20, 2017
Also make sure that client.conf SSLOptions do not override cupsd.conf
SSLOptions, and document the (hopefully obvious) fact that Allow* is less
secure and Deny* is more secure.

- cups/http-private.h: Add "_HTTP_TLS_SET_DEFAULT" flag for options set from
  client.conf.
- cups/tls-*.c: Use new flag.
- cups/tls-gnutls.c: Fix CBC cipher suite exclusion logic, and always disable
  anonymous DH.
- cups/usersys.c: Pass new flag when calling _httpTLSSetOptions.
- man/*: Update documentation.
@michaelrsweet
Copy link
Collaborator

[master 02c88e6] Fix cipher suite selection with GNU TLS (Issue #5145)

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

No branches or pull requests

2 participants