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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
