You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/01 00:42:09 UTC
[GitHub] [beam] YYTVicky opened a new pull request #11008: Update comment to
tell user this is not secure
YYTVicky opened a new pull request #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] alexvanboxel commented on issue #11008: Update comment to
tell user this is not secure
Posted by GitBox <gi...@apache.org>.
alexvanboxel commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-601569348
> What should be the next action here? Should we remove it? Or add this comment?
Personally I would close this PR as the original author didn't take actions on the commit. It doesn't add a lot of value and just adds to our cognitive load being here. It did bring too our attention that it's maybe interesting to think about a unified way of providing TLS for all IO.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] alexvanboxel commented on issue #11008: Update comment to
tell user this is not secure
Posted by GitBox <gi...@apache.org>.
alexvanboxel commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-595355472
Run Spotless PreCommit
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] alexvanboxel commented on issue #11008: Update comment to
tell user this is not secure
Posted by GitBox <gi...@apache.org>.
alexvanboxel commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-595462253
> Maybe we should disallow this behavior altogether no?
Good point, this doesn't feel right to be part of Beam. Maybe better remove or add "deprecation" and mark for removal.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] YYTVicky commented on issue #11008: Update comment to tell
user this is not secure
Posted by GitBox <gi...@apache.org>.
YYTVicky commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-606099826
> I filled https://issues.apache.org/jira/browse/BEAM-9564 to remove this risky option since it seems we agree on this. I will open a PR for this soon.
> I am going to close this ticket. Thanks for bringing awareness on this issue @YYTVicky
Sorry for the late reply. The point I want to raise here is that maybe we can leave a template on the comment to help the user to implement it when using it (e.g.add the checking code on check-client trusted or checkservertrusted).
A unified TLSv1.2 connection will be better since recent research showed that TLSv1.1 had a security issue.
A workable template would like:
`
new X509TrustManager(){
@Override
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
for (final X509TrustManager trustManager : trustManagers) {
try {
trustManager.checkClientTrusted(chain, authType);
return;
} catch (final CertificateException e) {
//LOGGER.debug(e.getMessage(), e);
}
}
throw new CertificateException("None of the TrustManagers trust this certificate chain");
}
@Override
public X509Certificate[] getAcceptedIssuers() {
for (final X509TrustManager trustManager : trustManagers) {
final List<X509Certificate> list = Arrays.asList(trustManager.getAcceptedIssuers());
certificates.addAll(list);
}
return certificates.toArray(new X509Certificate[] {});
}
@Override
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException{
if (chain == null) {
throw new IllegalArgumentException("checkServerTrusted:x509Certificate array isnull");
}
if (!(chain.length > 0)) {
throw new IllegalArgumentException("checkServerTrusted: X509Certificate is empty");
}
if (!(null != authType && authType.equalsIgnoreCase("RSA"))) {
throw new CertificateException("checkServerTrusted: AuthType is not RSA");
}
try {
TrustManagerFactory tmf = TrustManagerFactory.getInstance("X509");
tmf.init((KeyStore) null);
for (TrustManager trustManager : tmf.getTrustManagers()) {
((X509TrustManager) trustManager).checkServerTrusted(chain, authType);
}
} catch (Exception e) {
throw new CertificateException(e);
}
RSAPublicKey pubkey = (RSAPublicKey) chain[0].getPublicKey();
String encoded = new BigInteger(1 , pubkey.getEncoded()).toString(16);
final boolean expected = PUB_KEY.equalsIgnoreCase(encoded);
if (!expected) {
throw new CertificateException("checkServerTrusted: Expected public key: "
+ PUB_KEY + ", got public key:" + encoded);
}
}
};
`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] iemejia commented on issue #11008: Update comment to tell
user this is not secure
Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-595458732
Maybe we should disallow this behavior altogether no?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] alexvanboxel commented on issue #11008: Update comment to
tell user this is not secure
Posted by GitBox <gi...@apache.org>.
alexvanboxel commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-595362435
Run gradle spotless apply to fix your formatting:
https://builds.apache.org/job/beam_PreCommit_Spotless_Phrase/38/console
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] iemejia commented on issue #11008: Update comment to tell
user this is not secure
Posted by GitBox <gi...@apache.org>.
iemejia commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-601937789
I filled https://issues.apache.org/jira/browse/BEAM-9564 to remove this risky option since it seems we agree on this. I will open a PR for this soon.
I am going to close this ticket. Thanks for bringing awareness on this issue @YYTVicky
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] iemejia closed pull request #11008: Update comment to tell
user this is not secure
Posted by GitBox <gi...@apache.org>.
iemejia closed pull request #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] aaltay commented on issue #11008: Update comment to tell
user this is not secure
Posted by GitBox <gi...@apache.org>.
aaltay commented on issue #11008: Update comment to tell user this is not secure
URL: https://github.com/apache/beam/pull/11008#issuecomment-601455946
What should be the next action here? Should we remove it? Or add this comment?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services