Skip to content

Pr7817 2.9#8466

Closed
alexey-tikhonov wants to merge 17 commits intoSSSD:sssd-2-9from
alexey-tikhonov:pr7817-2.9
Closed

Pr7817 2.9#8466
alexey-tikhonov wants to merge 17 commits intoSSSD:sssd-2-9from
alexey-tikhonov:pr7817-2.9

Conversation

@alexey-tikhonov
Copy link
Member

@alexey-tikhonov alexey-tikhonov commented Feb 19, 2026

No description provided.

@georgij-sudo @claude
P11_CHILD: Invert if statement to reduce code nesting
5f4391b 
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
@georgij-sudo @claude
P11_CHILD: Implement passing const args to get_pkcs11_uri
e2496dc 
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
@georgij-sudo @claude
P11_CHILD: Extract slot processing into separate function
ffcf9b0 
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
@georgij-sudo @claude
P11_CHILD: Make p11_child iterate over all slots
4530a42 
Resolves: SSSD#5905

Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 19, 2026
View reviewed changes
Copy link

@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 refactors the smart card handling logic in p11_child_openssl.c by extracting slot-specific operations into a new do_slot function. This change enables support for processing multiple smart card tokens, an improvement over the previous implementation that only handled the first token found. While the refactoring is a good step forward, I've identified two critical vulnerabilities where strcmp is called with a potentially NULL argument, which could lead to a segmentation fault. These issues must be addressed by adding appropriate NULL checks.

src/p11_child/p11_child_openssl.c
token_name, slot_name, (int) slot_id, (int) module_id,
module_file_name);

if (mode == OP_AUTH && strcmp(token_name, token_name_in) != 0) {
Copy link

@gemini-code-assist gemini-code-assist bot Feb 19, 2026

Choose a reason for hiding this comment

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

critical

The strcmp call can cause a segmentation fault if token_name_in is NULL. The token_name_in parameter can be NULL if it's not provided on the command line when invoking p11_child. You should add a NULL check for token_name_in before calling strcmp.

    if (mode == OP_AUTH && token_name_in != NULL && strcmp(token_name, token_name_in) != 0) {

src/p11_child/p11_child_openssl.c
free(mod_name);

DEBUG(SSSDBG_TRACE_ALL, "dll name: [%s].\n", mod_file_name);
if (mode == OP_AUTH && strcmp(mod_file_name, module_name_in) != 0) {
Copy link

@gemini-code-assist gemini-code-assist bot Feb 19, 2026

Choose a reason for hiding this comment

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

critical

The strcmp call can cause a segmentation fault if module_name_in is NULL. The module_name_in parameter can be NULL if it's not provided on the command line when invoking p11_child. You should add a NULL check for module_name_in before calling strcmp.

            if (mode == OP_AUTH && module_name_in != NULL && strcmp(mod_file_name, module_name_in) != 0) {

claude and others added 3 commits February 19, 2026 07:49
@claude
UTIL: fix discarded-qualifiers warning in domain_to_basedn()
54ab295 
Use a separate `const char *dot` variable for the strchr() result
on the const input string, keeping the mutable `char *p` for the
later iteration over the output buffer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
SDAP: fix discarded-qualifiers warning in are_sids_from_same_dom()
e0fa18d 
Make rid1 and rid2 `const char *` since they only hold strrchr()
results from const input strings and are used for pointer arithmetic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
SDAP: fix discarded-qualifiers warnings in sdap_parse_range()
6760a73 
Make endptr and end_range `const char *` since they only hold
strchr()/strrchr() results from const input strings. Introduce a
separate `numendptr` variable for the strtouint32() output parameter.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
claude and others added 5 commits February 19, 2026 08:48
@claude
SDAP: fix discarded-qualifiers warning in split_extra_attr()
0421e43 
Make sep `const char *` since it only holds a strchr() result
from a const input string and is used for pointer arithmetic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
AD: fix discarded-qualifiers warnings in ad_access filter parsing
a22c8ae 
Make specdelim and kwdelim `const char *` in parse_sub_filter() and
parse_filter() since they only hold strchr() results from const
input strings and are used for pointer arithmetic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
CERTMAP: fix discarded-qualifiers warnings in sss_certmap.c
c44dc20 
Make delim in get_type_prefix() and sep in expand_sid() `const char *`
since they only hold strchr()/strrchr() results from const input
strings and are used for pointer arithmetic and reading.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
TEST: replace deprecated cmocka macros in test_nss_srv.c
58910fb 
Replace will_return_always with will_return_uint_always,
will_return_count with will_return_uint_count, and
assert_in_range with assert_uint_in_range.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
TEST: replace deprecated cmocka macro in test_dyndns.c
ffc71d3 
Use will_return_ptr_always instead of will_return_always in the
will_return_nl_cache_foreach_always wrapper macro since the value
passed is a pointer, not an integer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
claude and others added 5 commits February 19, 2026 10:44
@claude
TEST: replace deprecated cmocka macro in sss_will_return_always
5d91681 
Use will_return_uint_count instead of will_return_count in the
sss_will_return_always wrapper macro in common_mock.h.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
TEST: replace deprecated cmocka macro in test_sdap.c
ce96663 
Use will_return_ptr_always instead of will_return_always in
set_entry_parse() since the value passed is a pointer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
TEST: replace deprecated cmocka macros in test_responder_cache_req.c
afc0365 
Replace will_return_always with will_return_ptr_always for pointer
arguments and will_return_uint_always for integer arguments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
KRB5: fix discarded-qualifiers warning in compare_principal_realm()
35beb65 
Make at_sign `const char *` since it only holds a strchr() result
from a const input string and is used for reading.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
BUILD: fix parallel install race for sssdlib libraries
7d161b5 
Add explicit dependency so that install-sssdlibLTLIBRARIES waits for
install-pkglibLTLIBRARIES to complete. Without this, make -jN install
can fail because libtool tries to relink sssdlib plugins (e.g.
libsss_simple.la) before their pkglib dependencies (e.g.
libsss_util.la) are installed, causing "cannot find -lsss_util".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Blocked

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexey-tikhonov @georgij-sudo @claude