You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "Copilot (via GitHub)" <gi...@apache.org> on 2026/06/30 10:53:42 UTC
Re: [PR] add mTLS support [iotdb-client-csharp]
Copilot commented on code in PR #60:
URL: https://github.com/apache/iotdb-client-csharp/pull/60#discussion_r3498152692
##########
src/Apache.IoTDB/SessionPool.cs:
##########
@@ -524,6 +538,144 @@ private async Task<Client> CreateAndOpen(string host, int port, bool enableRpcCo
}
}
+ private static X509Certificate2 LoadClientCertificate(string clientCertificatePath, string clientCertificatePassword)
+ {
+ if (string.IsNullOrWhiteSpace(clientCertificatePath))
+ {
+ return null;
+ }
+
+ return clientCertificatePassword == null
+ ? new X509Certificate2(clientCertificatePath)
+ : new X509Certificate2(clientCertificatePath, clientCertificatePassword);
+ }
+
+ private static X509Certificate2Collection LoadRootCertificates(string rootCertificatePath)
+ {
+ if (string.IsNullOrWhiteSpace(rootCertificatePath))
+ {
+ return null;
+ }
+
+ var certificateBytes = File.ReadAllBytes(rootCertificatePath);
+ var certificates = LoadPemCertificates(certificateBytes);
+ if (certificates.Count > 0)
+ {
+ return certificates;
+ }
+
+ certificates.Import(certificateBytes);
+ return certificates;
+ }
+
+ private static X509Certificate2Collection LoadPemCertificates(byte[] certificateBytes)
+ {
+ const string beginCertificate = "-----BEGIN CERTIFICATE-----";
+ const string endCertificate = "-----END CERTIFICATE-----";
+
+ var certificates = new X509Certificate2Collection();
+ var certificateText = Encoding.ASCII.GetString(certificateBytes);
+ var startIndex = certificateText.IndexOf(beginCertificate, StringComparison.Ordinal);
+ while (startIndex >= 0)
+ {
+ startIndex += beginCertificate.Length;
+ var endIndex = certificateText.IndexOf(endCertificate, startIndex, StringComparison.Ordinal);
+ if (endIndex < 0)
+ {
+ break;
+ }
+
+ var base64 = certificateText.Substring(startIndex, endIndex - startIndex)
+ .Replace("\r", string.Empty)
+ .Replace("\n", string.Empty)
+ .Trim();
+ certificates.Add(new X509Certificate2(Convert.FromBase64String(base64)));
+ startIndex = certificateText.IndexOf(beginCertificate, endIndex + endCertificate.Length, StringComparison.Ordinal);
+ }
+
+ return certificates;
+ }
+
+ private static RemoteCertificateValidationCallback CreateRemoteCertificateValidationCallback(X509Certificate2Collection rootCertificates)
+ {
+ if (rootCertificates == null || rootCertificates.Count == 0)
+ {
+ return null;
+ }
+
+ return (sender, certificate, chain, sslPolicyErrors) =>
+ {
+ if ((sslPolicyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != 0 ||
+ (sslPolicyErrors & SslPolicyErrors.RemoteCertificateNotAvailable) != 0 ||
+ certificate == null)
+ {
+ return false;
+ }
+
+ var serverCertificate = certificate as X509Certificate2 ?? new X509Certificate2(certificate);
+ try
+ {
+ using var customChain = new X509Chain();
+ customChain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
+ customChain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority;
Review Comment:
When a custom root CA is provided, the RemoteCertificateValidationCallback currently rebuilds the chain but does not enforce that the leaf certificate is valid for TLS server authentication (EKU 1.3.6.1.5.5.7.3.1). Because the callback ignores SslPolicyErrors.RemoteCertificateChainErrors and X509Chain does not validate EKU by default, this can unintentionally accept certificates that are not appropriate for server auth as long as they chain to the provided root.
##########
src/Apache.IoTDB/SessionPool.cs:
##########
@@ -524,6 +538,144 @@ private async Task<Client> CreateAndOpen(string host, int port, bool enableRpcCo
}
}
+ private static X509Certificate2 LoadClientCertificate(string clientCertificatePath, string clientCertificatePassword)
+ {
+ if (string.IsNullOrWhiteSpace(clientCertificatePath))
+ {
+ return null;
+ }
+
+ return clientCertificatePassword == null
+ ? new X509Certificate2(clientCertificatePath)
+ : new X509Certificate2(clientCertificatePath, clientCertificatePassword);
Review Comment:
LoadClientCertificate treats an empty password differently from a missing password. If ClientCertificatePassword is provided but empty (e.g., connection string sets it to an empty value), X509Certificate2(path, "") will be used and can fail for certs that are unencrypted / expect a null password. Treating null/empty uniformly avoids surprising load failures.
##########
src/Apache.IoTDB/SessionPool.Builder.cs:
##########
@@ -100,9 +102,21 @@ public Builder SetUseSsl(bool useSsl)
return this;
}
- public Builder SetCertificatePath(string certificatePath)
+ public Builder SetClientCertificatePath(string clientCertificatePath)
{
- _certificatePath = certificatePath;
+ _clientCertificatePath = clientCertificatePath;
+ return this;
+ }
Review Comment:
Renaming/removing SetCertificatePath is a breaking change for existing users of the fluent builder. If the previous SetCertificatePath was intended for client certificate auth (as it was passed into the TLS transport), consider keeping it as an [Obsolete] shim that forwards to SetClientCertificatePath to preserve backward compatibility.
##########
README.md:
##########
@@ -63,6 +63,50 @@ Users can quickly get started by referring to the use cases under the Apache-IoT
For those who wish to delve deeper into the client's usage and explore more advanced features, the samples directory contains additional code samples.
+## TLS and mTLS
+
+Enable TLS by calling `SetUseSsl(true)`. The C# client uses the .NET certificate model and does not read Java truststores directly. If your certificates were generated with the JDK 17 Java/keytool workflow, `client.keystore` is PKCS#12 by default and can be used directly as the client certificate file; use `ca.crt` directly as the trusted root.
+
+| keytool artifact | C# client usage |
+| --- | --- |
+| `ca.crt` | Pass to `SetRootCertificatePath` / `RootCertificatePath` to trust the server certificate |
+| `client.keystore` | Contains the client private key and certificate chain; JDK 17 creates PKCS#12 by default, so pass it directly to `SetClientCertificatePath` |
+| `client.truststore` | Java client truststore; the C# client uses `ca.crt` instead |
+| `server.truststore` | Server-side truststore for trusting client certificates; not a C# client option |
+
+Only convert the keystore first if you are reusing an older JKS file, or if it was explicitly generated with `-storetype JKS`:
+
+```bash
+$KT -importkeystore \
+ -srckeystore client.keystore \
+ -srcstorepass $PWD \
+ -srcalias client \
+ -destkeystore client.p12 \
+ -deststoretype PKCS12 \
+ -deststorepass $PWD \
+ -destkeypass $PWD \
+ -destalias client
+```
+
+C# builder example:
+
+```csharp
+var sessionPool = new SessionPool.Builder()
+ .SetHost("127.0.0.1")
Review Comment:
The custom RemoteCertificateValidationCallback rejects SslPolicyErrors.RemoteCertificateNameMismatch. With the example using an IP address, TLS will fail unless the server certificate contains that IP in the SAN. Using a hostname that matches the certificate (e.g., localhost) avoids a confusing “it doesn’t work” experience.
##########
src/Apache.IoTDB/SessionPool.cs:
##########
@@ -448,15 +458,19 @@ public async Task<string> GetTimeZone()
}
}
- private async Task<Client> CreateAndOpen(string host, int port, bool enableRpcCompression, int timeout, bool useSsl, string cert, string sqlDialect, string database, CancellationToken cancellationToken = default)
+ private async Task<Client> CreateAndOpen(string host, int port, bool enableRpcCompression, int timeout, bool useSsl, string clientCertificatePath, string clientCertificatePassword, string rootCertificatePath, string sqlDialect, string database, CancellationToken cancellationToken = default)
{
TTransport socket;
if (useSsl)
{
+ var clientCertificate = LoadClientCertificate(clientCertificatePath, clientCertificatePassword);
+ var rootCertificates = LoadRootCertificates(rootCertificatePath);
+ var remoteCertificateValidationCallback = CreateRemoteCertificateValidationCallback(rootCertificates);
+ var localCertificateSelectionCallback = CreateLocalCertificateSelectionCallback(clientCertificate);
Review Comment:
Client/root certificates are loaded from disk and parsed on every CreateAndOpen call (including for each pooled client and each reconnect). This adds repeated file I/O and certificate parsing overhead and can amplify operational impact under reconnect storms. Consider loading/caching the certificates once per SessionPool instance and reusing them for all connections, disposing them when the pool is disposed.
##########
src/Apache.IoTDB/TableSessionPool.Builder.cs:
##########
@@ -103,9 +105,21 @@ public Builder SetUseSsl(bool useSsl)
return this;
}
- public Builder SetCertificatePath(string certificatePath)
+ public Builder SetClientCertificatePath(string clientCertificatePath)
{
- _certificatePath = certificatePath;
+ _clientCertificatePath = clientCertificatePath;
+ return this;
+ }
Review Comment:
Renaming/removing SetCertificatePath is a breaking change for TableSessionPool.Builder users. Consider keeping SetCertificatePath as an [Obsolete] shim forwarding to SetClientCertificatePath to preserve source compatibility.
##########
README_ZH.md:
##########
@@ -61,6 +61,49 @@ dotnet add package Apache.IoTDB
对于希望深入了解客户端用法并探索更高级特性的用户，samples目录包含了额外的代码示例。
+## TLS 和 mTLS
+
+通过 `SetUseSsl(true)` 开启 TLS。C# 客户端使用 .NET 的证书模型，不直接读取 Java truststore；如果证书按 JDK 17 的 Java/keytool 文档生成，`client.keystore` 默认就是 PKCS#12，可以直接作为客户端证书文件使用，并直接使用 `ca.crt` 作为信任根。
+
+| keytool 产物 | C# 客户端用法 |
+| --- | --- |
+| `ca.crt` | 传给 `SetRootCertificatePath` / `RootCertificatePath`，用于信任服务端证书 |
+| `client.keystore` | 包含客户端私钥和证书链；JDK 17 默认是 PKCS#12，直接传给 `SetClientCertificatePath` |
+| `client.truststore` | Java 客户端的 truststore；C# 侧用 `ca.crt`，不需要这个文件 |
+| `server.truststore` | 服务端用于信任客户端证书，不是 C# 客户端参数 |
+
+只有在复用旧版 JDK 生成的 JKS 文件，或显式使用 `-storetype JKS` 生成 keystore 时，才需要先转换为 PKCS#12：
+
+```bash
+$KT -importkeystore \
+ -srckeystore client.keystore \
+ -srcstorepass $PWD \
+ -srcalias client \
+ -destkeystore client.p12 \
+ -deststoretype PKCS12 \
+ -deststorepass $PWD \
+ -destkeypass $PWD \
+ -destalias client
+```
+
+C# builder 示例：
+
+```csharp
+var sessionPool = new SessionPool.Builder()
+ .SetHost("127.0.0.1")
Review Comment:
当 RootCertificatePath 启用自定义 CA 校验时，代码会拒绝 RemoteCertificateNameMismatch。示例里使用 IP（127.0.0.1）会导致 TLS 失败，除非证书 SAN 中包含该 IP。建议示例改为使用与证书匹配的主机名（例如 localhost），避免用户按文档操作却握手失败。
##########
README.md:
##########
@@ -63,6 +63,50 @@ Users can quickly get started by referring to the use cases under the Apache-IoT
For those who wish to delve deeper into the client's usage and explore more advanced features, the samples directory contains additional code samples.
+## TLS and mTLS
+
+Enable TLS by calling `SetUseSsl(true)`. The C# client uses the .NET certificate model and does not read Java truststores directly. If your certificates were generated with the JDK 17 Java/keytool workflow, `client.keystore` is PKCS#12 by default and can be used directly as the client certificate file; use `ca.crt` directly as the trusted root.
+
+| keytool artifact | C# client usage |
+| --- | --- |
+| `ca.crt` | Pass to `SetRootCertificatePath` / `RootCertificatePath` to trust the server certificate |
+| `client.keystore` | Contains the client private key and certificate chain; JDK 17 creates PKCS#12 by default, so pass it directly to `SetClientCertificatePath` |
+| `client.truststore` | Java client truststore; the C# client uses `ca.crt` instead |
+| `server.truststore` | Server-side truststore for trusting client certificates; not a C# client option |
+
+Only convert the keystore first if you are reusing an older JKS file, or if it was explicitly generated with `-storetype JKS`:
+
+```bash
+$KT -importkeystore \
+ -srckeystore client.keystore \
+ -srcstorepass $PWD \
+ -srcalias client \
+ -destkeystore client.p12 \
+ -deststoretype PKCS12 \
+ -deststorepass $PWD \
+ -destkeypass $PWD \
+ -destalias client
+```
+
+C# builder example:
+
+```csharp
+var sessionPool = new SessionPool.Builder()
+ .SetHost("127.0.0.1")
+ .SetPort(6667)
+ .SetUseSsl(true)
+ .SetRootCertificatePath("tls-certs/ca.crt")
+ .SetClientCertificatePath("tls-certs/client.keystore")
+ .SetClientCertificatePassword("IoTDB")
+ .Build();
+```
+
+The ADO.NET connection string supports the same options:
+
+```text
+DataSource=127.0.0.1;Port=6667;UseSsl=True;RootCertificatePath=tls-certs/ca.crt;ClientCertificatePath=tls-certs/client.keystore;ClientCertificatePassword=IoTDB
Review Comment:
This connection string example uses an IP address. When RootCertificatePath is set, the code enforces host name validation and will fail unless the server cert SAN includes the IP. Prefer a hostname matching the certificate (e.g., localhost) or explicitly document the SAN requirement.
##########
README_ZH.md:
##########
@@ -61,6 +61,49 @@ dotnet add package Apache.IoTDB
对于希望深入了解客户端用法并探索更高级特性的用户，samples目录包含了额外的代码示例。
+## TLS 和 mTLS
+
+通过 `SetUseSsl(true)` 开启 TLS。C# 客户端使用 .NET 的证书模型，不直接读取 Java truststore；如果证书按 JDK 17 的 Java/keytool 文档生成，`client.keystore` 默认就是 PKCS#12，可以直接作为客户端证书文件使用，并直接使用 `ca.crt` 作为信任根。
+
+| keytool 产物 | C# 客户端用法 |
+| --- | --- |
+| `ca.crt` | 传给 `SetRootCertificatePath` / `RootCertificatePath`，用于信任服务端证书 |
+| `client.keystore` | 包含客户端私钥和证书链；JDK 17 默认是 PKCS#12，直接传给 `SetClientCertificatePath` |
+| `client.truststore` | Java 客户端的 truststore；C# 侧用 `ca.crt`，不需要这个文件 |
+| `server.truststore` | 服务端用于信任客户端证书，不是 C# 客户端参数 |
+
+只有在复用旧版 JDK 生成的 JKS 文件，或显式使用 `-storetype JKS` 生成 keystore 时，才需要先转换为 PKCS#12：
+
+```bash
+$KT -importkeystore \
+ -srckeystore client.keystore \
+ -srcstorepass $PWD \
+ -srcalias client \
+ -destkeystore client.p12 \
+ -deststoretype PKCS12 \
+ -deststorepass $PWD \
+ -destkeypass $PWD \
+ -destalias client
+```
+
+C# builder 示例：
+
+```csharp
+var sessionPool = new SessionPool.Builder()
+ .SetHost("127.0.0.1")
+ .SetPort(6667)
+ .SetUseSsl(true)
+ .SetRootCertificatePath("tls-certs/ca.crt")
+ .SetClientCertificatePath("tls-certs/client.keystore")
+ .SetClientCertificatePassword("IoTDB")
+ .Build();
+```
+
+ADO.NET 连接字符串也支持相同配置：
+
+```text
+DataSource=127.0.0.1;Port=6667;UseSsl=True;RootCertificatePath=tls-certs/ca.crt;ClientCertificatePath=tls-certs/client.keystore;ClientCertificatePassword=IoTDB
Review Comment:
这个连接字符串示例使用 IP 地址。当前实现会在提供 RootCertificatePath 时严格校验主机名，除非服务端证书 SAN 包含该 IP，否则会握手失败。建议示例改为 localhost 或补充说明 SAN 要求。
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org