You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/08/29 09:12:36 UTC

[GitHub] [skywalking-java] dmsolr opened a new pull request #15: Support mTLS for gRPC channel

dmsolr opened a new pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15


   <!--
       ⚠️ Please make sure to read this template first, pull requests that don't accord with this template
       maybe closed without notice.
       Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
       Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
   -->
   
   <!-- ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👇 ====
   ### Fix <bug description or the bug issue number or bug issue link>
   - [ ] Add a unit test to verify that the fix works.
   - [ ] Explain briefly why the bug exists and how to fix it.
        ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👆 ==== -->
   
   <!-- ==== 🔌 Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist 👇 ====
   ### Add an agent plugin to support <framework name>
   - [ ] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking/blob/master/docs/en/guides/Plugin-test.md)
   - [ ] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-bootstrap/src/main/resources/component-libraries.yml)
   - [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
        ==== 🔌 Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist 👆 ==== -->
   
   <!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
   ### Improve the performance of <class or module or ...>
   - [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
   - [ ] The benchmark result.
   ```text
   <Paste the benchmark results here>
   ```
   - [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
        ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
   
   <!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
   ### <Feature description>
   - [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
   - [ ] Update the documentation to include this new feature.
   - [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
   - [ ] If it's UI related, attach the screenshots below.
        ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
   - [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.md).
   


-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698491065



##########
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:
       You deleted line `File caFile = new File(AgentPackagePath.getPath(), CA_FILE_NAME);` is for avoiding this complex `if/else`. Could you share way removing that line?




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] hanahmily commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698688820



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -8,19 +8,34 @@ at the same time, the SkyWalking backend is in another region (VPC).
 > Because of that, security requirement is very obvious.
 
 ## Authentication Mode
-Only support **no mutual auth**.
 - Use this [script](../../../../../tools/TLS/tls_key_generate.sh) if you are not familiar with how to generate key files.
-- Find `ca.crt`, and use it at client side
-- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC SSL` of the OAP server doc.
+- Find `ca.crt`, and use it at client side. In `mTLS` mode, `client.crt` and `client.pem` are required at client side.
+- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC Security` of the OAP server doc.
   for more details.
 
 ## 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 enables TLS automatically after the `ca.crt`(by default `/ca` folder in agent package) file is 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 mTLS enabled. More details can be found in `receiver-sharing-server` section in `application.yaml`. Please refer to `gRPC Security` and `gRPC/HTTP server for receiver`.
+- Copy CA certificate, certificate and private key of client into `agent/ca`.
+- 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:/ca/ca.crt}
+agent.ssl_key_path=${SW_AGENT_SSL_KEY_PATH:/ca/client.pem}

Review comment:
       Sorry, I forget :(.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r697989396



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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.

Review comment:
       There was a sentence `Only support no mutual auth.` in this doc, please fix.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r697989642



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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`. Please refer to [gRPC SSL](../../backend/grpc-ssl.md)  
+- Configure Client-side SSL/TLS in `agent.conf`.
+- Change `SW_AGENT_COLLECTOR_BACKEND_SERVICES` to host and port of `receiver-sharing-server`.

Review comment:
       ```suggestion
   - Change `SW_AGENT_COLLECTOR_BACKEND_SERVICES` targeting to host and port of `receiver-sharing-server`.
   ```




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698872434



##########
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();

Review comment:
       The previous logic only calls the method once, even check twice. The second API call is unnecessary.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698685415



##########
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();

Review comment:
       Remove means remove this part `caFile.exist()`, right. `File#isFile()` is true means the file existed.
   JDK docs say:
   > return: true if and only if the file denoted by this abstract pathname exists and is a normal file; false otherwise
   
   Tests `caFile#isFile()` twice, that is previously logic. There is a case to enable  SSL/TLS with a none CA certificate by force switch. And the CA file is detected, SSL/TLS is enabled by default. 
   The second time, the CA certificate has to load it to SSL context when it existed.
   




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng merged pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15


   


-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
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.

##########
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();

Review comment:
       Remove means remove this part `caFile.exist()`, right. `File#isFile()` is true means the file existed.
   JDK docs say:
   > return: true if and only if the file denoted by this abstract pathname exists and is a normal file; false otherwise
   
   Tests `caFile#isFile()` twice, that is previously logic. There is a case to enable  SSL/TLS with a none CA certificate by force switch. And the CA file is detected, SSL/TLS is enabled by default. 
   The second time, the CA certificate has to load it to SSL context when it existed.
   

##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -8,19 +8,34 @@ at the same time, the SkyWalking backend is in another region (VPC).
 > Because of that, security requirement is very obvious.
 
 ## Authentication Mode
-Only support **no mutual auth**.
 - Use this [script](../../../../../tools/TLS/tls_key_generate.sh) if you are not familiar with how to generate key files.
-- Find `ca.crt`, and use it at client side
-- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC SSL` of the OAP server doc.
+- Find `ca.crt`, and use it at client side. In `mTLS` mode, `client.crt` and `client.pem` are required at client side.
+- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC Security` of the OAP server doc.
   for more details.
 
 ## 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 enables TLS automatically after the `ca.crt`(by default `/ca` folder in agent package) file is 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 mTLS enabled. More details can be found in `receiver-sharing-server` section in `application.yaml`. Please refer to `gRPC Security` and `gRPC/HTTP server for receiver`.
+- Copy CA certificate, certificate and private key of client into `agent/ca`.
+- 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:/ca/ca.crt}
+agent.ssl_key_path=${SW_AGENT_SSL_KEY_PATH:/ca/client.pem}

Review comment:
       Sorry, I forget :(.

##########
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:
       I got your point.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698118213



##########
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:
       Make sense.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698902072



##########
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:
       Then your logic is back to https://github.com/apache/skywalking-java/pull/15#discussion_r698096140. This is not what we need to deal with, and it is endless. 
   
   And I am not meaning about configurable. My point is `File caFile = new File(AgentPackagePath.getPath(), CA_FILE_NAME);`. This is usually the standard way to process a file/path of a parent path. Let's keep in this way.
   




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] kezhenxu94 commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698124863



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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.

Review comment:
       ```suggestion
   - Agent enables TLS automatically after the `ca.crt`(by default `/ca` folder in agent package) file is detected.
   ```

##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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.

Review comment:
       Specifically, line 11 @dmsolr 

##########
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()) {

Review comment:
       ```suggestion
                       } else if (!certFile.isFile() || !keyFile.isFile()) {
   ```

##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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`. Please refer to [gRPC SSL](../../backend/grpc-ssl.md)  

Review comment:
       ```suggestion
   - Sharing gRPC server must be started with mTLS enabled. More details can be found in `receiver-sharing-server` section in `application.yaml`. Please refer to [gRPC SSL](../../backend/grpc-ssl.md)  
   ```




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698145994



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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`. Please refer to [gRPC SSL](../../backend/grpc-ssl.md)  

Review comment:
       Please don't. This is a dev doc(notice `latest`). We never will have a static path for backend document because it changes with versions, including the URL. That is why I recommend for `key words` to search document. 
   
   @Jtrust is going to add doc search for every versions of documents. Then it is more friendly and useful than right now.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698927994



##########
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:
       I got your point.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698902266



##########
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:
       Once you did this in the codebase, we could have to face pull requests or requests to change how to process file path in the whole project, which could be confused.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698014581



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -25,9 +24,9 @@ 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`. Please refer to [gRPC SSL](../../backend/grpc-ssl.md)  
+- Sharing gRPC server must be started with enabled mTLS. More details see `receiver-sharing-server` section in `application.yaml` of SkyWalking OAP Server.  

Review comment:
       What is `receiver-sharing-server` section for now? I think you should refer to the correct keyword? The keyword should be able to used here to search for documentation.
   
   ![image](https://user-images.githubusercontent.com/5441976/131251651-f7e8d19a-b7e2-4231-ae25-449f0e572112.png)
   
   From https://github.com/apache/skywalking/pull/7565/files#diff-c6b2f69e272d64016aa069e5b2469b95aeabec06cea77c3f88b734dea84ff4cd, it should be `Security(SSL/TLS/mTLS)`.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698492947



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -8,19 +8,34 @@ at the same time, the SkyWalking backend is in another region (VPC).
 > Because of that, security requirement is very obvious.
 
 ## Authentication Mode
-Only support **no mutual auth**.
 - Use this [script](../../../../../tools/TLS/tls_key_generate.sh) if you are not familiar with how to generate key files.
-- Find `ca.crt`, and use it at client side
-- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC SSL` of the OAP server doc.
+- Find `ca.crt`, and use it at client side. In `mTLS` mode, `client.crt` and `client.pem` are required at client side.
+- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC Security` of the OAP server doc.
   for more details.
 
 ## 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 enables TLS automatically after the `ca.crt`(by default `/ca` folder in agent package) file is 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 mTLS enabled. More details can be found in `receiver-sharing-server` section in `application.yaml`. Please refer to `gRPC Security` and `gRPC/HTTP server for receiver`.
+- Copy CA certificate, certificate and private key of client into `agent/ca`.
+- 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:/ca/ca.crt}
+agent.ssl_key_path=${SW_AGENT_SSL_KEY_PATH:/ca/client.pem}

Review comment:
       It seems this https://github.com/apache/skywalking-java/pull/15#discussion_r698096325 still gets unresolved, could you share why?




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698491065



##########
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:
       You deleted line `File caFile = new File(AgentPackagePath.getPath(), CA_FILE_NAME);` is for avoiding this complex `if/else`. Could you share way removing that line?

##########
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();

Review comment:
       Also, why remove this? It seems you are copying `caFile.isFile()` in 2 places instead. I am confused about the principle of code change.

##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -8,19 +8,34 @@ at the same time, the SkyWalking backend is in another region (VPC).
 > Because of that, security requirement is very obvious.
 
 ## Authentication Mode
-Only support **no mutual auth**.
 - Use this [script](../../../../../tools/TLS/tls_key_generate.sh) if you are not familiar with how to generate key files.
-- Find `ca.crt`, and use it at client side
-- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC SSL` of the OAP server doc.
+- Find `ca.crt`, and use it at client side. In `mTLS` mode, `client.crt` and `client.pem` are required at client side.
+- Find `server.crt` ,`server.pem` and `ca.crt`. Use them at server side. Please refer to `gRPC Security` of the OAP server doc.
   for more details.
 
 ## 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 enables TLS automatically after the `ca.crt`(by default `/ca` folder in agent package) file is 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 mTLS enabled. More details can be found in `receiver-sharing-server` section in `application.yaml`. Please refer to `gRPC Security` and `gRPC/HTTP server for receiver`.
+- Copy CA certificate, certificate and private key of client into `agent/ca`.
+- 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:/ca/ca.crt}
+agent.ssl_key_path=${SW_AGENT_SSL_KEY_PATH:/ca/client.pem}

Review comment:
       It seems this https://github.com/apache/skywalking-java/pull/15#discussion_r698096325 still gets unresolved, could you share why?

##########
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();

Review comment:
       The previous logic only calls the method once, even check twice. The second API call is unnecessary.

##########
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:
       Then your logic is back to https://github.com/apache/skywalking-java/pull/15#discussion_r698096140. This is not what we need to deal with, and it is endless. 
   
   And I am not meaning about configurable. My point is `File caFile = new File(AgentPackagePath.getPath(), CA_FILE_NAME);`. This is usually the standard way to process a file/path of a parent path. Let's keep in this way.
   

##########
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:
       Once you did this in the codebase, we could have to face pull requests or requests to change how to process file path in the whole project, which could be confused.

##########
File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java
##########
@@ -135,6 +135,21 @@
          * Force open TLS for gRPC channel if true.
          */
         public static boolean FORCE_TLS = false;
+
+        /**
+         * SSL trusted ca file. If it exists, will enable TLS for gRPC channel.
+         */
+        public static String SSL_TRUSTED_CA_PATH = "ca" + Constants.PATH_SEPARATOR + "ca.crt";
+
+        /**
+         * Key cert chain file. If ssl_cert_chain and ssl_key exist, will enable mTLS for gRPC channel.
+         */
+        public static String SSL_CERT_CHAIN_PATH;
+
+        /**
+         * Private key file. If ssl_cert_chain and ssl_key exist, will enable mTLS for gRPC channel.
+         */
+        public static String SSL_KEY_PATH;

Review comment:
       According to recent requirements, all configuration should be added in default `agent.config`, with all system env variable names should be documents in agent doc.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698134171



##########
File path: docs/en/setup/service-agent/java-agent/TLS.md
##########
@@ -17,10 +17,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`. Please refer to [gRPC SSL](../../backend/grpc-ssl.md)  

Review comment:
       The gRPC SSL refers to the main repo. So we cannot use a relative path. Could we use this URL, https://skywalking.apache.org/docs/main/latest/en/setup/backend/grpc-security/




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698940004



##########
File path: apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java
##########
@@ -135,6 +135,21 @@
          * Force open TLS for gRPC channel if true.
          */
         public static boolean FORCE_TLS = false;
+
+        /**
+         * SSL trusted ca file. If it exists, will enable TLS for gRPC channel.
+         */
+        public static String SSL_TRUSTED_CA_PATH = "ca" + Constants.PATH_SEPARATOR + "ca.crt";
+
+        /**
+         * Key cert chain file. If ssl_cert_chain and ssl_key exist, will enable mTLS for gRPC channel.
+         */
+        public static String SSL_CERT_CHAIN_PATH;
+
+        /**
+         * Private key file. If ssl_cert_chain and ssl_key exist, will enable mTLS for gRPC channel.
+         */
+        public static String SSL_KEY_PATH;

Review comment:
       According to recent requirements, all configuration should be added in default `agent.config`, with all system env variable names should be documents in agent doc.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#issuecomment-907763714


   Please fix dead link


-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] dmsolr commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698169195



##########
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:
       Got it




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng merged pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15


   


-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #15: Support mTLS for gRPC channel

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #15:
URL: https://github.com/apache/skywalking-java/pull/15#discussion_r698491618



##########
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();

Review comment:
       Also, why remove this? It seems you are copying `caFile.isFile()` in 2 places instead. I am confused about the principle of code change.




-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org