Skip to content
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

Allow multiple services associated to the same port #7790

Closed
wants to merge 4 commits into from

Conversation

scabrero
Copy link
Contributor

@scabrero scabrero commented Jan 8, 2025

When the backend has multiple services are associated to the same port then concurrent NSS lookups fail.

Example:

$ cat getent.sh 
#!/bin/bash
getent services msa &
pid1=$!
getent services msb &
pid2=$!
getent services msc &
pid3=$!
wait $pid1
rc1=$?
wait $pid2
rc2=$?
wait $pid3
rc3=$?
echo "Return code for msa: $rc1"
echo "Return code for msb: $rc2"
echo "Return code for msc: $rc3"
# bash getent.sh 
msc                   23456/tcp
Return code for msa: 2
Return code for msb: 2
Return code for msc: 0

RFC6335 Section 5 allows more than one service associated with a particular port, but sysdb_store_service() does not and replaces the previously cached entries for that port, causing the NSS queries to fail.

This MR allows storing multiple entries for the same port, returning only the first result on service-by-port lookup.

RFC6335 Section 5 allows more than one service associated with a
particular port.

In this case always return the first result returned by
sysdb_getservbyport().

Signed-off-by: Samuel Cabrero <[email protected]>
RFC6335 section 5 allow multiple services associated to the same
transport and port. This commit allows storing multiple service entries
in the cache for the same port and the lookup functions will return all
matching entries. The cache_req plugin will pick the first result.

Signed-off-by: Samuel Cabrero <[email protected]>
Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

Thank you, ack.

We are currently trying to get rid of intg tests and we are converting them to tests/system. Would you like to convert it yourself as part of this pull request? It may however be little bit more complicated as it needs some new bits in https://github.com/SSSD/sssd-test-framework

I am fine approving it as is though.

Copy link
Contributor

@aplopez aplopez left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@alexey-tikhonov
Copy link
Member

Pushed PR: #7790

  • master
    • 56ef896 - INTG-TESTS: Add Tests for service by name and by port lookups
    • b1c1649 - SYSDB: Allow multiple services associated to the same port
    • f911e38 - SYSDB: Use temporary memory context to allocate the results
    • 2e6fdb6 - CACHE_REQ: always return the first result in service by port lookups
  • sssd-2-10
    • a84e2f4 - INTG-TESTS: Add Tests for service by name and by port lookups
    • ea4b933 - SYSDB: Allow multiple services associated to the same port
    • cb03f09 - SYSDB: Use temporary memory context to allocate the results
    • 143dd27 - CACHE_REQ: always return the first result in service by port lookups

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

Successfully merging this pull request may close these issues.

4 participants