You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "shinrich (via GitHub)" <gi...@apache.org> on 2023/03/31 15:39:14 UTC
[GitHub] [trafficserver] shinrich opened a new pull request, #9572: Add logic to make the server.policy and server.properties settings reloadable
shinrich opened a new pull request, #9572:
URL: https://github.com/apache/trafficserver/pull/9572
Registered callback and added autest.
This closes #9563
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich merged pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "shinrich (via GitHub)" <gi...@apache.org>.
shinrich merged PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572#issuecomment-1494120084
[approve ci cmake]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on a diff in pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572#discussion_r1155826324
##########
iocore/net/SSLConfig.cc:
##########
@@ -189,6 +189,65 @@ set_paths_helper(const char *path, const char *filename, char **final_path, char
}
}
+int
+UpdateServerPolicy(const char * /* name ATS_UNUSED */, RecDataT /* data_type ATS_UNUSED */, RecData data, void *cookie)
+{
+ SSLConfigParams *params = SSLConfig::acquire();
+ char *verify_server = data.rec_string;
+ if (params != nullptr && verify_server != nullptr) {
+ Debug("ssl_load", "New Server Policy %s", verify_server);
+ params->SetServerPolicy(verify_server);
+ } else {
+ Debug("ssl_load", "Failed to load new Server Policy %p %p", verify_server, params);
+ }
+ return 0;
+}
+
+int
+UpdateServerPolicyProperties(const char * /* name ATS_UNUSED */, RecDataT /* data_type ATS_UNUSED */, RecData data, void *cookie)
+{
+ SSLConfigParams *params = SSLConfig::acquire();
+ char *verify_server = data.rec_string;
+ if (params != nullptr && verify_server != nullptr) {
+ params->SetServerPolicyProperties(verify_server);
+ }
+ return 0;
+}
+
+void
+SSLConfigParams::SetServerPolicyProperties(const char *verify_server)
+{
+ if (strcmp(verify_server, "SIGNATURE") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::SIGNATURE_MASK;
+ } else if (strcmp(verify_server, "NAME") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::NAME_MASK;
+ } else if (strcmp(verify_server, "ALL") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::ALL_MASK;
+ } else if (strcmp(verify_server, "NONE") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::NONE;
+ } else {
+ Warning("%s is invalid for proxy.config.ssl.client.verify.server.properties. Should be one of SIGNATURE, NAME, or ALL",
+ verify_server);
+ verifyServerProperties = YamlSNIConfig::Property::NONE;
+ }
+}
+
+void
+SSLConfigParams::SetServerPolicy(const char *verify_server)
+{
+ if (strcmp(verify_server, "DISABLED") == 0) {
+ verifyServerPolicy = YamlSNIConfig::Policy::DISABLED;
+ } else if (strcmp(verify_server, "PERMISSIVE") == 0) {
+ verifyServerPolicy = YamlSNIConfig::Policy::PERMISSIVE;
+ } else if (strcmp(verify_server, "ENFORCED") == 0) {
+ verifyServerPolicy = YamlSNIConfig::Policy::ENFORCED;
+ } else {
+ Warning("%s is invalid for proxy.config.ssl.client.verify.server.policy. Should be one of DISABLED, PERMISSIVE, or ENFORCED",
Review Comment:
Same here, I would mention that this is now `DISABLED`
##########
iocore/net/SSLConfig.cc:
##########
@@ -189,6 +189,65 @@ set_paths_helper(const char *path, const char *filename, char **final_path, char
}
}
+int
+UpdateServerPolicy(const char * /* name ATS_UNUSED */, RecDataT /* data_type ATS_UNUSED */, RecData data, void *cookie)
+{
+ SSLConfigParams *params = SSLConfig::acquire();
+ char *verify_server = data.rec_string;
+ if (params != nullptr && verify_server != nullptr) {
+ Debug("ssl_load", "New Server Policy %s", verify_server);
+ params->SetServerPolicy(verify_server);
+ } else {
+ Debug("ssl_load", "Failed to load new Server Policy %p %p", verify_server, params);
+ }
+ return 0;
+}
+
+int
+UpdateServerPolicyProperties(const char * /* name ATS_UNUSED */, RecDataT /* data_type ATS_UNUSED */, RecData data, void *cookie)
+{
+ SSLConfigParams *params = SSLConfig::acquire();
+ char *verify_server = data.rec_string;
+ if (params != nullptr && verify_server != nullptr) {
+ params->SetServerPolicyProperties(verify_server);
+ }
+ return 0;
+}
+
+void
+SSLConfigParams::SetServerPolicyProperties(const char *verify_server)
+{
+ if (strcmp(verify_server, "SIGNATURE") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::SIGNATURE_MASK;
+ } else if (strcmp(verify_server, "NAME") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::NAME_MASK;
+ } else if (strcmp(verify_server, "ALL") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::ALL_MASK;
+ } else if (strcmp(verify_server, "NONE") == 0) {
+ verifyServerProperties = YamlSNIConfig::Property::NONE;
+ } else {
+ Warning("%s is invalid for proxy.config.ssl.client.verify.server.properties. Should be one of SIGNATURE, NAME, or ALL",
Review Comment:
I know this was already in the code but(not big issue) I would probably mention that `NONE` will be set in this case as per the docs `ALL` seems the default, it could be misleading and someone may think `ALL` will be used in this case.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "shinrich (via GitHub)" <gi...@apache.org>.
shinrich commented on PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572#issuecomment-1494339581
@brbzull0 good points. I pushed up another commit to address.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572#issuecomment-1698006775
Cherry-picked to v9.2.x
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "zwoop (via GitHub)" <gi...@apache.org>.
zwoop commented on PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572#issuecomment-1696515957
@shinrich @bneradt Do we want this back ported to 9.2.x ?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #9572: Add logic to make the server.policy and server.properties settings reloadable
Posted by "shinrich (via GitHub)" <gi...@apache.org>.
shinrich commented on PR #9572:
URL: https://github.com/apache/trafficserver/pull/9572#issuecomment-1697856186
Seems like a bug fix that would be good to backport. I'm not relying on this fix however. So getting it in 10.x would be good enough for me. Seems low risk in any case.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org