TEST: Current value of ssh_hash_known_hosts causes error in the default configuration in FIPS mode #5925
Conversation
dparmar18
force-pushed
the
test_ssh_hash_knownhosts
branch
from
df2bbfc
to
117129c
Compare
dparmar18
force-pushed
the
test_ssh_hash_knownhosts
branch
from
da6a927
to
c3f081f
Compare
dparmar18
force-pushed
the
test_ssh_hash_knownhosts
branch
from
e901177
to
38da67e
Compare
| ssh_hash_known_hosts | ||
| """ | ||
| tools = sssdTools(multihost.client[0]) | ||
| server_host = multihost.master[0].sys_hostname |
There was a problem hiding this comment.
Where is this host added at IPA server?
dparmar18
force-pushed
the
test_ssh_hash_knownhosts
branch
2 times, most recently
from
ce2e1b2
to
b0a2085
Compare
src/tests/multihost/ipa/test_misc.py
Outdated
| if "ssh_hash_known_hosts" in sssd_conf: | ||
| ssh_section = "ssh" | ||
| ssh_param = {"ssh_hash_known_hosts": ""} | ||
| tools.sssd_conf(ssh_section, ssh_param, action="delete") |
There was a problem hiding this comment.
Can't you use action="delete" unconditionally (without check option is present first)? What happens if you try to delete non-existent option?
There was a problem hiding this comment.
If I try to delete that doesn't exist then it will throw exception
There was a problem hiding this comment.
Doesn't it support "nothrow" mode/arg?
But even if it throws, it would be easier to try/catch than to check.
There was a problem hiding this comment.
The test case will be skipped upon encountering an exception therefore as discussed I'll have to stick with this.
There was a problem hiding this comment.
How does is it work in case exception is handled in place?
There was a problem hiding this comment.
Usually, it would skip from there but I found a workaround for expecting an exception and still continue the execution, I'll push it if it succeeds.
There was a problem hiding this comment.
What "it"?
If you handle exception in place it doesn't propagate anywhere.
…lt configuration in FIPS mode. Explanation - In SSSD the default value for ssh_hash_known_hosts is set to true, It should be changed to false for consistency with the OpenSSH setting that does not hashes host names by default Verifies Issue: SSSD#5848 Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2014249
dparmar18
force-pushed
the
test_ssh_hash_knownhosts
branch
from
dc20f7c
to
a6d06ef
Compare
src/tests/multihost/ipa/test_misc.py
Outdated
| multihost.master[0].run_command("ipa host-mod %s --sshpubkey=" | ||
| "\"$(cat /tmp/ssh_host0003_rsa.pub)\" " | ||
| "--updatedns" | ||
| % multihost.client[0].sys_hostname) |
There was a problem hiding this comment.
I don't understand this.
-
You assign a key to
client[0].sys_hostnamebut later test looks formaster[0].sys_hostname. This doesn't make sense.
Probably this works because IPA master has own key always set? I don't know this. Could you please check? -
ssh-keyscanshould be used to figure out key of a host, not a new random key generated and assigned in IPA db.
(but answer to 1) is more important)
There was a problem hiding this comment.
- I'm not assigning a key to
client[0].sys_hostname, here I'm running this command on the server that adds a key bound with the client host. It will be likeipa host-mod client.ipa.test --sshpubkey="$(cat /tmp/ssh_host0003_rsa.pub)" --updatednswhere client.ipa.test is a client i.eclient[0].sys_hostname
There was a problem hiding this comment.
You can call it "bound" instead of "assign" but that doesn't change a thing.
Later test looks for server_host (if server_host in known_hosts.stdout_text ...), not for a "client.ipa.test".
There was a problem hiding this comment.
yes because SSH is being done on client:
multihost.client[0].run_command(cmd,stdin_text="Secret123",raiseonerr=False)
There was a problem hiding this comment.
yes because SSH is being done on client:
multihost.client[0].run_command(cmd,stdin_text="Secret123",raiseonerr=False)
It doesn't matter where ssh is being executed - every node will receive the same map from the server.
This is just a mean to trigger known-hosts list update.
Your test makes sure client's key (fingerprint) is specified in the list, but searches for server's key.
There was a problem hiding this comment.
Working on this now, thanks for acknowledging.
|
And it still fails |
I hope it passes now, have pushed the required changes |
|
Please rebase to the latest master branch. |
dparmar18
force-pushed
the
test_ssh_hash_knownhosts
branch
3 times, most recently
from
eca0bd6
to
2124ffd
Compare
|
This PR had conflicts while rebasing, therefore here is the new PR with everything sorted: #5996 |
alexey-tikhonov
added
the
superseded
Explanation
It should be changed to false for consistency with the OpenSSH
setting that does not hashes hostnames by default
Verifies
Issue: #5848
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2014249