Oved Ourfali has posted comments on this change.
Change subject: WIP Support foreman SSL provider
......................................................................
Patch Set 2: (9 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetProviderCertificateChainQuery.java
Line 24: HostProviderProxy proxy = ((HostProviderProxy)
ProviderProxyFactory.getInstance().create(hostProvider));
Line 25:
getQueryReturnValue().setReturnValue(chainToString(proxy.getCertificateChain()));
Line 26: }
Line 27:
Line 28: private List<String> chainToString(List<X509Certificate> chain) {
Will look into this type.
Line 29: if (chain != null) {
Line 30: List<String> stringChain = new ArrayList<String>();
Line 31: for( X509Certificate certificate : chain ) {
Line 32: StringBuilder certStringBuilder = new StringBuilder();
Line 34: certStringBuilder.append('\n');
Line 35: stringChain.add(certStringBuilder.toString());
Line 36: }
Line 37: return stringChain;
Line 38: }
no need for else here, as first part returns the string, and if not returned
then we return null.
It's a matter of style I guess.
Line 39: return null;
Line 40: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/host/provider/foreman/ForemanHostProviderProxy.java
Line 131: if
(hostUrl.getProtocol().equalsIgnoreCase(HTTPS_PROTOCOL)) {
Line 132: URL trustStorePath = new URL(FILE_URL_PREFIX +
EngineLocalConfig.getInstance().getPKIExternalTrustStore());
Line 133: String trustStorePassword =
EngineLocalConfig.getInstance().getPKIExternalTrustStorePassword();
Line 134: boolean enableSniExtension =
EngineLocalConfig.getInstance().isEnableSniExtension();
Line 135: System.setProperty("jsse.enableSNIExtension",
String.valueOf(enableSniExtension));
Will put it in the engine jboss properties.
Line 136: int hostPort = hostUrl.getPort();
Line 137: if (hostPort == -1) {
Line 138: hostPort = DEFAULT_SECURED_PORT;
Line 139: }
Line 134: boolean enableSniExtension =
EngineLocalConfig.getInstance().isEnableSniExtension();
Line 135: System.setProperty("jsse.enableSNIExtension",
String.valueOf(enableSniExtension));
Line 136: int hostPort = hostUrl.getPort();
Line 137: if (hostPort == -1) {
Line 138: hostPort = DEFAULT_SECURED_PORT;
Will check that out.
Line 139: }
Line 140: Protocol httpsProtocol = new Protocol("https", new
AuthSSLProtocolSocketFactory(null, null, trustStorePath, trustStorePassword),
hostPort);
Line 141:
httpClient.getHostConfiguration().setHost(hostUrl.getHost(), hostPort,
httpsProtocol);
Line 142: this.isSecured = true;
Line 142: this.isSecured = true;
Line 143: } else {
Line 144: int hostPort = hostUrl.getPort();
Line 145: if (hostPort == -1) {
Line 146: hostPort = DEFAULT_PORT;
didn't understand this comment. Will check for the constant, if that's what you
meant.
Line 147: }
Line 148:
httpClient.getHostConfiguration().setHost(hostUrl.getHost(), hostPort);
Line 149: }
Line 150:
objectMapper.configure(Feature.FAIL_ON_UNKNOWN_PROPERTIES, false);
Line 196: conn.connect();
Line 197: return chain;
Line 198: } catch (SSLHandshakeException e) {
Line 199: return chain;
Line 200: } catch (NoSuchAlgorithmException e) {
Having several ones give you information on what can go wrong, and one can put
other handlers for that in the future.
It's a matter of style I guess.
See no harm in doing that.
Line 201: throw new
VdcBLLException(VdcBllErrors.PROVIDER_FAILURE, e.getMessage());
Line 202: } catch (KeyManagementException e) {
Line 203: throw new
VdcBLLException(VdcBllErrors.PROVIDER_FAILURE, e.getMessage());
Line 204: } catch (IOException e) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ImportProviderCertificateChainCommand.java
Line 86: String alias = getProviderName() + "-" +
(certIndex + 1);
Line 87: ks.setCertificateEntry(alias, cert);
Line 88: }
Line 89: ks.store(out, trustStorePassword.toCharArray());
Line 90: out.close();
nice catch.
Line 91: setSucceeded(true);
Line 92: } catch (NoSuchAlgorithmException e) {
Line 93: throw new
VdcBLLException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR,
e.getMessage());
Line 94: } catch (CertificateException e) {
Line 89: ks.store(out, trustStorePassword.toCharArray());
Line 90: out.close();
Line 91: setSucceeded(true);
Line 92: } catch (NoSuchAlgorithmException e) {
Line 93: throw new
VdcBLLException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR,
e.getMessage());
see previous comment on that.
Line 94: } catch (CertificateException e) {
Line 95: throw new
VdcBLLException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR,
e.getMessage());
Line 96: } catch (IOException e) {
Line 97: throw new
VdcBLLException(VdcBllErrors.PROVIDER_IMPORT_CERTIFICATE_CHAIN_ERROR,
e.getMessage());
....................................................
File packaging/etc/pki/installCA.sh
Line 78: keytool -import -noprompt -trustcacerts -alias cacert -keypass "$PASS"
-file certs/ca.der -keystore ./.truststore -storepass "$PASS"
Line 79:
Line 80: # Generate the external truststore also trusting the CA certificate
Line 81: keytool -import -noprompt -trustcacerts -alias cacert -keypass "$PASS"
-file certs/ca.der -keystore ./.truststore_external -storepass "$PASS"
Line 82:
It makes sense for some services, like ovirt, will use the same CA, so I
thought that having it inside the external trust store as well is a good
starting point.
Anyway, if you disagree on that, then I'll create an empty one.
Line 83: echo " "
Line 84: echo "} Creating client certificates for oVirt..."
Line 85: enroll_certificate engine "$PASS"
"/C=${COUNTRY}/O=${ORG}/CN=${SUBJECT}"
Line 86: enroll_certificate apache "$PASS"
"/C=${COUNTRY}/O=${ORG}/CN=${SUBJECT}"
--
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: 2
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
