Skip to content

Fix ignored GnuTLS options when SYSTEM priorities are unavailable - #1701

Open
Shubham-Padkonde wants to merge 2 commits into
OpenPrinting:masterfrom
Shubham-Padkonde:fix/gnutls-missing-system-priority
Open

Shubham-Padkonde wants to merge 2 commits into
OpenPrinting:masterfrom
Shubham-Padkonde:fix/gnutls-missing-system-priority

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

When GnuTLS has no named SYSTEM priority, @SYSTEM,NORMAL:... is rejected: both names are interpreted as configured priorities, so NORMAL is not a built-in fallback there. CUPS discards that error and can negotiate TLS 1.3 despite MaxTLS1.2, or accept TLS 1.2 despite MinTLS1.3.

Retry an unresolved priority string without the @SYSTEM, prefix, retaining the requested protocol and cipher options. Check the result in both the direct and legacy GnuTLS API paths and release the session/credentials if priority setup still fails. A valid named system policy and NoSystem retain their existing behavior.

Add eight local TLS handshake checks and run them in the GnuTLS CI job. The tests create temporary certificates/configuration and compile a small client against the built CUPS library; they require Python 3, a C compiler, and the openssl command. No external server or printer is contacted.

Validation on Ubuntu 22.04, GCC 11.4 and GnuTLS 3.7.3:

  • Before the fix, the missing-system maximum/minimum TLS checks fail (three failures); the fixed implementation passes all eight cases.
  • Full build and eight checks pass with gnutls_priority_set_direct and with ac_cv_func_gnutls_priority_set_direct=no to exercise the older API path.
  • Full make test is not green in this environment: the external Google OIDC certificate-validation check fails and the scheduler reports 35 errors instead of 33 because Avahi is not running. The unchanged-source legacy-API build reproduces the same failures. Other scheduler command, restart and job-history checks pass.
  • Python formatting/lint and git diff --check pass.

This addresses the missing-system-policy part of #1677. The separate observation about the default maximum re-enabling protocols restricted by a valid system policy remains outside this patch, so this PR does not close the entire issue. macOS, Windows, physical printers, and older GnuTLS releases were not tested locally; the legacy API was exercised on GnuTLS 3.7.3.

Assisted by Codex/GPT-6 for investigation, coding, and validation.

Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>
Comment thread cups/tls-gnutls.c

#ifdef HAVE_GNUTLS_PRIORITY_SET_DIRECT
gnutls_priority_set_direct(http->tls, priority_string, NULL);
status = gnutls_priority_set_direct(http->tls, priority_string, NULL);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be better to have the check for @System before we assign it into priority string, instead of handling it here, although it adds new set of HAVE_GNUTLS_PRIORITY_SET_DIRECT ifdef.

Just do not forget to initialize the string with NULL terminator in case of error, as I did :( .

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in db6e871: @SYSTEM is now probed before the priority string is built (with set_direct or priority_init/deinit depending on HAVE_GNUTLS_PRIORITY_SET_DIRECT), priority_string is initialized to an empty string first, and the retry after failure is removed. test/testssloptions.py passes all 8 cases with GnuTLS 3.7.3 on both the set_direct and the priority_init code paths.

Probe @System first and only prepend it when it is configured, instead of
retrying without it after the combined priority string fails.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 this pull request may close these issues.

2 participants