You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by masaori335 <gi...@git.apache.org> on 2016/09/08 07:42:26 UTC
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
GitHub user masaori335 opened a pull request:
https://github.com/apache/trafficserver/pull/990
TS-3216: Add HPKP Support
- A Pull-Request for hpkp-003.patch on [TS-3216](https://issues.apache.org/jira/browse/TS-3216)
- Differences from hpkp-003.patch on TS-3216 is below
- Rebase on latest master
- Remove calculation of pins from certs
- Add a configuration to specify comma separated list of pins
Recently I found a use case that pinning only root and intermediate certificate. ( e.g. [Analysis github.com on report-uri](https://report-uri.io/home/pkp_analyse/https%3A%2F%2Fgithub.com) )
To support this use case and add many backup pins, add a configuration of list of pins.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/masaori335/trafficserver ts-3216
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/990.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 #990
----
commit e83aba58688d4d63b0eb8b718553510dd93a1c74
Author: Masaori Koshiba <ma...@apache.org>
Date: 2016-08-30T02:53:24Z
TS-3216: Add HPKP Support
----
---
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 #990: TS-3216: Add HPKP Support
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/990
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/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.
---
[GitHub] trafficserver pull request #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r80829654
--- Diff: proxy/hdrs/HdrToken.cc ---
@@ -209,6 +210,8 @@ static HdrTokenFieldInfo _hdrtoken_strs_field_initializers[] = {
{"Proxy-Authorization", MIME_SLOTID_NONE, MIME_PRESENCE_PROXY_AUTHORIZATION, (HTIF_HOPBYHOP | HTIF_PROXYAUTH)},
{"Proxy-Connection", MIME_SLOTID_PROXY_CONNECTION, MIME_PRESENCE_PROXY_CONNECTION, (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)},
{"Public", MIME_SLOTID_NONE, MIME_PRESENCE_PUBLIC, (HTIF_COMMAS | HTIF_MULTVALS)},
+ {"Public-Key-Pins", MIME_SLOTID_NONE, MIME_PRESENCE_NONE, HTIF_NONE},
--- End diff --
@SolidWallOfCode Should I add those line end of this array? or just remove?
Actually I'm not strongly want to add WKS strings, I just followed Strict-Transport-Security header for HSTS.
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/990
@masaori335 Any thoughts about tests for this?
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78309860
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1866,10 +1934,17 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
keyblock = ssl_context_enable_tickets(ctx, NULL);
}
+ // Generate HPKP header if hpkp is enabled.
+ if (sslMultCertSettings.hpkp_enabled >= 0 ? sslMultCertSettings.hpkp_enabled : params->hpkp_enabled) {
--- End diff --
It is possible:) `sslMultCertSettings.hpkp_enabled` is initialized by `-1`, so settings in ssl_multicert.config (`0` or `1`) is preceded if they're written. This is expected behavior, 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 pull request #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78202428
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1866,10 +1934,17 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
keyblock = ssl_context_enable_tickets(ctx, NULL);
}
+ // Generate HPKP header if hpkp is enabled.
+ if (sslMultCertSettings.hpkp_enabled >= 0 ? sslMultCertSettings.hpkp_enabled : params->hpkp_enabled) {
--- End diff --
So it is not possible to enable HPKP globally then disable it on specific certificates? I think the expected behavior is for the certificate settings to always have precedence (if they are specified). See [TS-2773](https://issues.apache.org/jira/browse/TS-2773) for
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 closed the pull request at:
https://github.com/apache/trafficserver/pull/990
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r80831720
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1895,10 +1963,17 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
keyblock = ssl_context_enable_tickets(ctx, NULL);
}
+ // Generate HPKP header if hpkp is enabled.
+ if (sslMultCertSettings.hpkp_enabled >= 0 ? sslMultCertSettings.hpkp_enabled : params->hpkp_enabled) {
+ hpkp = ssl_context_enable_hpkp(params, sslMultCertSettings);
--- End diff --
It looks like `SSLInitServerContext` doesn't handle SSLCertContext. What I'm missing?
---
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 #990: TS-3216: Add HPKP Support
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/990
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/779/ 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 #990: TS-3216: Add HPKP Support
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/990
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/743/ 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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78421317
--- Diff: proxy/http/HttpConfig.cc ---
@@ -1178,6 +1180,8 @@ HttpConfig::reconfigure()
params->oride.proxy_response_hsts_max_age = m_master.oride.proxy_response_hsts_max_age;
params->oride.proxy_response_hsts_include_subdomains = m_master.oride.proxy_response_hsts_include_subdomains;
+ params->oride.proxy_response_hpkp_enabled = m_master.oride.proxy_response_hpkp_enabled;
--- End diff --
Is this used anywhere?
---
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 #990: TS-3216: Add HPKP Support
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78425356
--- Diff: proxy/hdrs/HdrToken.cc ---
@@ -209,6 +210,8 @@ static HdrTokenFieldInfo _hdrtoken_strs_field_initializers[] = {
{"Proxy-Authorization", MIME_SLOTID_NONE, MIME_PRESENCE_PROXY_AUTHORIZATION, (HTIF_HOPBYHOP | HTIF_PROXYAUTH)},
{"Proxy-Connection", MIME_SLOTID_PROXY_CONNECTION, MIME_PRESENCE_PROXY_CONNECTION, (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)},
{"Public", MIME_SLOTID_NONE, MIME_PRESENCE_PUBLIC, (HTIF_COMMAS | HTIF_MULTVALS)},
+ {"Public-Key-Pins", MIME_SLOTID_NONE, MIME_PRESENCE_NONE, HTIF_NONE},
--- End diff --
This could break cache compatibility because it re-orders the WKS strings.
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78200394
--- Diff: iocore/net/SSLCertLookup.cc ---
@@ -167,6 +167,10 @@ SSLCertContext::release()
keyblock = NULL;
}
+ if (hpkp) {
+ ats_free(hpkp);
--- End diff --
You don't need the NULL check before ats_free.
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on the issue:
https://github.com/apache/trafficserver/pull/990
@SolidWallOfCode can you please look at the WKS changes?
---
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 #990: TS-3216: Add HPKP Support
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/990
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/660/ 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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78203123
--- Diff: iocore/net/SSLUtils.cc ---
@@ -344,6 +361,11 @@ set_context_cert(SSL *ssl)
ctx = cc->ctx;
}
+ if (cc && cc->hpkp) {
+ netvc->hpkp = cc->hpkp;
--- End diff --
Who owns the ``hpkp`` pointer at this time?
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78419692
--- Diff: mgmt/RecordsConfig.cc ---
@@ -1302,6 +1302,24 @@ static const RecordElement RecordsConfig[] =
//##############################################################################
//#
+ //# HPKP (Public Key Pinning Extension for HTTP) Configuration
+ //#
+ //##############################################################################
+ {RECT_CONFIG, "proxy.config.ssl.hpkp.enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-1]", RECA_NULL}
+ ,
+ {RECT_CONFIG, "proxy.config.ssl.hpkp.report_only", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-1]", RECA_NULL}
+ ,
+ {RECT_CONFIG, "proxy.config.ssl.hpkp.include_subdomains", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
+ ,
+ {RECT_CONFIG, "proxy.config.ssl.hpkp.max_age", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL}
+ ,
+ {RECT_CONFIG, "proxy.config.ssl.hpkp.pins", RECD_STRING, NULL, RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
+ ,
+ {RECT_CONFIG, "proxy.config.ssl.hpkp.report_uri", RECD_STRING, NULL, RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
+ ,
--- End diff --
For these to actually be dynamic you need to register for updates to them, most likely in ``SSLCertificateConfig::startup()``.
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78311103
--- Diff: iocore/net/SSLUtils.cc ---
@@ -344,6 +361,11 @@ set_context_cert(SSL *ssl)
ctx = cc->ctx;
}
+ if (cc && cc->hpkp) {
+ netvc->hpkp = cc->hpkp;
--- End diff --
Right, it could be problem.
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78773240
--- Diff: iocore/net/SSLNetVConnection.cc ---
@@ -951,6 +955,11 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
return EVENT_DONE;
}
+ if (cc && cc->hpkp) {
+ this->hpkp = (HPKP *)ats_malloc(sizeof(HPKP));
+ memcpy(this->hpkp, cc->hpkp, sizeof(HPKP));
--- End diff --
Also, is there an arena we can allocate this on?
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on the issue:
https://github.com/apache/trafficserver/pull/990
I'm going to fix issues and add a test by TSQA.
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78418739
--- Diff: iocore/net/SSLConfig.cc ---
@@ -81,6 +81,13 @@ SSLConfigParams::SSLConfigParams()
ssl_session_cache_timeout = 0;
ssl_session_cache_auto_clear = 1;
configExitOnLoadError = 0;
+
+ hpkp_enabled = 0;
+ hpkp_report_only = 0;
+ hpkp_include_subdomains = 0;
+ hpkp_max_age = 0;
+ hpkp_pins = NULL;
+ hpkp_report_uri = NULL;
--- End diff --
Do these need to be initialized to -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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78418970
--- Diff: iocore/net/SSLNetVConnection.cc ---
@@ -951,6 +955,11 @@ SSLNetVConnection::sslStartHandShake(int event, int &err)
return EVENT_DONE;
}
+ if (cc && cc->hpkp) {
+ this->hpkp = (HPKP *)ats_malloc(sizeof(HPKP));
+ memcpy(this->hpkp, cc->hpkp, sizeof(HPKP));
--- End diff --
How about in C++?
```C
this->hpkp = new HPKP(*cc->hpkp);
```
---
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 #990: TS-3216: Add HPKP Support
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/990
After a quick look this looks fine.
I'm not thrilled with storing the SSLCertContext in the SSLNetVConnection. I understand that you are doing this to get the specified settings for HPKP for that connection. But you also have two copies of the SSL_CTX (one in the SSLCertContext and the one used by openssl which is attainable from the ssl object). If there is a plugin that replaces the certificate in the certificate callback, these two values will be inconsistent. But I guess that isn't such a big deal. The alternative would be to add 3 or 4 bools directly to the SSLNetVConnection object and copy over the HPKP values directly. That is probably even worse.
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78310812
--- Diff: iocore/net/SSLUtils.cc ---
@@ -344,6 +361,11 @@ set_context_cert(SSL *ssl)
ctx = cc->ctx;
}
+ if (cc && cc->hpkp) {
+ netvc->hpkp = cc->hpkp;
--- End diff --
SSLCertContext still owns that.
Hmm, copying the values of hpkp is only one choice?
---
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 #990: TS-3216: Add HPKP Support
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/990
FreeBSD build *successful*! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/764/ 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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78201056
--- Diff: proxy/hdrs/HdrToken.cc ---
@@ -70,7 +70,8 @@ static const char *_hdrtoken_strs[] = {
"Newsgroups", // NNTP
"Organization", // NNTP
"Path", // NNTP
- "Pragma", "Proxy-Authenticate", "Proxy-Authorization", "Proxy-Connection", "Public", "Range",
+ "Pragma", "Proxy-Authenticate", "Proxy-Authorization", "Proxy-Connection", "Public-Key-Pins-Report-Only", "Public-Key-Pins",
+ "Public", "Range",
--- End diff --
Can you please use clang no-format so that this array is consistently formatted with on entry per line?
---
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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78422101
--- Diff: iocore/net/SSLUtils.cc ---
@@ -1895,10 +1963,17 @@ ssl_store_ssl_context(const SSLConfigParams *params, SSLCertLookup *lookup, cons
keyblock = ssl_context_enable_tickets(ctx, NULL);
}
+ // Generate HPKP header if hpkp is enabled.
+ if (sslMultCertSettings.hpkp_enabled >= 0 ? sslMultCertSettings.hpkp_enabled : params->hpkp_enabled) {
+ hpkp = ssl_context_enable_hpkp(params, sslMultCertSettings);
--- End diff --
This needs to be done from ``SSLInitServerContext`` so that it works correctly for ``TSSslServerContextCreate``.
---
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 #990: TS-3216: Add HPKP Support
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/990
Linux build *successful*! See https://ci.trafficserver.apache.org/job/Github-Linux/675/ 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 #990: TS-3216: Add HPKP Support
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r78310891
--- Diff: iocore/net/SSLUtils.cc ---
@@ -344,6 +361,11 @@ set_context_cert(SSL *ssl)
ctx = cc->ctx;
}
+ if (cc && cc->hpkp) {
+ netvc->hpkp = cc->hpkp;
--- End diff --
in that case, you are racing against configuration reload and will dereference a bad pointer if the transaction takes a long time.
---
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 #990: TS-3216: Add HPKP Support
Posted by shinrich <gi...@git.apache.org>.
Github user shinrich commented on the issue:
https://github.com/apache/trafficserver/pull/990
I like the change to store the pointer to hpkp in the netvc rather than pointer to the certificate context. Looks good to me.
---
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 #990: TS-3216: Add HPKP Support
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/990#discussion_r93656836
--- Diff: proxy/hdrs/HdrToken.cc ---
@@ -209,6 +210,8 @@ static HdrTokenFieldInfo _hdrtoken_strs_field_initializers[] = {
{"Proxy-Authorization", MIME_SLOTID_NONE, MIME_PRESENCE_PROXY_AUTHORIZATION, (HTIF_HOPBYHOP | HTIF_PROXYAUTH)},
{"Proxy-Connection", MIME_SLOTID_PROXY_CONNECTION, MIME_PRESENCE_PROXY_CONNECTION, (HTIF_COMMAS | HTIF_MULTVALS | HTIF_HOPBYHOP)},
{"Public", MIME_SLOTID_NONE, MIME_PRESENCE_PUBLIC, (HTIF_COMMAS | HTIF_MULTVALS)},
+ {"Public-Key-Pins", MIME_SLOTID_NONE, MIME_PRESENCE_NONE, HTIF_NONE},
--- End diff --
Add at the end.
---
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 #990: TS-3216: Add HPKP Support
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the issue:
https://github.com/apache/trafficserver/pull/990
@masaori335 Any update on this?
---
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 #990: TS-3216: Add HPKP Support
Posted by masaori335 <gi...@git.apache.org>.
Github user masaori335 commented on the issue:
https://github.com/apache/trafficserver/pull/990
Sorry, no updates. Now, I'm thinking about close this Pull-Request once and come back after ssl_multicert.config is replaced by lua.
---
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.
---