You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by shinrich <gi...@git.apache.org> on 2016/07/18 18:40:41 UTC
[GitHub] trafficserver pull request #810: TS-4671: Allow multicert line with action b...
GitHub user shinrich opened a pull request:
https://github.com/apache/trafficserver/pull/810
TS-4671: Allow multicert line with action but no ssl_cert_name
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shinrich/trafficserver ts-4679
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/810.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #810
----
commit 404b93a3af0552cb20f4df3e173af2718831b433
Author: shinrich <sh...@ieee.org>
Date: 2016-07-18T18:39:03Z
TS-4671: Allow for multicert line with action specified but not ssl_cert_name.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4679: Allow multicert line with action but no s...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/810
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/483/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4671: Allow multicert line with action but no s...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/810
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/344/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #810: TS-4679: Allow multicert line with action b...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r72265027
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1583,7 +1583,10 @@ SSLInitServerContext(const SSLConfigParams *params, const ssl_user_config &sslMu
SSL_CTX_set_default_passwd_cb_userdata(ctx, &ud);
}
- if (sslMultCertSettings.cert) {
+ if (!sslMultCertSettings.cert && sslMultCertSettings.opt != SSLCertContext::OPT_TUNNEL) {
+ Warning("No ssl_cert_name specified and no tunnel action set");
+ goto fail;
+ } else if (sslMultCertSettings.cert) {
--- End diff --
I think this is "ok", it just reads a little odd. I probably would have preferred e.g.
if (!sslMultCertSettings.cert) {
if (sslMultCertSettings.opt != SSLCertContext::OPT_TUNNEL) {
Warning( ...)
}
} else {
SimpleTokenizer cert_tok((const ...
I know, a little nitpick, but the fear being that we repeat the conditional multiple time, having a risk of inconsistencies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #810: TS-4679: Allow multicert line with action b...
Posted by biilmann <gi...@git.apache.org>.
Github user biilmann commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r75587907
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1583,7 +1583,12 @@ SSLInitServerContext(const SSLConfigParams *params, const ssl_user_config &sslMu
SSL_CTX_set_default_passwd_cb_userdata(ctx, &ud);
}
- if (sslMultCertSettings.cert) {
+ if (!sslMultCertSettings.cert) {
+ if (sslMultCertSettings.opt != SSLCertContext::OPT_TUNNEL) {
--- End diff --
Done https://issues.apache.org/jira/browse/TS-4769
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4671: Allow multicert line with action but no s...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/810
@shinrich I think you linked this to the wrong Jira number? It should be TS-4679, right ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4679: Allow multicert line with action but no s...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/810
:+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #810: TS-4671: Allow multicert line with action b...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r71376108
--- Diff: iocore/net/SSLUtils.cc ---
@@ -2010,8 +2013,9 @@ ssl_extract_certificate(const matcher_line *line_info, ssl_user_config &sslMultC
}
}
if (!sslMultCertSettings.cert) {
- Warning("missing %s tag", SSL_CERT_TAG);
- return false;
+ // TS-4679: It is ok to be missing the cert. At least if the action is set to tunnel
--- End diff --
I think you should just nuke this commented code, and maybe add a comment above about the special lenience case for the tunneling (where you check if sslMultCertSettings.cert is NULL or not).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4679: Allow multicert line with action but no s...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/810
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/380/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #810: TS-4679: Allow multicert line with action b...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r75587822
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1583,7 +1583,12 @@ SSLInitServerContext(const SSLConfigParams *params, const ssl_user_config &sslMu
SSL_CTX_set_default_passwd_cb_userdata(ctx, &ud);
}
- if (sslMultCertSettings.cert) {
+ if (!sslMultCertSettings.cert) {
+ if (sslMultCertSettings.opt != SSLCertContext::OPT_TUNNEL) {
--- End diff --
@biilmann Can you please file a Jira for that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4679: Allow multicert line with action but no s...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/810
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/484/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4679: Allow multicert line with action but no s...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/810
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/381/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #810: TS-4679: Allow multicert line with action b...
Posted by biilmann <gi...@git.apache.org>.
Github user biilmann commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/810#discussion_r75587688
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1583,7 +1583,12 @@ SSLInitServerContext(const SSLConfigParams *params, const ssl_user_config &sslMu
SSL_CTX_set_default_passwd_cb_userdata(ctx, &ud);
}
- if (sslMultCertSettings.cert) {
+ if (!sslMultCertSettings.cert) {
+ if (sslMultCertSettings.opt != SSLCertContext::OPT_TUNNEL) {
--- End diff --
This breaks the `TSSslServerContextCreate` plugin API method.
That method calls `SSLCreateServerContext` that delegates to `SSLInitServerContext` with an empty `sslMultCertSettings`.
This is useful for plugins that want to set the certificate/ca chain/private key themselves, for example to allow lazy loading of SNI certificates...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #810: TS-4679: Allow multicert line with action b...
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich closed the pull request at:
https://github.com/apache/trafficserver/pull/810
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #810: TS-4671: Allow multicert line with action but no s...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/810
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/450/ for details.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---