Oved Ourfali has posted comments on this change. Change subject: WIP Support foreman SSL provider ......................................................................
Patch Set 3: (7 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/BaseProviderProxy.java Line 74: ); Line 75: conn = (HttpsURLConnection) url.openConnection(); Line 76: conn.setSSLSocketFactory(ctx.getSocketFactory()); Line 77: conn.connect(); Line 78: return chain; No. Will check that in the future. For now we don't expect providers that are known/trusted by the JRE. Line 79: } catch (SSLHandshakeException e) { Line 80: return chain; Line 81: } catch (NoSuchAlgorithmException e) { Line 82: handleException(VdcBllErrors.PROVIDER_FAILURE, e); Line 75: conn = (HttpsURLConnection) url.openConnection(); Line 76: conn.setSSLSocketFactory(ctx.getSocketFactory()); Line 77: conn.connect(); Line 78: return chain; Line 79: } catch (SSLHandshakeException e) { As the handshake will fail if the host isn't trusted, host name isn't verified, etc, but in this case we would also like to get the chain back, and make it trusted. Line 80: return chain; Line 81: } catch (NoSuchAlgorithmException e) { Line 82: handleException(VdcBllErrors.PROVIDER_FAILURE, e); Line 83: } catch (KeyManagementException e) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ExternalTrustStoreInitializer.java Line 10: import org.ovirt.engine.core.utils.EngineLocalConfig; Line 11: Line 12: public class ExternalTrustStoreInitializer { Line 13: Line 14: private static final String FILE_URL_PREFIX = "file:"; Weird... it worked well even without it.... Will change that. Line 15: Line 16: public static String getTrustStorePath() { Line 17: File varDir = EngineLocalConfig.getInstance().getVarDir(); Line 18: return varDir + "/" + "external_truststore"; Line 36: // Passing null stream will create a new empty trust store Line 37: trustStore.load(null, getTrustStorePassword().toCharArray()); Line 38: trustStore.store(out, getTrustStorePassword().toCharArray()); Line 39: } catch (Exception e) { Line 40: e.printStackTrace(); Autogenerated code. Will fix. Line 41: } Line 42: finally { Line 43: if (out != null) { Line 44: try { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ImportProviderCertificateChainCommand.java Line 80: ks = KeyStore.getInstance(KeyStore.getDefaultType()); Line 81: ks.load(in, trustStorePassword.toCharArray()); Line 82: out = new FileOutputStream(trustStore); Line 83: // In case there is only one certificate, we insert it. Line 84: // Otherwise, we need to insert the entire chain except the end certificate If the user authorizes a chain with one certificate, then it is implied that he wants this trusted... don't want to make the UI over-complicated as I see no need to do that in this case. Line 85: int numberOfCertificatesToInsert = chain.size() == 1 ? 1 : chain.size() - 1; Line 86: for (int certIndex = 0; certIndex < numberOfCertificatesToInsert; ++certIndex) { Line 87: Certificate certificate = chain.get(certIndex); Line 88: String alias = Guid.NewGuid().toString(); Line 96: handleException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR, e); Line 97: } catch (IOException e) { Line 98: handleException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR, e); Line 99: } catch (KeyStoreException e) { Line 100: handleException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR, e); Already answered that one. It is a matter of style, and I do see a value in that, allowing to see what exceptions can be expected here. Can be useful when maintaining the code in the future. Line 101: } Line 102: finally { Line 103: if (in != null) { Line 104: try { .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/EngineLocalConfig.java Line 62: public boolean isHttpEnabled() { Line 63: return getBoolean("ENGINE_HTTP_ENABLED"); Line 64: } Line 65: Line 66: public boolean isEnableSniExtension() { Yes. Was removed in the next patch set... but I didn't push it fast enough :-) nice catch :-) Line 67: return getBoolean("ENGINE_ENABLE_SNI_EXTENSION"); Line 68: } Line 69: Line 70: public int getHttpPort() { -- To view, visit http://gerrit.ovirt.org/15128 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35343409d74a4f90aae726b46781f27ce08a981a Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches