Skip to content

cmake: fix pkg-config-based detection in FindGSS.cmake #14430

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

Closed
wants to merge 5 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Aug 7, 2024

Before this patch pkg-config-based detection was ignored, and used
solely as a path hint for native detection.

  • fix pkg_search_module() result prefix to match what code expects:
    _GSS (was: _GSS_PKG). Update variable that were in sync with old
    prefix.

  • update the pkg-config codepath to use _GSS_MODULE_NAME to detect
    GSS flavour. This requires CMake 3.16.
    Otherwise fall back to the old method. (The old method doesn't seem to
    work anymore (?) as of CMake 3.30.1. Documented
    <prefix>_<modulename>_VERSION variable is defined, but empty.)

  • update the pkg-config codepath to use _GSS_VERSION set by CMake.
    Resort to the old code when this variable is empty. (The old code
    doesn't seem to work anymore (?) as of CMake 3.30.1)

  • fix pkg-config codepath to set the documented result variables.

  • align native detection variable names with those generated by
    pkg_search_module() in the pkg-config codepath.

  • GHA/macos: enable GSS Heimdal in a cmake job.
    Uses the native detection.

  • GHA/linux: enable GSS Heimdal in cmake and autotools jobs.
    CMake uses pkg-config-based detection.

  • suppress test 2077 and 2078 results on Linux + Heimdal.

    FAIL-IGNORED 2077: 'curl --fail --negotiate to unauthenticated service fails' HTTP, HTTP GET, GSS-API
    FAIL-IGNORED 2078: 'curl --negotiate should not send empty POST request only' HTTP, HTTP GET, GSS-API
    

    Failing with valgrind errors in both autotools and cmake builds:
    https://github.com/curl/curl/actions/runs/10282222581/job/28453472068?pr=14430#step:38:3638
    https://github.com/curl/curl/actions/runs/10282222581/job/28453473398?pr=14430#step:38:7831

Closes #14430

@github-actions github-actions bot added the CI Continuous Integration label Aug 7, 2024
@vszakats vszakats force-pushed the cm-findgss-fixup branch 2 times, most recently from 98e0ded to 17679e9 Compare August 7, 2024 03:18
- fix `pkg_search_module()` result prefix to match what code expects:
  `_GSS` (was: `_GSS_PKG`). Update variable that were in sync with old
  prefix.

- update the pkg-config codepath to use `_GSS_MODULE_NAME` to detect
  GSS flavour. This requires CMake 3.16.
  Otherwise fall back to the old method. (The old method doesn't seem to
  work anymore (?) as of CMake 3.30.1. Contrary to CMake docs,
  `<prefix>_<modulename>_VERSION` variable is left empty.)

- update the pkg-config codepath to use `_GSS_VERSION` set by CMake.
  Resort to the old code when this variable is empty. (The old code
  doesn't seem to work anymore (?) as of CMake 3.30.1)

Closes #xxxxx
@vszakats vszakats force-pushed the cm-findgss-fixup branch 2 times, most recently from 16bda97 to aeb1d46 Compare August 7, 2024 11:47
GHA/macos: try fixing GSS heimdal detection
silence 2077, 2078 for cmake GSS heimdal job

enable heimdal for autotools job

extend TFLAGS exception for autotools heimdal builds
@vszakats vszakats closed this in 1467597 Aug 7, 2024
@vszakats vszakats deleted the cm-findgss-fixup branch August 7, 2024 13:15
vszakats added a commit that referenced this pull request Jun 30, 2025
Replace the `libstubgss.so`-based overload solution with one built into
libcurl at compile-time.

The previous, `LD_PRELOAD`-based, solution was non-portable, allowlisted
for Linux, BSD and Solaris. It also required non-debug builds, which
turned out to be an accidental condition:
7d342c7. It also required a curl tool
built against a shared libcurl. Detecting this condition wasn't always
accurate, e.g. with certain cmake configurations.

The overload solution also didn't work on macOS, though it theoretically
should have:
- #17653
- #2394

Experiments on making the overload solution work in more envs:
- #17759
  That revealed that it also did not work on NetBSD, in CI.

The replacement solution is overloading the necessary GSS-API functions
for test 2056 and 2057 at compile time. It requires a debug-enabled curl
build (due to its insecure nature).

This makes these tests run on all platforms. Including most GSS jobs in
CI, that are running tests. (the exception is old-linux, non-debug jobs,
where it felt overkill to enable debug for this.)

The refactored GSS stub code needs to overload less than before because
it's free to use the official GSS API. (This didn't work with
the overload solution on Alpine for example). It can also use libcurl
functions, allowing to replace `snprintf()` with `msnprintf()`.

OS/400 is also overloading GSS API functions. I haven't tested how this
works after this PR. In theory it should, because this PR doesn't rely
on preprocessor overrides.

Note that for future GSS tests, it may be necessary to stub these GSS
API functions: `gss_inquire_context()`, `gss_unwrap()`, `gss_wrap()`.
They are on codepaths not (yet) touched by tests.

Also:
- stub-gss: check for token buffer overrun.
- stub-gss: replace size macros with `sizeof()`.
- GHA: enable debug for some jobs with GSS.
- GHA/linux: ignore results for 2056 and 2057 in the valgrind job.
  They leak the same way as seen with 2077 and 2078.
  Ref: 7020ba7 #17462
  Ref: 1467597 #14430
- GHA/linux: fix to ignore `gss_import_name()` leaks in valgrind builds.
  only.
- lib/vauth/krb5_gssapi: reduce variable scope.
- lib/vauth/spnego_gssapi: reduce variable scope.
- tests/libtest: drop code and build logic dealing with `libstubgss`.
- runtests:
  - drop `ld_preload` feature.
  - drop special handling of `LD_PRELOAD` env in tests.
  - drop logic dealing with shared curl tool detection.
  - drop `LD_PRELOAD` envs from tests.

Follow-up to 56d949d #1687

Closes #17752
vszakats added a commit to vszakats/curl that referenced this pull request Jul 3, 2025
vszakats added a commit to vszakats/curl that referenced this pull request Jul 3, 2025
When processing `--cflags` received from `krb5-config` for `gssapi`:

- fix to not break on multiple `-I` options. Before this patch only
  the first `-I` option was processed as a header directory, subsequent
  ones ended up in C flags as a raw directory, without the `-I` part.
  Follow-up to 558814e

- fix to not duplicate C flags.
  Regression from 1467597 curl#14430

- drop local variable `_val` by re-using `_flag`.

- tidy up comments.

Ref: curl#17802 (comment)

Closes curl#17805
vszakats added a commit that referenced this pull request Jul 3, 2025
When processing `--cflags` received from `krb5-config` for `gssapi`:

- fix to not break on multiple `-I` options. Before this patch only
  the first `-I` option was processed as a header directory, subsequent
  ones ended up in C flags as a raw directory, without the `-I` arg.
  Follow-up to 558814e

- fix to not duplicate C flags.
  Regression from 1467597 #14430

- drop local variable `_val` by re-using `_flag`.

- tidy up comments.

Ref: #17802 (comment)

Closes #17805
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant