Oved Ourfali 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;
I know, but I need to push this as soon as possible, so leaving that aside for
now.
Will add a comment on that, so that once it presents itself then we would have
a clue on what is the issue.
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) {
It can fail on host name verification (although I can add a verifier that
returns true here, as I saw in the code sample you sent me).
Will do that, and remove this code.
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
It won't be implicit.
I'll write down that in case it contains one certificate, then approving will
make it trusted, and if it is a chain then the end certificate won't be
entered.
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);
We don't have to agree on that one. It can always be split up, but from viewing
the code you have no idea what can go wrong if you catch it in an "catch
(Exception) " block.
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 <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
