dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698673208
##########
File path:
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/remote/TLSChannelBuilder.java
##########
@@ -23,31 +23,61 @@
import io.grpc.netty.NettyChannelBuilder;
import io.netty.handler.ssl.SslContextBuilder;
import java.io.File;
-import javax.net.ssl.SSLException;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
import org.apache.skywalking.apm.agent.core.boot.AgentPackageNotFoundException;
import org.apache.skywalking.apm.agent.core.boot.AgentPackagePath;
import org.apache.skywalking.apm.agent.core.conf.Config;
-import org.apache.skywalking.apm.agent.core.conf.Constants;
+import org.apache.skywalking.apm.agent.core.logging.api.ILog;
+import org.apache.skywalking.apm.agent.core.logging.api.LogManager;
+import org.apache.skywalking.apm.agent.core.util.PrivateKeyUtil;
+import org.apache.skywalking.apm.util.StringUtil;
/**
- * Detect the `/ca` folder in agent package, if `ca.crt` exists, start TLS (no
mutual auth).
+ * If only ca.crt exists, start TLS. If cert, key and ca files exist, enable
mTLS.
*/
public class TLSChannelBuilder implements ChannelBuilder<NettyChannelBuilder> {
- private static String CA_FILE_NAME = "ca" + Constants.PATH_SEPARATOR +
"ca.crt";
+ private static final ILog LOGGER =
LogManager.getLogger(TLSChannelBuilder.class);
@Override
public NettyChannelBuilder build(
- NettyChannelBuilder managedChannelBuilder) throws
AgentPackageNotFoundException, SSLException {
- File caFile = new File(AgentPackagePath.getPath(), CA_FILE_NAME);
- boolean isCAFileExist = caFile.exists() && caFile.isFile();
- if (Config.Agent.FORCE_TLS || isCAFileExist) {
+ NettyChannelBuilder managedChannelBuilder) throws
AgentPackageNotFoundException, IOException {
+
+ File caFile = new
File(toAbsolutePath(Config.Agent.SSL_TRUSTED_CA_PATH));
+ if (Config.Agent.FORCE_TLS || caFile.isFile()) {
SslContextBuilder builder = GrpcSslContexts.forClient();
- if (isCAFileExist) {
+
+ if (caFile.isFile()) {
+ String certPath = Config.Agent.SSL_CERT_CHAIN_PATH;
+ String keyPath = Config.Agent.SSL_KEY_PATH;
+ if (StringUtil.isNotBlank(certPath) &&
StringUtil.isNotBlank(keyPath)) {
+ File keyFile = new File(toAbsolutePath(keyPath));
+ File certFile = new File(toAbsolutePath(certPath));
+
+ if (certFile.isFile() && keyFile.isFile()) {
+ try (InputStream cert = new FileInputStream(certFile);
+ InputStream key =
PrivateKeyUtil.loadDecryptionKey(keyPath)) {
+ builder.keyManager(cert, key);
+ }
+ } else if (!certFile.isFile() || !keyFile.isFile()) {
+ LOGGER.warn("Failed to enable mTLS caused by cert or
key cannot be found.");
+ }
+ }
+
builder.trustManager(caFile);
}
- managedChannelBuilder =
managedChannelBuilder.negotiationType(NegotiationType.TLS)
-
.sslContext(builder.build());
+
managedChannelBuilder.negotiationType(NegotiationType.TLS).sslContext(builder.build());
}
return managedChannelBuilder;
}
+
+ private static String toAbsolutePath(final String path) throws
AgentPackageNotFoundException {
+ if (path.startsWith("/")) {
+ return AgentPackagePath.getPath() + path;
+ } else {
+ return AgentPackagePath.getPath() + "/" + path;
+ }
Review comment:
Before, CA certificate file is a default value which was `/ca/ca.cert`.
Now, I make it is configurable. Because users need to configure the client's
certificate and private key. I think CA certificate is still unconfigurable,
which is a little odd.
As they are configurable, I think we need to adapt start with `/` or not.
Sure, it is unnecessary. Required users fill the path started with `/` and
documented it in docs.
--
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]
