Alon Bar-Lev has posted comments on this change.
Change subject: Wrap validation of fingerprint in each connect using
EngineSSHClient
......................................................................
Patch Set 1: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 369: if
(getParameters().getvds().getSSHKeyFingerprint().isEmpty()) {
Line 370: sshclient.setVds(getParameters().getvds());
Line 371: try {
Line 372:
getParameters().getvds().setSSHKeyFingerprint(sshclient.getHostFingerprint());
Line 373:
DbFacade.getInstance().getVdsStaticDao().save(getParameters().getVdsStaticData());
this logic should be within EngineSSHClient
Line 374: } catch (Exception e) {
Line 375: log.warnFormat(
Line 376: "couldn't set fingerprint for vds",
Line 377: e);
Line 381: }
Line 382: sshclient.setHardTimeout(timeout);
Line 383: sshclient.setSoftTimeout(timeout);
Line 384: sshclient.setPassword(getParameters().getPassword());
Line 385: return (SSHClient) sshclient;
Why do you need the cast?
Line 386: }
Line 387:
Line 388: /**
Line 389: * getInstalledVdsIdIfExists
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsDeploy.java
Line 904
Line 905
Line 906
Line 907
Line 908
if dialog knows _vds, why does it need the setHost?
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/gluster/GlusterUtil.java
Line 118: }
Line 119: }
Line 120:
Line 121: protected SSHClient connect(String serverName) {
Line 122: SSHClient client = new SSHClient();
why?
just don't set _vds, and if _vds is null then do not enforce. I think we talked
about this.
Always use the engine interfaces.
Line 123: Integer timeout = Config.<Integer>
GetValue(ConfigValues.ConnectToServerTimeoutInSeconds) * 1000;
Line 124: client.setHardTimeout(timeout);
Line 125: client.setSoftTimeout(timeout);
Line 126: client.setHost(serverName, SSH_PORT);
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHClient.java
Line 24: */
Line 25: public class EngineSSHClient extends SSHClient {
Line 26:
Line 27: private static final Log log =
LogFactory.getLog(EngineSSHDialog.class);
Line 28: private VDS vdsClient;
why vdsClient and not just vds?
please add _ prefix within these classes to non public / none interface so I
can maintain them better.
Line 29: /**
Line 30: * Constructor.
Line 31: */
Line 32: public EngineSSHClient() {
Line 54: super.connect();
Line 55: if (vdsClient != null) {
Line 56: String hostfp = getHostFingerprint();
Line 57: if (!vdsClient.getSSHKeyFingerprint().equals(hostfp)) {
Line 58: throw new GeneralSecurityException("Invalid
fingerprint got " +
please don't over indent, one indent per block.
please use string.format.
String actual = this.getHostFingerprint();
String expected = _vds.getSSHFingerprint();
if (isEmpty(expected)) {
_vds.setSSHFingerprint(actual);
// persist
}
else if (!actual.equals(expected)) {
throw new GeneralSecurityException(
String.format(
"Invalid fin... '%s' expected '%s'",
actual,
expected
)
);
}
Line 59:
vdsClient.getSSHKeyFingerprint() +
Line 60: " exected " +
hostfp);
Line 61: }
Line 62: }
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHClient.java
Line 31: import org.apache.sshd.SshClient;
Line 32: import org.apache.sshd.client.ServerKeyVerifier;
Line 33: import org.apache.sshd.client.future.AuthFuture;
Line 34: import org.apache.sshd.client.future.ConnectFuture;
Line 35: import org.ovirt.engine.core.utils.crypt.OpenSSHUtils;
I do not want to add more dependencies to this package.
Notice that this package can be copied as-is to other project without any
change.
Line 36:
Line 37: public class SSHClient {
Line 38: private static final String COMMAND_FILE_RECEIVE = "test -r '%2$s'
&& md5sum -b '%2$s' | cut -d ' ' -f 1 >&2 && %1$s < '%2$s'";
Line 39: private static final String COMMAND_FILE_SEND = "%1$s > '%2$s' &&
md5sum -b '%2$s' | cut -d ' ' -f 1 >&2";
--
To view, visit http://gerrit.ovirt.org/16126
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic01517a153406c8bafc672c20b0bf8686763a2f5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
