Alon Bar-Lev has posted comments on this change.
Change subject: WIP Support foreman SSL provider
......................................................................
Patch Set 3: (4 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;
But as we discussed, if for some reason the root is within the trust store, you
do not get it out of the chain, you need to try to complete it from the JRE
trust store.
I guess this will be present itself in future.
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) {
Handshake can fail for many many reasons. It will not fail if host is untrusted
because we install the above X509TrustManager.
Can you give an example where it fails?
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/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
There is no implicit in security.
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);
Did not see this response.
Anyway, I disagree. In future when and if required you can always split it up
for the desired exceptions.
Line 101: }
Line 102: finally {
Line 103: if (in != null) {
Line 104: try {
--
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
