alien11689 commented on code in PR #60:
URL: https://github.com/apache/aries-rsa/pull/60#discussion_r3046906120
##########
provider/tcp/src/main/java/org/apache/aries/rsa/provider/tcp/TcpProvider.java:
##########
@@ -54,14 +65,18 @@
RemoteConstants.REMOTE_INTENTS_SUPPORTED + "=osgi.basic",
RemoteConstants.REMOTE_INTENTS_SUPPORTED + "=osgi.async",
RemoteConstants.REMOTE_CONFIGS_SUPPORTED + "=" +
TcpProvider.TCP_CONFIG_TYPE //
-})
+ },
+ configurationPid="org.apache.aries.rsa.provider.tcp")
Review Comment:
should we make a const for this property also?
##########
provider/tcp/src/main/java/org/apache/aries/rsa/provider/tcp/Config.java:
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.aries.rsa.provider.tcp;
+
+import java.util.Map;
+import java.util.UUID;
+
+/**
+ * A convenience class for extracting endpoint configuration properties,
+ * provider properties and defaults.
+ */
+public class Config {
+
+ // endpoint service properties
+ static final String PORT = "aries.rsa.port";
+ static final String HOSTNAME = "aries.rsa.hostname";
+ static final String BIND_ADDRESS = "aries.rsa.bindAddress";
+ static final String ID = "aries.rsa.id";
+ static final String THREADS = "aries.rsa.numThreads";
+ static final String TIMEOUT = "osgi.basic.timeout";
+
+ // provider component properties
+ static final String KEYSTORE = "aries.rsa.keyStore";
+ static final String TRUSTSTORE = "aries.rsa.trustStore";
+ static final String KEYSTORE_PASSWORD = "aries.rsa.keyStorePassword";
+ static final String TRUSTSTORE_PASSWORD = "aries.rsa.trustStorePassword";
+ static final String KEY_ALIAS = "aries.rsa.keyAlias";
+ static final String MTLS = "aries.rsa.mtls";
+
+ static final int DYNAMIC_PORT = 0;
+ static final int DEFAULT_TIMEOUT_MILLIS = 300000;
+ static final int DEFAULT_NUM_THREADS = 10;
+
+ private final Map<String, Object> props;
+ private final String uuid = UUID.randomUUID().toString(); // fallback id
Review Comment:
instead of uuid we can call the field`fallbackId`
##########
provider/tcp/src/test/java/org/apache/aries/rsa/provider/tcp/TcpProviderTLSTest.java:
##########
@@ -0,0 +1,159 @@
+package org.apache.aries.rsa.provider.tcp;
+
+import org.apache.aries.rsa.provider.tcp.myservice.MyService;
+import org.apache.aries.rsa.provider.tcp.myservice.MyServiceImpl;
+import org.apache.aries.rsa.spi.Endpoint;
+import org.apache.aries.rsa.spi.ImportedService;
+import org.apache.aries.rsa.util.EndpointHelper;
+import org.easymock.EasyMock;
+import org.junit.After;
+import org.junit.Test;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceException;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.aries.rsa.provider.tcp.TcpProviderTest.getFreePort;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringStartsWith.startsWith;
+import static org.junit.Assert.assertEquals;
+
+public class TcpProviderTLSTest {
+
+ private static String
+ KEYSTORE =
TcpProviderTest.class.getResource("/keystore.p12").getPath(),
+ KEYSTORE2 =
TcpProviderTest.class.getResource("/keystore2.p12").getPath(),
+ TRUSTSTORE =
TcpProviderTest.class.getResource("/truststore.p12").getPath(),
+ KEYSTORE_PASSWORD = "password",
+ TRUSTSTORE_PASSWORD = "password1";
+
+ private MyService myServiceProxy;
+ private Endpoint ep;
+ private ImportedService importedService;
+
+ public void test(Map<String, Object> providerProps) throws IOException {
Review Comment:
should it be private? I thought first that we are using some old test
framework, not Junit 4
##########
provider/tcp/src/test/java/org/apache/aries/rsa/provider/tcp/TcpProviderTLSTest.java:
##########
@@ -0,0 +1,159 @@
+package org.apache.aries.rsa.provider.tcp;
+
+import org.apache.aries.rsa.provider.tcp.myservice.MyService;
+import org.apache.aries.rsa.provider.tcp.myservice.MyServiceImpl;
+import org.apache.aries.rsa.spi.Endpoint;
+import org.apache.aries.rsa.spi.ImportedService;
+import org.apache.aries.rsa.util.EndpointHelper;
+import org.easymock.EasyMock;
+import org.junit.After;
+import org.junit.Test;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceException;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.aries.rsa.provider.tcp.TcpProviderTest.getFreePort;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringStartsWith.startsWith;
+import static org.junit.Assert.assertEquals;
+
+public class TcpProviderTLSTest {
+
+ private static String
+ KEYSTORE =
TcpProviderTest.class.getResource("/keystore.p12").getPath(),
Review Comment:
you can provide here the commands you used to create the files - just in
case we want to add more
##########
provider/tcp/src/main/java/org/apache/aries/rsa/provider/tcp/TcpProvider.java:
##########
@@ -77,6 +92,50 @@ private static <T> Set<T> union(Collection<T>...
collections) {
return union;
}
+ private void initSocketFactories(Config config) {
+ try {
+ mtls = config.isMtls();
+ String keyStore = config.getKeyStore();
+ String trustStore = config.getTrustStore();
+ if (mtls) {
+ // both client and server need the keystore and truststore
+ if (keyStore == null || keyStore.isEmpty() || trustStore ==
null || trustStore.isEmpty())
+ throw new RuntimeException("MTLS requires keystore and
truststore");
+ SSLContext context = NetUtil.createSSLContext(
+ keyStore, config.getKeyStorePassword(),
+ trustStore, config.getTrustStorePassword(),
+ config.getKeyAlias());
+ serverSocketFactory =
NetUtil.createMTLSServerSocketFactory(context);
+ socketFactory = context.getSocketFactory();
+ } else {
+ if (keyStore == null || keyStore.isEmpty()) {
+ serverSocketFactory = ServerSocketFactory.getDefault(); //
plain sockets
+ } else {
+ // server only needs keystore
+ SSLContext context = NetUtil.createSSLContext(
+ keyStore, config.getKeyStorePassword(), null, null,
config.getKeyAlias());
+ serverSocketFactory = context.getServerSocketFactory();
+ }
+ if (trustStore == null || trustStore.isEmpty()) {
+ socketFactory = SocketFactory.getDefault(); // plain
sockets
+ } else {
+ // client only needs truststore
+ SSLContext context = NetUtil.createSSLContext(
+ null, null, trustStore,
config.getTrustStorePassword(), null);
+ socketFactory = context.getSocketFactory();
+ }
+ }
+ } catch (NoSuchAlgorithmException | KeyManagementException |
UnrecoverableKeyException | IOException |
+ KeyStoreException | CertificateException e) {
+ throw new RuntimeException("Error initializing SSL Context", e);
Review Comment:
can we throw some better exception? I mean our exception
##########
provider/tcp/src/test/java/org/apache/aries/rsa/provider/tcp/TcpProviderTLSTest.java:
##########
@@ -0,0 +1,159 @@
+package org.apache.aries.rsa.provider.tcp;
+
+import org.apache.aries.rsa.provider.tcp.myservice.MyService;
+import org.apache.aries.rsa.provider.tcp.myservice.MyServiceImpl;
+import org.apache.aries.rsa.spi.Endpoint;
+import org.apache.aries.rsa.spi.ImportedService;
+import org.apache.aries.rsa.util.EndpointHelper;
+import org.easymock.EasyMock;
+import org.junit.After;
+import org.junit.Test;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceException;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.aries.rsa.provider.tcp.TcpProviderTest.getFreePort;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringStartsWith.startsWith;
+import static org.junit.Assert.assertEquals;
+
+public class TcpProviderTLSTest {
+
+ private static String
+ KEYSTORE =
TcpProviderTest.class.getResource("/keystore.p12").getPath(),
+ KEYSTORE2 =
TcpProviderTest.class.getResource("/keystore2.p12").getPath(),
+ TRUSTSTORE =
TcpProviderTest.class.getResource("/truststore.p12").getPath(),
+ KEYSTORE_PASSWORD = "password",
+ TRUSTSTORE_PASSWORD = "password1";
+
+ private MyService myServiceProxy;
+ private Endpoint ep;
+ private ImportedService importedService;
+
+ public void test(Map<String, Object> providerProps) throws IOException {
+ Class<?>[] exportedInterfaces = new Class[] {MyService.class};
+ TcpProvider provider = new TcpProvider();
+ provider.activate(providerProps);
+ Map<String, Object> props = new HashMap<>();
+ EndpointHelper.addObjectClass(props, exportedInterfaces);
+ int port = getFreePort();
+ props.put("aries.rsa.hostname", "localhost");
Review Comment:
can we use the defined consts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]