RATIS-1747. Support keyManager and trustManager in tlsConfig. #785
Conversation
|
Hi @szetszwo , I see currently Ozone is using ratis 2.4.0 release. And it looks like ratis master(3.0) is not compatible with branch-2 in some API. Will we keep on maintaining both branch-2 and master(3.0) for ratis? |
|
The failed UT TestWatchRequestWithGrpc is irrelevant. It passed in local. |
@ChenSammi , we should move Ozone to Ratis 3.0 soon. |
There was a problem hiding this comment.
@ChenSammi , thanks a lot for working on this!
- For historical reason, we have
GrpcTlsConfig. We should deprecate it soon. Please don't add new API to it. - We should also update
NettyUtils. Then, we may copy some utility methods toGrpcUtiland replace the gRPC code.
See https://issues.apache.org/jira/secure/attachment/13052291/785_review.patch
pom.xml
Outdated
| @@ -218,6 +218,8 @@ | |||
| <test.exclude.pattern>_</test.exclude.pattern> | |||
| <!-- number of threads/forks to use when running tests in parallel, see parallel-tests profile --> | |||
| <testsThreadCount>4</testsThreadCount> | |||
|
|
|||
| <bouncycastle.version>1.67</bouncycastle.version> | |||
There was a problem hiding this comment.
Use 1.70. IntelliJ reported some security vulnerabilities on 1.67.
ratis-common/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.bouncycastle</groupId> | ||
| <artifactId>bcprov-jdk15on</artifactId> | ||
| <version>${bouncycastle.version}</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.bouncycastle</groupId> | ||
| <artifactId>bcpkix-jdk15on</artifactId> | ||
| <version>${bouncycastle.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
Move org.bouncycastle dependencies to ratis-test and add
<scope>test</scope>| private final CertificatesConf trustCertificates; | ||
| private CertificatesConf trustCertificates; | ||
| private TrustManager trustManager; |
| private final PrivateKeyConf privateKey; | ||
| private PrivateKeyConf privateKey; | ||
| /** Certificates for the private key. */ | ||
| private final CertificatesConf keyCertificates; | ||
| private CertificatesConf keyCertificates; | ||
| private KeyManager keyManager; |
There was a problem hiding this comment.
Keep all the fields final.
| import org.bouncycastle.util.io.pem.PemObject; | ||
| import org.bouncycastle.util.io.pem.PemReader; |
There was a problem hiding this comment.
Let's move SecurityTestUtils to ratis-test. Then, we don't have to add org.bouncycastle dependencies to ratis-common.
| public TrustManager getSslTrustManager() { | ||
| return Optional.ofNullable(getTrustManager()) | ||
| .map(TrustManagerConf::getTrustManager) | ||
| .orElse(null); | ||
| } | ||
|
|
||
| public KeyManager getSslKeyManager() { | ||
| return Optional.ofNullable(getKeyManager()) | ||
| .map(KeyManagerConf::getKeyManager) | ||
| .orElse(null); | ||
| } |
There was a problem hiding this comment.
Let's don't add new API to GrpcTlsConfig, which will be deprecated soon. We should use the API from TlsConf.
|
Thanks @szetszwo . A new patch is uploaded to address the comments. |
|
@ChenSammi , thanks a lot for the update! There is conflict. Could you resolve it? |
ChenSammi
force-pushed
the
RATIS-1747
branch
from
7a88d16
to
ee667f5
Compare
|
Hi @szetszwo , could you help to merge it? I have re-based the patch against the master because there is some file conflict. There is one unit test failure which is irrelevant. |
|
Thanks @szetszwo . Could you tell me how to publish a ratis 3.0 snapshot? so it can be used in Ozone master branch. |
|
I see. I don't have the committer privilege yet. @szetszwo , could you do me a favor and help to publish a 3.0 SNAPSHOT release at your convenient time? |
|
@ChenSammi , just have deployed 3.0.0-729d3dc-SNAPSHOT . |
|
@szetszwo , thanks a lot. |
(cherry picked from commit 729d3dc)
|
@ChenSammi , also deployed 2.4.2-8b8bdda-SNAPSHOT. |
https://issues.apache.org/jira/browse/RATIS-1747