Skip to content

fix: declare FIPS service-indicator funcs in curve25519.c - #3514

Open
dougch wants to merge 1 commit into
aws:mainfrom
dougch:fix-curve25519-service-indicator-include
Open

fix: declare FIPS service-indicator funcs in curve25519.c#3514
dougch wants to merge 1 commit into
aws:mainfrom
dougch:fix-curve25519-service-indicator-include

Conversation

@dougch

@dougch dougch commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Issues:

No GitHub issue. Root cause surfaced by the clang-tidy run on #3477 (RFC 8032 Duvet annotations), which added comment lines adjacent to these calls and re-triggered a pre-existing implicit-function-declaration warning. This PR fixes the underlying cause independently of that annotation work.

Description of changes:

crypto/fipsmodule/curve25519/curve25519.c calls FIPS_service_indicator_lock_state() / FIPS_service_indicator_unlock_state() but does not include the header that declares them, crypto/fipsmodule/service_indicator/internal.h. Nothing in its existing include chain reaches that header (the public <openssl/service_indicator.h> does not declare the lock/unlock functions), so the calls rely on an implicit function declaration.

This change adds #include "../service_indicator/internal.h", matching the 12 other fipsmodule translation units that use these functions (e.g. hmac.c, ec_key.c, ecdh.c, e_aesccm.c).

Call-outs:

  • No functional change today: in FIPS builds the module links as a unit and the real symbol resolves; the assumed int f() prototype vs. actual void f(void) is harmless for a no-arg call.
  • The fix matters as latent correctness: implicit declarations are a hard error under C23 / newer clang, and in non-FIPS builds the missing include bypasses the intended OPENSSL_INLINE no-op definition.

Testing:

No new tests. Covered by existing curve25519/Ed25519 suites; the change only makes an already-used declaration visible. Verified the include path matches sibling fipsmodule files.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

curve25519.c calls FIPS_service_indicator_lock_state/unlock_state but
never includes their declaring header, relying on an implicit function
declaration. Add "../service_indicator/internal.h" to match the 12
other fipsmodule files that use these functions.

Implicit declarations are a hard error under C23/newer clang, and in
non-FIPS builds this bypasses the intended OPENSSL_INLINE no-op.
@dougch
dougch requested a review from a team as a code owner September 10, 2026 21:59
@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security ReviewView Report

Please review before merging.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.40%. Comparing base (e4117b4) to head (6520bfe).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3514      +/-   ##
==========================================
+ Coverage   78.36%   78.40%   +0.03%     
==========================================
  Files         700      700              
  Lines      125684   125684              
  Branches    17377    17377              
==========================================
+ Hits        98490    98539      +49     
+ Misses      26322    26272      -50     
- Partials      872      873       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

4 participants