You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by persiaAziz <gi...@git.apache.org> on 2016/08/29 14:29:48 UTC
[GitHub] trafficserver pull request #938: TS-4263: Global key block configurable via ...
GitHub user persiaAziz opened a pull request:
https://github.com/apache/trafficserver/pull/938
TS-4263: Global key block configurable via Records.config
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/persiaAziz/trafficserver TS-4263
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/938.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 #938
----
commit 63aca0a5749cf7d77f49e22bbbf82336f70f55e8
Author: Persia Aziz <pe...@yahoo-inc.com>
Date: 2016-08-26T18:15:12Z
TS-4263: Global key block configurable via Records.config
----
---
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 #938: TS-4263: Global key block configurable via Records...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/938
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/537/ 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 #938: TS-4263: Global key block configurable via Records...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/938
make clang-format
---
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 #938: TS-4263: Global key block configurable via ...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/938#discussion_r76831604
--- Diff: iocore/net/SSLUtils.cc ---
@@ -544,7 +547,64 @@ ssl_context_enable_ecdh(SSL_CTX *ctx)
return ctx;
}
+static ssl_ticket_key_block *
+ssl_create_ticket_keyblock(const char *ticket_key_path)
--- End diff --
Yes most of the code are duplicated from the ssl_context_enable_tickets, but in this function I am just creating the key block not enabling session ticket for any particular context, So I decided to make a separate function. What we can do is have ssl_context_enable_ticket call ssl_create_ticket_keyblock. That way the code will look cleaner
---
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 #938: TS-4263: Global key block configurable via ...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/938#discussion_r76835010
--- Diff: iocore/net/SSLUtils.cc ---
@@ -544,7 +547,64 @@ ssl_context_enable_ecdh(SSL_CTX *ctx)
return ctx;
}
+static ssl_ticket_key_block *
+ssl_create_ticket_keyblock(const char *ticket_key_path)
--- End diff --
Exactly. :)
---
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 #938: TS-4263: Global key block configurable via ...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/938#discussion_r76828573
--- Diff: iocore/net/SSLUtils.cc ---
@@ -544,7 +547,64 @@ ssl_context_enable_ecdh(SSL_CTX *ctx)
return ctx;
}
+static ssl_ticket_key_block *
+ssl_create_ticket_keyblock(const char *ticket_key_path)
--- End diff --
Hmmm, maybe I'm missing something, but it seems most of this code is duplicated from ssl_context_enable_tickets() ? Can we not refactor this such that we don't get such massive code duplication?
---
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 #938: TS-4263: Global key block configurable via ...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz closed the pull request at:
https://github.com/apache/trafficserver/pull/938
---
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 #938: TS-4263: Global key block configurable via Records...
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/938
[approve ci]
---
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 #938: TS-4263: Global key block configurable via Records...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/938
Linux build *failed*! See https://ci.trafficserver.apache.org/job/Github-Linux/535/ 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 #938: TS-4263: Global key block configurable via Records...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on the issue:
https://github.com/apache/trafficserver/pull/938
[approve ci]
---
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 #938: TS-4263: Global key block configurable via Records...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/938
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/641/ 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 #938: TS-4263: Global key block configurable via Records...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/938
Closing this PR since I messed up my branch with git [AGAIN!]
---
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 #938: TS-4263: Global key block configurable via Records...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on the issue:
https://github.com/apache/trafficserver/pull/938
Thanks @zwoop . I was using a tool for clang-formatting.
Please approve
---
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 #938: TS-4263: Global key block configurable via ...
Posted by zwoop <gi...@git.apache.org>.
Github user zwoop commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/938#discussion_r76828078
--- Diff: iocore/net/SSLUtils.cc ---
@@ -95,6 +95,8 @@ struct ssl_user_config {
ssl_user_config() : opt(SSLCertContext::OPT_NONE)
{
REC_ReadConfigInt32(session_ticket_enabled, "proxy.config.ssl.server.session_ticket.enable");
+ REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename");
+ Debug("ssl", "ticket key filename %s", (const char *)ticket_key_filename);
}
int session_ticket_enabled; // ssl_ticket_enabled - session ticket enabled
--- End diff --
I know it's not in your patch, but can you fix the line where we declare ticket_key_filename such that clang-format doesn't wrap the line? I.e. make the comment shorter (or multiple lines). It just looked really odd, so good time to clean that up here :).
---
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 #938: TS-4263: Global key block configurable via ...
Posted by persiaAziz <gi...@git.apache.org>.
Github user persiaAziz commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/938#discussion_r76832926
--- Diff: iocore/net/SSLUtils.cc ---
@@ -95,6 +95,8 @@ struct ssl_user_config {
ssl_user_config() : opt(SSLCertContext::OPT_NONE)
{
REC_ReadConfigInt32(session_ticket_enabled, "proxy.config.ssl.server.session_ticket.enable");
+ REC_ReadConfigStringAlloc(ticket_key_filename, "proxy.config.ssl.server.ticket_key.filename");
+ Debug("ssl", "ticket key filename %s", (const char *)ticket_key_filename);
}
int session_ticket_enabled; // ssl_ticket_enabled - session ticket enabled
--- End diff --
Sure :)
---
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 #938: TS-4263: Global key block configurable via Records...
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/938
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/639/ 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.
---