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