Skip to content

Fixes for OpenSSL-4 support#8614

Open
sumit-bose wants to merge 3 commits intoSSSD:masterfrom
sumit-bose:openssl4_fixes
Open

Fixes for OpenSSL-4 support#8614
sumit-bose wants to merge 3 commits intoSSSD:masterfrom
sumit-bose:openssl4_fixes

Conversation

@sumit-bose
Copy link
Copy Markdown
Contributor

In OpenSSL-4 ASN1_STRING is completely opaque and suitable functions must be used to return the components. Additionally some functions are changed to return const values.

There is one open issue, X509_STORE_get0_objects() is deprecated in OpenSSL-4, but still present. This will be fixed in a separate PR.

In OpenSSL-4 ASN1_STRING is an opaque object and components can only be
accessed wit the corresponding functions.
In OpenSSL-4 some functions are changed to return const values.
In OpenSSL-4 some functions are changed to return const values.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements compatibility for OpenSSL 4.0 by introducing an OSSL4_CONST macro and replacing direct access to OpenSSL structures with accessor functions. The review feedback suggests moving the duplicated OSSL4_CONST macro definition to a common header file to improve maintainability and ensure consistency across the project.

Comment thread src/lib/certmap/sss_cert_content_crypto.c
Comment thread src/p11_child/p11_child_openssl.c
@pbrezina pbrezina self-assigned this Apr 17, 2026
@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Apr 20, 2026
@alexey-tikhonov alexey-tikhonov self-assigned this Apr 20, 2026
@alexey-tikhonov alexey-tikhonov self-requested a review April 20, 2026 07:10
Copy link
Copy Markdown
Contributor

@sssd-bot sssd-bot left a comment

Choose a reason for hiding this comment

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

Review done using Claude Code / claude-opus-4-6

Functional Issues

No functional issues found. The accessor replacements (ASN1_STRING_get0_data, ASN1_STRING_length) and const-qualification changes are correct and consistent across both files. Return types match variable declarations, and the accessor functions have been available since OpenSSL 1.1.0 so there is no compatibility concern for supported OpenSSL versions.

Nits & Non-functional Issues

  1. Duplicate OSSL4_CONST macro definition — The macro is defined identically in src/lib/certmap/sss_cert_content_crypto.c:39-43 and src/p11_child/p11_child_openssl.c:40-44. It should be moved to a shared header (e.g. src/util/crypto/sss_crypto.h) to avoid divergence if the version check ever needs updating.

  2. Unnecessary cast at GEN_DIRNAME — At src/lib/certmap/sss_cert_content_crypto.c:677, current->d.directoryName (type X509_NAME *) is explicitly cast to OSSL4_CONST X509_NAME *. This cast is unnecessary because C implicitly converts pointer-to-T to pointer-to-const-T. Passing current->d.directoryName directly would work and be cleaner.

Confirmed Issues from Existing Review Comments

  1. Duplicate macro (gemini-code-assist) — Both review comments from gemini-code-assist flag the duplicated OSSL4_CONST macro across the two files. This is a valid concern (see nit #1 above). The macro should live in a single shared header.

@sumit-bose
Copy link
Copy Markdown
Contributor Author

Hi,

just a comment about the duplicated OSSL4_CONST macro. I did this somewhat on purpose because the only common header file src/util/crypto/sss_crypto.h is imo not suitable because if currently is not related to OpenSSL functionality in any way and I would like to keep it this way. Adding a new header file just to add the macro looked like too much overhead to me.

HTH

bye,
Sumit

@alexey-tikhonov
Copy link
Copy Markdown
Member

@sumit-bose, re:

  • 0d0d8b1: looks like the only type where const vs non-const is important between openssl versions is X509_EXTENSION * for X509_EXTENSION_get_data()? Looks like all other types can be 'const' with any openssl version.
  • 10bb4fe: do we still want to support OpenSSL 1.1 in 'master' branch? If not then X509_NAME * could be 'const' unconditionally.

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

Labels

Changes requested no-backport This should go to target branch only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants