Bug 1467898
Summary: | [RFE] Verify keys when configuring | ||
---|---|---|---|
Product: | [oVirt] ovirt-engine-metrics | Reporter: | Yedidyah Bar David <didi> |
Component: | Generic | Assignee: | Shirly Radco <sradco> |
Status: | CLOSED DEFERRED | QA Contact: | Lukas Svaty <lsvaty> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | unspecified | CC: | bugs, mperina, sradco |
Target Milestone: | --- | Keywords: | FutureFeature |
Target Release: | --- | Flags: | oourfali:
ovirt-future?
rule-engine: planning_ack? rule-engine: devel_ack? rule-engine: testing_ack? |
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2020-04-01 14:46:01 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | Infra | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 1490404 | ||
Bug Blocks: |
Description
Yedidyah Bar David
2017-07-05 12:49:30 UTC
Before the solution to bug 1438821, we did verify keys. It probably wasn't comfortable, but we did. So one can claim that current bug is a regression, not an RFE. If we decide to postpone it to some undefined far future, I'd prefer to revert bug 1438821, and keep the behavior less comfortable but more secure, suggesting some manual workaround instead for those that prefer the opposite. When adding a host, we do allow providing or fetching the host's public key fingerprint for validation. Shirly - your input? I sound to me that the engine needs to make sure the keys are valid. Yaniv K, what do you think? (In reply to Yaniv Lavi (Dary) from comment #3) > I sound to me that the engine needs to make sure the keys are valid. > Yaniv K, what do you think? Indeed, would be nice. But AFAIK we have not done it previously. How is it done elsewhere when working with Ansible? (In reply to Yaniv Kaul from comment #4) > (In reply to Yaniv Lavi (Dary) from comment #3) > > I sound to me that the engine needs to make sure the keys are valid. > > Yaniv K, what do you think? > > Indeed, would be nice. But AFAIK we have not done it previously. > How is it done elsewhere when working with Ansible? AFAIK by default Ansible don't care about host fingerprints, we just pass engine private key /etc/pki/ovirt-engine/keys/engine_id_rsa to Ansible and it connects to the host wihtout any issue. Regarding storing SSH public key instead of fingerprint, Alon tried to pushed that in https://gerrit.ovirt.org/50226 but it didn't get in as it broke V3 API because of removing fingerprint. So if needed we can finish it up although not sure if we can make it till 4.2 feature freeze. (In reply to Martin Perina from comment #5) > (In reply to Yaniv Kaul from comment #4) > > (In reply to Yaniv Lavi (Dary) from comment #3) > > > I sound to me that the engine needs to make sure the keys are valid. > > > Yaniv K, what do you think? > > > > Indeed, would be nice. But AFAIK we have not done it previously. > > How is it done elsewhere when working with Ansible? No idea, in general. > > AFAIK by default Ansible don't care about host fingerprints, we just pass > engine private key /etc/pki/ovirt-engine/keys/engine_id_rsa to Ansible and > it connects to the host wihtout any issue. AFAIU (and see) ansible by default runs ssh more-or-less like a user would. This means that the server's (ovirt hosts, in our case) public key is looked up at ~/.ssh/known_hosts (where '~' is /root, as we need root to access the engine db without credentials), and if not found, ssh (and ansible) prompt, asking the user to confirm the key. > > Regarding storing SSH public key instead of fingerprint, Must it be instead? Can't we keep both? I do not think keeping also the fingerprint will be such a big waste, hardware-resources-wise. And dropping it might in principle be a big waste of human resources (for whoever that needs to adapt existing code to the new api). > Alon tried to > pushed that in https://gerrit.ovirt.org/50226 but it didn't get in as it > broke V3 API because of removing fingerprint. So if needed we can finish it > up although not sure if we can make it till 4.2 feature freeze. The commit message explicitly says it's breaking restapi compat as it was targetted 4.0. Perhaps we can reconsider, either keeping both or making the engine extract the fingerprint out of the public key, for api compat - but I suspect it would be simpler to keep both, and probably less wasteful - assuming this data is read much more than it's written. (In reply to Yedidyah Bar David from comment #6) > (In reply to Martin Perina from comment #5) > > (In reply to Yaniv Kaul from comment #4) > > > (In reply to Yaniv Lavi (Dary) from comment #3) > > > > I sound to me that the engine needs to make sure the keys are valid. > > > > Yaniv K, what do you think? > > > > > > Indeed, would be nice. But AFAIK we have not done it previously. > > > How is it done elsewhere when working with Ansible? > > No idea, in general. > > > > > AFAIK by default Ansible don't care about host fingerprints, we just pass > > engine private key /etc/pki/ovirt-engine/keys/engine_id_rsa to Ansible and > > it connects to the host wihtout any issue. > > AFAIU (and see) ansible by default runs ssh more-or-less like a user would. > This means that the server's (ovirt hosts, in our case) public key is looked > up at ~/.ssh/known_hosts (where '~' is /root, as we need root to access the > engine db without credentials), and if not found, ssh (and ansible) prompt, > asking the user to confirm the key. > > > > > Regarding storing SSH public key instead of fingerprint, > > Must it be instead? Can't we keep both? I do not think keeping also the > fingerprint will be such a big waste, hardware-resources-wise. And dropping > it might in principle be a big waste of human resources (for whoever that > needs to adapt existing code to the new api). AFAIK there's no issue to add public key to RESTAPI (and generate fingerprint for it), but we cannot remove using figerprint only (without public key), because we would break the API. And that was the main reason why Alon failed, because we still need to support both APIv3 and APIv4. But if needed we can reopen the discussion and see what we can do ... > > > Alon tried to > > pushed that in https://gerrit.ovirt.org/50226 but it didn't get in as it > > broke V3 API because of removing fingerprint. So if needed we can finish it > > up although not sure if we can make it till 4.2 feature freeze. > > The commit message explicitly says it's breaking restapi compat as it was > targetted 4.0. Perhaps we can reconsider, either keeping both or making the > engine extract the fingerprint out of the public key, for api compat - but I > suspect it would be simpler to keep both, and probably less wasteful - > assuming this data is read much more than it's written. So if required we can start discussing it, but I don't think we will be able to finish this one till oVirt 4.2 feature freeze, but it could be possible for some 4.2.z (assuming we will get to the agreement around "breaking the API" I think we close this bug. (In reply to Shirly Radco from comment #8) > I think we close this bug. Please discuss the security implications with Didi. I do not see why this bug is now less critical than when I filed it. The security implications are obvious, and are not related to metrics, or even to oVirt. For examples, search the net for things like "StrictHostKeyChecking insecure". See also comment 1. The main reason this is less critical than the general case, is that we can probably assume that hosts connected to by metrics are also regularly used by the engine, and can hope that if something as bad as e.g. someone impersonating a host machine happens, users will notice this quickly due to other problems, hopefully evident also on the engine (e.g. VMs disappearing, stuff like that). I missed the dependent bug for this, that is targeted to 4.4. Will keep it open. Thanks. This bug didn't get any attention for a while, we didn't have the capacity to make any progress. If you deeply care about it or want to work on it please assign/target accordingly This bug didn't get any attention for a while, we didn't have the capacity to make any progress. If you deeply care about it or want to work on it please assign/target accordingly ok, closing. Please reopen if still relevant/you want to work on it. ok, closing. Please reopen if still relevant/you want to work on it. |