You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2019/05/14 14:54:39 UTC
[cxf] branch master updated: Removed some unused code from XKMS +
added some tests
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/master by this push:
new 5ce20d7 Removed some unused code from XKMS + added some tests
5ce20d7 is described below
commit 5ce20d7f3c7bf0c028f290a9a92704662adc8832
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Tue May 14 15:54:20 2019 +0100
Removed some unused code from XKMS + added some tests
---
.../xkms/x509/repo/file/FileCertificateRepo.java | 15 ++--
.../org/apache/cxf/xkms/x509/utils/X509Utils.java | 11 ---
.../xkms/x509/validator/ValidateRequestParser.java | 2 +-
.../x509/repo/file/FileCertificateRepoTest.java | 80 ++++++++++------------
4 files changed, 45 insertions(+), 63 deletions(-)
diff --git a/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepo.java b/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepo.java
index 3a34bae..47379d8 100644
--- a/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepo.java
+++ b/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepo.java
@@ -54,6 +54,7 @@ public class FileCertificateRepo implements CertificateRepo {
private static final String CRLS_PATH = "crls";
private static final String CAS_PATH = "cas";
private static final String SPLIT_REGEX = "\\s*,\\s*";
+ private static final Pattern FILE_NAME_PATTERN = Pattern.compile("[a-zA-Z_0-9-_]");
private final File storageDir;
private final CertificateFactory certFactory;
@@ -82,10 +83,7 @@ public class FileCertificateRepo implements CertificateRepo {
String name = crl.getIssuerX500Principal().getName();
try {
String path = convertIdForFileSystem(name) + ".cer";
- Pattern p = Pattern.compile("[a-zA-Z_0-9-_]");
- if (!p.matcher(path).find()) {
- throw new URISyntaxException(path, "Input did not match [a-zA-Z_0-9-_].");
- }
+ validateFilesystemPath(path);
File certFile = new File(storageDir + "/" + CRLS_PATH, path);
certFile.getParentFile().mkdirs();
@@ -148,13 +146,12 @@ public class FileCertificateRepo implements CertificateRepo {
path = cert.getSubjectDN().getName();
}
path = convertIdForFileSystem(path) + ".cer";
- validateCertificatePath(path);
+ validateFilesystemPath(path);
return path;
}
- private void validateCertificatePath(String path) throws URISyntaxException {
- Pattern p = Pattern.compile("[a-zA-Z_0-9-_]");
- if (!p.matcher(path).find()) {
+ private void validateFilesystemPath(String path) throws URISyntaxException {
+ if (!FILE_NAME_PATTERN.matcher(path).find()) {
throw new URISyntaxException(path, "Input did not match [a-zA-Z_0-9-_].");
}
}
@@ -267,7 +264,7 @@ public class FileCertificateRepo implements CertificateRepo {
public X509Certificate findByEndpoint(String endpoint) {
try {
String path = convertIdForFileSystem(endpoint) + ".cer";
- validateCertificatePath(path);
+ validateFilesystemPath(path);
File certFile = new File(storageDir.getAbsolutePath() + "/" + path);
if (!certFile.exists()) {
LOG.warn(String.format("Certificate not found for endpoint %s, path %s", endpoint,
diff --git a/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/utils/X509Utils.java b/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/utils/X509Utils.java
index bb31a84..2b54a1a 100644
--- a/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/utils/X509Utils.java
+++ b/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/utils/X509Utils.java
@@ -27,15 +27,12 @@ import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
import java.util.List;
-import java.util.UUID;
import java.util.logging.Logger;
import javax.xml.bind.JAXBElement;
import javax.xml.namespace.QName;
import org.apache.cxf.common.logging.LogUtils;
-import org.apache.cxf.xkms.model.xkms.LocateRequestType;
-import org.apache.cxf.xkms.model.xkms.LocateResultType;
import org.apache.cxf.xkms.model.xkms.UnverifiedKeyBindingType;
import org.apache.cxf.xkms.model.xmldsig.KeyInfoType;
import org.apache.cxf.xkms.model.xmldsig.ObjectFactory;
@@ -126,14 +123,6 @@ public final class X509Utils {
return keyInfo;
}
- LocateResultType createResponse(LocateRequestType request) {
- LocateResultType ret = new LocateResultType();
- ret.setId(UUID.randomUUID().toString());
- ret.setRequestId(request.getId());
- ret.setService("http://services.sopera.org/xkms/v2.0");
- return ret;
- }
-
public static void assertElementNotNull(Object element, Class<?> elementClass) {
if (element == null) {
throw new IllegalArgumentException(elementClass.getName() + " must be set");
diff --git a/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/validator/ValidateRequestParser.java b/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/validator/ValidateRequestParser.java
index e545d0f..4c57372 100644
--- a/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/validator/ValidateRequestParser.java
+++ b/services/xkms/xkms-x509-handlers/src/main/java/org/apache/cxf/xkms/x509/validator/ValidateRequestParser.java
@@ -45,7 +45,7 @@ public final class ValidateRequestParser {
public static List<X509Certificate> parse(ValidateRequestType request) {
List<X509Certificate> certs = new ArrayList<>();
- if ((request.getQueryKeyBinding()) != null && (request.getQueryKeyBinding().getKeyInfo() != null)) {
+ if (request.getQueryKeyBinding() != null && request.getQueryKeyBinding().getKeyInfo() != null) {
List<Object> keyInfoContent = request.getQueryKeyBinding().getKeyInfo().getContent();
for (Object keyInfoObject : keyInfoContent) {
if (keyInfoObject instanceof JAXBElement<?>) {
diff --git a/services/xkms/xkms-x509-handlers/src/test/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepoTest.java b/services/xkms/xkms-x509-handlers/src/test/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepoTest.java
index 00b3acd..734ce11 100644
--- a/services/xkms/xkms-x509-handlers/src/test/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepoTest.java
+++ b/services/xkms/xkms-x509-handlers/src/test/java/org/apache/cxf/xkms/x509/repo/file/FileCertificateRepoTest.java
@@ -18,27 +18,24 @@
*/
package org.apache.cxf.xkms.x509.repo.file;
-import java.io.BufferedOutputStream;
-import java.io.BufferedReader;
import java.io.File;
import java.io.FileInputStream;
-import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
-import java.io.InputStreamReader;
import java.net.URISyntaxException;
import java.security.cert.CertificateException;
import java.security.cert.CertificateFactory;
import java.security.cert.X509Certificate;
-import javax.xml.bind.DatatypeConverter;
-
import org.apache.cxf.xkms.handlers.Applications;
import org.apache.cxf.xkms.model.xkms.UseKeyWithType;
-import org.junit.Assert;
import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
public class FileCertificateRepoTest {
private static final String EXAMPLE_SUBJECT_DN = "CN=www.issuer.com, L=CGN, ST=NRW, C=DE, O=Issuer";
private static final String EXPECTED_CERT_FILE_NAME = "CN-www.issuer.com_L-CGN_ST-NRW_C-DE_O-Issuer.cer";
@@ -59,14 +56,14 @@ public class FileCertificateRepoTest {
fileRegisterHandler.saveCertificate(cert, key);
File certFile = new File(storageDir, fileRegisterHandler.getCertPath(cert, key));
- Assert.assertTrue("Cert file " + certFile + " should exist", certFile.exists());
+ assertTrue("Cert file " + certFile + " should exist", certFile.exists());
try (FileInputStream fis = new FileInputStream(certFile)) {
X509Certificate outCert = loadTestCert(fis);
- Assert.assertEquals(cert, outCert);
+ assertEquals(cert, outCert);
}
X509Certificate resultCert = fileRegisterHandler.findBySubjectDn(EXAMPLE_SUBJECT_DN);
- Assert.assertNotNull(resultCert);
+ assertNotNull(resultCert);
}
private X509Certificate loadTestCert(InputStream is) throws IOException, CertificateException {
@@ -74,49 +71,48 @@ public class FileCertificateRepoTest {
return (X509Certificate)factory.generateCertificate(is);
}
- private String read(InputStream is) throws java.io.IOException {
- StringBuilder fileData = new StringBuilder(1000);
- BufferedReader reader = new BufferedReader(new InputStreamReader(is));
- char[] buf = new char[1024];
- int numRead = 0;
- while ((numRead = reader.read(buf)) != -1) {
- String readData = String.valueOf(buf, 0, numRead);
- fileData.append(readData);
- buf = new char[1024];
- }
- reader.close();
- return fileData.toString();
- }
-
- @SuppressWarnings("unused")
- private void convertBase64ToCer(String sourcePath, String destPath) throws IOException {
- InputStream is = this.getClass().getResourceAsStream(sourcePath);
- String certString = read(is);
- is.close();
- byte[] certData = DatatypeConverter.parseBase64Binary(certString);
- File file = new File(destPath);
- try (FileOutputStream fos = new FileOutputStream(file);
- BufferedOutputStream bos = new BufferedOutputStream(fos)) {
- bos.write(certData);
- bos.close();
- }
- }
-
@Test
public void testFindBySubjectName() throws CertificateException {
File storageDir = new File("src/test/resources/store1");
- Assert.assertTrue(storageDir.exists());
- Assert.assertTrue(storageDir.isDirectory());
+ assertTrue(storageDir.exists());
+ assertTrue(storageDir.isDirectory());
FileCertificateRepo persistenceManager = new FileCertificateRepo("src/test/resources/store1");
X509Certificate resCert = persistenceManager.findBySubjectDn(EXAMPLE_SUBJECT_DN);
- Assert.assertNotNull(resCert);
+ assertNotNull(resCert);
}
@Test
public void testConvertDnForFileSystem() throws CertificateException {
String convertedName = new FileCertificateRepo("src/test/resources/store1")
.convertIdForFileSystem(EXAMPLE_SUBJECT_DN);
- Assert.assertEquals("CN-www.issuer.com_L-CGN_ST-NRW_C-DE_O-Issuer", convertedName);
+ assertEquals("CN-www.issuer.com_L-CGN_ST-NRW_C-DE_O-Issuer", convertedName);
}
+ @Test
+ public void testGetCertPath() throws CertificateException, URISyntaxException, IOException {
+ File storageDir = new File("target/teststore2");
+ storageDir.mkdirs();
+ FileCertificateRepo fileRegisterHandler = new FileCertificateRepo("target/teststore2");
+
+ InputStream is = this.getClass().getResourceAsStream("/store1/" + EXPECTED_CERT_FILE_NAME);
+ if (is == null) {
+ throw new RuntimeException("Can not find path " + is + " in classpath");
+ }
+ X509Certificate cert = loadTestCert(is);
+
+ UseKeyWithType key = new UseKeyWithType();
+ key.setApplication(Applications.SERVICE_ENDPOINT.getUri());
+ key.setIdentifier(EXAMPLE_SUBJECT_DN);
+
+ String path = fileRegisterHandler.getCertPath(cert, key);
+ assertEquals(EXPECTED_CERT_FILE_NAME, path);
+
+ // Test that we're not vulnerable to a path traversal attack
+ key.setIdentifier("../../../test.txt");
+
+ path = fileRegisterHandler.getCertPath(cert, key);
+ assertEquals(".._.._.._test.txt.cer", path);
+ }
+
+
}