This is an automated email from the git hooks/post-receive script. ebourg-guest pushed a commit to branch master in repository ca-certificates-java.
commit 42bedee6326199845b1e127176552185fc5a8521 Author: Emmanuel Bourg <[email protected]> Date: Mon Mar 24 12:32:19 2014 +0000 Extracted the keystore handling code from UpdateCertificates into a distinct class --- debian/changelog | 1 + .../java/org/debian/security/KeyStoreHandler.java | 146 +++++++++++++++++++++ .../org/debian/security/UpdateCertificates.java | 133 ++----------------- .../org/debian/security/KeyStoreHandlerTest.java | 90 +++++++++++++ .../debian/security/UpdateCertificatesTest.java | 107 ++++----------- 5 files changed, 278 insertions(+), 199 deletions(-) diff --git a/debian/changelog b/debian/changelog index 36586ec..fb1d71a 100644 --- a/debian/changelog +++ b/debian/changelog @@ -6,6 +6,7 @@ ca-certificates-java (20130816) UNRELEASED; urgency=medium * Limit the memory used by java to 64M when updating the certificates (Closes: 576453) * Mavenized the project + * Code refactoring * d/control: Standards-Version updated to 3.9.5 (no changes) * Switch to debhelper level 9 diff --git a/src/main/java/org/debian/security/KeyStoreHandler.java b/src/main/java/org/debian/security/KeyStoreHandler.java new file mode 100644 index 0000000..6ed7c72 --- /dev/null +++ b/src/main/java/org/debian/security/KeyStoreHandler.java @@ -0,0 +1,146 @@ +/* + * Copyright (C) 2011 Torsten Werner <[email protected]> + * Copyright (C) 2012 Damien Raude-Morvan <[email protected]> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +package org.debian.security; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.cert.Certificate; +import java.security.cert.CertificateFactory; + +/** + * Handles read/write operations on a keystore. + */ +class KeyStoreHandler { + + /** The path of the keystore */ + private String filename; + + /** The password of the keystore */ + private char[] password; + + private KeyStore ks; + + private CertificateFactory certFactory; + + KeyStoreHandler(String filename, char[] password) throws GeneralSecurityException, IOException, InvalidKeystorePasswordException { + this.filename = filename; + this.password = password; + this.certFactory = CertificateFactory.getInstance("X.509"); + + load(); + } + + /** + * Try to open an existing keystore or create an new one. + */ + public void load() throws GeneralSecurityException, IOException, InvalidKeystorePasswordException { + KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); + File file = new File(filename); + FileInputStream in = null; + if (file.canRead()) { + in = new FileInputStream(file); + } + try { + ks.load(in, password); + } catch (IOException e) { + throw new InvalidKeystorePasswordException("Cannot open Java keystore. Is the password correct?", e); + } finally { + if (in != null) { + in.close(); + } + } + this.ks = ks; + } + + /** + * Write actual keystore content to disk. + */ + public void save() throws GeneralSecurityException, UnableToSaveKeystoreException { + try { + FileOutputStream certOutputFile = new FileOutputStream(filename); + ks.store(certOutputFile, password); + certOutputFile.close(); + } catch (IOException e) { + throw new UnableToSaveKeystoreException("There was a problem saving the new Java keystore.", e); + } + } + + /** + * Add or replace existing cert in keystore with given alias. + */ + public void addAlias(String alias, String path) throws KeyStoreException { + Certificate cert = loadCertificate(path); + if (cert == null) { + return; + } + addAlias(alias, cert); + } + + /** + * Add or replace existing cert in keystore with given alias. + */ + public void addAlias(String alias, Certificate cert) throws KeyStoreException { + if (contains(alias)) { + System.out.println("Replacing " + alias); + ks.deleteEntry(alias); + } else { + System.out.println("Adding " + alias); + } + ks.setCertificateEntry(alias, cert); + } + + /** + * Delete cert in keystore at given alias. + */ + public void deleteAlias(String alias) throws GeneralSecurityException { + if (contains(alias)) { + System.out.println("Removing " + alias); + ks.deleteEntry(alias); + } + } + + /** + * Returns true when alias exist in keystore. + */ + public boolean contains(String alias) throws KeyStoreException { + return ks.containsAlias(alias); + } + + /** + * Try to load a certificate instance from given path. + */ + private Certificate loadCertificate(String path) { + Certificate certificate = null; + try { + FileInputStream in = new FileInputStream(path); + certificate = certFactory.generateCertificate(in); + in.close(); + } catch (Exception e) { + System.err.println("Warning: there was a problem reading the certificate file " + + path + ". Message:\n " + e.getMessage()); + } + return certificate; + } +} diff --git a/src/main/java/org/debian/security/UpdateCertificates.java b/src/main/java/org/debian/security/UpdateCertificates.java index 495f37e..e4f8205 100644 --- a/src/main/java/org/debian/security/UpdateCertificates.java +++ b/src/main/java/org/debian/security/UpdateCertificates.java @@ -15,23 +15,16 @@ * You should have received a copy of the GNU General Public License along * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * */ package org.debian.security; import java.io.BufferedReader; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; import java.security.GeneralSecurityException; -import java.security.KeyStore; -import java.security.KeyStoreException; import java.security.cert.Certificate; -import java.security.cert.CertificateFactory; /** * This code is a re-implementation of the idea from Ludwig Nussel found in @@ -43,13 +36,7 @@ import java.security.cert.CertificateFactory; */ public class UpdateCertificates { - private char[] password = null; - - private String ksFilename = null; - - private KeyStore ks = null; - - private CertificateFactory certFactory = null; + private KeyStoreHandler keystore; public static void main(String[] args) throws IOException, GeneralSecurityException { String passwordString = "changeit"; @@ -61,10 +48,10 @@ public class UpdateCertificates { } try { - UpdateCertificates uc = new UpdateCertificates(passwordString, "/etc/ssl/certs/java/cacerts"); + UpdateCertificates uc = new UpdateCertificates("/etc/ssl/certs/java/cacerts", passwordString); // Force reading of inputstream in UTF-8 uc.processChanges(new InputStreamReader(System.in, "UTF8")); - uc.writeKeyStore(); + uc.finish(); } catch (InvalidKeystorePasswordException e) { e.printStackTrace(System.err); System.exit(1); @@ -74,41 +61,17 @@ public class UpdateCertificates { } } - public UpdateCertificates(final String passwordString, final String keystoreFile) throws IOException, GeneralSecurityException, InvalidKeystorePasswordException { - this.password = passwordString.toCharArray(); - this.ksFilename = keystoreFile; - this.ks = openKeyStore(); - this.certFactory = CertificateFactory.getInstance("X.509"); - } - - /** - * Try to open a existing keystore or create an new one. - */ - private KeyStore openKeyStore() throws GeneralSecurityException, IOException, InvalidKeystorePasswordException { - KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType()); - File certInputFile = new File(this.ksFilename); - FileInputStream certInputStream = null; - if (certInputFile.canRead()) { - certInputStream = new FileInputStream(certInputFile); - } - try { - ks.load(certInputStream, this.password); - } catch (IOException e) { - throw new InvalidKeystorePasswordException("Cannot open Java keystore. Is the password correct?", e); - } - if (certInputStream != null) { - certInputStream.close(); - } - return ks; + public UpdateCertificates(String keystoreFile, String password) throws IOException, GeneralSecurityException, InvalidKeystorePasswordException { + this.keystore = new KeyStoreHandler(keystoreFile, password.toCharArray()); } /** * Until reader EOF, try to read changes and send each to {@link #parseLine(String)}. */ - protected void processChanges(final Reader reader) throws IOException, GeneralSecurityException { + protected void processChanges(Reader reader) throws IOException, GeneralSecurityException { String line; - BufferedReader bufferedStdinReader = new BufferedReader(reader); - while ((line = bufferedStdinReader.readLine()) != null) { + BufferedReader in = new BufferedReader(reader); + while ((line = in.readLine()) != null) { try { parseLine(line); } catch (UnknownInputException e) { @@ -123,93 +86,25 @@ public class UpdateCertificates { * or {@link #deleteAlias(String)}. */ protected void parseLine(final String line) throws GeneralSecurityException, IOException, UnknownInputException { - assert this.ks != null; - String path = line.substring(1); String filename = path.substring(path.lastIndexOf("/") + 1); String alias = "debian:" + filename; if (line.startsWith("+")) { - Certificate cert = loadCertificate(path); - if (cert == null) { - return; - } - addAlias(alias, cert); + keystore.addAlias(alias, path); } else if (line.startsWith("-")) { - deleteAlias(alias); + keystore.deleteAlias(alias); // Remove old non-prefixed aliases, too. This code should be // removed after the release of Wheezy. - deleteAlias(filename); + keystore.deleteAlias(filename); } else { throw new UnknownInputException(line); } } /** - * Delete cert in keystore at given alias. - */ - private void deleteAlias(final String alias) throws GeneralSecurityException { - assert this.ks != null; - - if (contains(alias)) { - System.out.println("Removing " + alias); - this.ks.deleteEntry(alias); - } - } - - /** - * Add or replace existing cert in keystore with given alias. - */ - private void addAlias(final String alias, final Certificate cert) throws KeyStoreException { - assert this.ks != null; - - if (contains(alias)) { - System.out.println("Replacing " + alias); - this.ks.deleteEntry(alias); - } else { - System.out.println("Adding " + alias); - } - this.ks.setCertificateEntry(alias, cert); - } - - /** - * Returns true when alias exist in keystore. - */ - protected boolean contains(String alias) throws KeyStoreException { - assert this.ks != null; - - return this.ks.containsAlias(alias); - } - - /** - * Try to load a certificate instance from given path. + * Write the pending changes to the keystore file. */ - private Certificate loadCertificate(final String path) { - assert this.certFactory != null; - - Certificate cert = null; - try { - FileInputStream certFile = new FileInputStream(path); - cert = this.certFactory.generateCertificate(certFile); - certFile.close(); - } catch (Exception e) { - System.err.println("Warning: there was a problem reading the certificate file " + - path + ". Message:\n " + e.getMessage()); - } - return cert; - } - - /** - * Write actual keystore content to disk. - */ - protected void writeKeyStore() throws GeneralSecurityException, UnableToSaveKeystoreException { - assert this.ks != null; - - try { - FileOutputStream certOutputFile = new FileOutputStream(this.ksFilename); - this.ks.store(certOutputFile, this.password); - certOutputFile.close(); - } catch (IOException e) { - throw new UnableToSaveKeystoreException("There was a problem saving the new Java keystore.", e); - } + protected void finish() throws GeneralSecurityException, UnableToSaveKeystoreException { + keystore.save(); } } diff --git a/src/test/java/org/debian/security/KeyStoreHandlerTest.java b/src/test/java/org/debian/security/KeyStoreHandlerTest.java new file mode 100644 index 0000000..964dc50 --- /dev/null +++ b/src/test/java/org/debian/security/KeyStoreHandlerTest.java @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2012 Damien Raude-Morvan <[email protected]> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +package org.debian.security; + +import java.io.File; + +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * @author Emmanuel Bourg + * @version $Revision$, $Date$ + */ +public class KeyStoreHandlerTest { + + private String ksFilename = "./target/test-classes/tests-cacerts"; + private char[] ksPassword = "changeit".toCharArray(); + + /** + * Test a simple open then write without any modification. + */ + @Test + public void testNoop() throws Exception { + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword); + keystore.save(); + } + + /** + * Test a to open a keystore and write without any modification + * and then try to open it again with wrong password : will throw a + * InvalidKeystorePassword + */ + @Test + public void testWriteThenOpenWrongPwd() throws Exception { + try { + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword); + keystore.save(); + } catch (InvalidKeystorePasswordException e) { + fail(); + } + + try { + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, "wrongpassword".toCharArray()); + fail(); + keystore.save(); + } catch (InvalidKeystorePasswordException e) { + assertEquals("Cannot open Java keystore. Is the password correct?", e.getMessage()); + } + } + + /** + * Test a to open a keystore then remove its backing File (and replace it + * with a directory with the same name) and try to write in to disk : + * will throw an UnableToSaveKeystore + */ + @Test + public void testDeleteThenWrite() throws Exception { + try { + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword); + + // Replace actual file by a directory ! + File file = new File(ksFilename); + file.delete(); + file.mkdir(); + + // Will fail with some IOException + keystore.save(); + fail(); + } catch (UnableToSaveKeystoreException e) { + assertEquals("There was a problem saving the new Java keystore.", e.getMessage()); + } + } +} diff --git a/src/test/java/org/debian/security/UpdateCertificatesTest.java b/src/test/java/org/debian/security/UpdateCertificatesTest.java index 5e5ed27..ea68239 100644 --- a/src/test/java/org/debian/security/UpdateCertificatesTest.java +++ b/src/test/java/org/debian/security/UpdateCertificatesTest.java @@ -14,7 +14,6 @@ * You should have received a copy of the GNU General Public License along * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * */ package org.debian.security; @@ -38,79 +37,22 @@ public class UpdateCertificatesTest { private static final String REMOVE_CACERT = "-/usr/share/ca-certificates/spi-inc.org/spi-cacert-2008.crt"; private static final String ADD_CACERT = "+/usr/share/ca-certificates/spi-inc.org/spi-cacert-2008.crt"; - private String ksFilename; - private String ksPassword; + private String ksFilename = "./target/test-classes/tests-cacerts"; + private String ksPassword = "changeit"; @Before public void start() { - ksFilename = "./target/test-classes/tests-cacerts"; - ksPassword = "changeit"; // Delete any previous file File keystore = new File(ksFilename); keystore.delete(); } /** - * Test a simple open then write without any modification. - */ - @Test - public void testNoop() throws Exception { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); - uc.writeKeyStore(); - } - - /** - * Test a to open a keystore and write without any modification - * and then try to open it again with wrong password : will throw a - * InvalidKeystorePassword - */ - @Test - public void testWriteThenOpenWrongPwd() throws Exception { - try { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); - uc.writeKeyStore(); - } catch (InvalidKeystorePasswordException e) { - fail(); - } - - try { - UpdateCertificates uc = new UpdateCertificates("wrongpassword", ksFilename); - fail(); - uc.writeKeyStore(); - } catch (InvalidKeystorePasswordException e) { - assertEquals("Cannot open Java keystore. Is the password correct?", e.getMessage()); - } - } - - /** - * Test a to open a keystore then remove its backing File (and replace it - * with a directory with the same name) and try to write in to disk : - * will throw an UnableToSaveKeystore - */ - @Test - public void testDeleteThenWrite() throws Exception { - try { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); - - // Replace actual file by a directory ! - File keystore = new File(ksFilename); - keystore.delete(); - keystore.mkdir(); - - // Will fail with some IOException - uc.writeKeyStore(); - fail(); - } catch (UnableToSaveKeystoreException e) { - assertEquals("There was a problem saving the new Java keystore.", e.getMessage()); - } - } - - /** * Try to send an invalid command ("x") in parseLine : throw UnknownInput */ @Test public void testWrongCommand() throws Exception { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword); try { uc.parseLine(INVALID_CACERT); fail(); @@ -124,11 +66,12 @@ public class UpdateCertificatesTest { */ @Test public void testAdd() throws Exception { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword); uc.parseLine(ADD_CACERT); - uc.writeKeyStore(); + uc.finish(); - assertEquals(true, uc.contains(ALIAS_CACERT)); + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray()); + assertEquals(true, keystore.contains(ALIAS_CACERT)); } /** @@ -137,11 +80,12 @@ public class UpdateCertificatesTest { */ @Test public void testAddInvalidCert() throws Exception { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword); uc.parseLine("+/usr/share/ca-certificates/null.crt"); - uc.writeKeyStore(); + uc.finish(); - assertEquals(false, uc.contains("debian:null.crt")); + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray()); + assertEquals(false, keystore.contains("debian:null.crt")); } /** @@ -150,12 +94,13 @@ public class UpdateCertificatesTest { */ @Test public void testReplace() throws Exception { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword); uc.parseLine(ADD_CACERT); uc.parseLine(ADD_CACERT); - uc.writeKeyStore(); + uc.finish(); - assertEquals(true, uc.contains(ALIAS_CACERT)); + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray()); + assertEquals(true, keystore.contains(ALIAS_CACERT)); } /** @@ -163,12 +108,13 @@ public class UpdateCertificatesTest { */ @Test public void testRemove() throws Exception { - UpdateCertificates uc = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates uc = new UpdateCertificates(ksFilename, ksPassword); uc.parseLine(REMOVE_CACERT); - uc.writeKeyStore(); + uc.finish(); // We start with empty KS, so it shouldn't do anything - assertEquals(false, uc.contains(ALIAS_CACERT)); + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray()); + assertEquals(false, keystore.contains(ALIAS_CACERT)); } /** @@ -176,17 +122,18 @@ public class UpdateCertificatesTest { */ @Test public void testAddThenRemove() throws Exception { - UpdateCertificates ucAdd = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates ucAdd = new UpdateCertificates(ksFilename, ksPassword); ucAdd.parseLine(ADD_CACERT); - ucAdd.writeKeyStore(); + ucAdd.finish(); - assertEquals(true, ucAdd.contains(ALIAS_CACERT)); + KeyStoreHandler keystore = new KeyStoreHandler(ksFilename, ksPassword.toCharArray()); + assertEquals(true, keystore.contains(ALIAS_CACERT)); - UpdateCertificates ucRemove = new UpdateCertificates(ksPassword, ksFilename); + UpdateCertificates ucRemove = new UpdateCertificates(ksFilename, ksPassword); ucRemove.parseLine(REMOVE_CACERT); - ucRemove.writeKeyStore(); + ucRemove.finish(); - assertEquals(false, ucRemove.contains(ALIAS_CACERT)); + keystore.load(); + assertEquals(false, keystore.contains(ALIAS_CACERT)); } - } -- Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-java/ca-certificates-java.git _______________________________________________ pkg-java-commits mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-java-commits
