Alon Bar-Lev has posted comments on this change. Change subject: core: Store only single certificate ......................................................................
Patch Set 11: (5 comments) http://gerrit.ovirt.org/#/c/33717/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ExternalTrustStoreInitializer.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ExternalTrustStoreInitializer.java: Line 26 Line 27 Line 28 Line 29 Line 30 why do you need to load? the getInstance should be sufficient. Line 38 Line 39 Line 40 Line 41 Line 42 why is this related to the trust store? please add a new variable for the password of the external providers store. Line 17: public class ExternalTrustStoreInitializer { Line 18: Line 19: private static final Logger log = LoggerFactory.getLogger(ExternalTrustStoreInitializer.class); Line 20: Line 21: public static void init() { I do not like we create a file if not required. We should create only when we save the keystore and file is missing. Line 22: if (!EngineLocalConfig.getInstance().getExternalProvidersTrustStore().exists()) { Line 23: try (OutputStream out = Line 24: new FileOutputStream(EngineLocalConfig.getInstance().getExternalProvidersTrustStore())) { Line 25: String password = EngineLocalConfig.getInstance().getPKITrustStorePassword(); Line 57: throw new RuntimeException(e); Line 58: } Line 59: } Line 60: Line 61: public static void setCertificateChain(List<? extends Certificate> chain) throws CertificateEncodingException, addCertificateChain Line 62: KeyStoreException { Line 63: KeyStore ks = ExternalTrustStoreInitializer.getTrustStore(); Line 64: Certificate certificate = chain.get(chain.size() - 1); Line 65: String alias = Integer.toString(certificate.getEncoded().hashCode()); Line 63: KeyStore ks = ExternalTrustStoreInitializer.getTrustStore(); Line 64: Certificate certificate = chain.get(chain.size() - 1); Line 65: String alias = Integer.toString(certificate.getEncoded().hashCode()); Line 66: ks.setCertificateEntry(alias, certificate); Line 67: setTrustStore(ks); I would have expected: saveTrustStore( loadTrustStore().setCertificate( Integer.toString(certificate.getEncoded().hashCode()), chain.get(chain.size() - 1) ) ) Line 68: } -- To view, visit http://gerrit.ovirt.org/33717 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic9bd8cd7f913cf23eca839452b6e113f749966f7 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
