hanahmily commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698096140



##########
File path: 
apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/remote/TLSChannelBuilder.java
##########
@@ -23,31 +23,64 @@
 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 path;
+        } else if (path.startsWith("./")) {

Review comment:
       How to handle other relative paths, for instance, `../`, `.././..` and 
etc?
   
   What I mean is we don't have to parse relative paths, just appending the 
agent package path to it looks like a tidier way

##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +16,23 @@ Only support **no mutual auth**.
 ## Open and config TLS
 
 ### Agent config
-- Place `ca.crt` into `/ca` folder in agent package. Notice, `/ca` is not 
created in distribution, please create it by yourself.
-
-- Agent open TLS automatically after the `/ca/ca.crt` file detected.
+- Agent open TLS automatically after the `ca.crt`(by default `/ca` folder in 
agent package) file detected.
 - TLS with no CA mode could be activated by this setting.
 ```
-agent.force_tls=${SW_AGENT_FORCE_TLS:false}
+agent.force_tls=${SW_AGENT_FORCE_TLS:true}
+```
+
+## Enable mutual TLS
+
+- Sharing gRPC server must be started with enabled mTLS. More details see 
`receiver-sharing-server` section in `application.yaml` of SkyWalking OAP 
Server.  
+- Configure Client-side SSL/TLS in `agent.conf`.
+- Change `SW_AGENT_COLLECTOR_BACKEND_SERVICES` targeting to host and port of 
`receiver-sharing-server`.
+
+For example:
 ```
+agent.force_tls=${SW_AGENT_FORCE_TLS:true}
+agent.ssl_trusted_ca_path=${SW_AGENT_SSL_TRUSTED_CA_PATH:/path/to/ca.crt}
+agent.ssl_key_path=${SW_AGENT_SSL_KEY_PATH:/path/to/client.pem}

Review comment:
       We should document which private key format is supported just like the 
server-side.




-- 
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]


Reply via email to