Add back in the option to conf_remap the verify_server settings.#4663
Conversation
shinrich
force-pushed
the
make-verify-server-settings-overridable
branch
from
a1d7787 to
7143d53
Compare
|
|
||
| You can override this global setting on a per domain basis in the ssl_servername.yaml file using the :ref:`verify_server_properties attribute<override-verify-server-properties>`. | ||
|
|
||
| You can also override via the conf_remap plugin, but the changes in ssl_server_name.yaml will take precedence. |
There was a problem hiding this comment.
Shouldn't a transaction override be the highest precedence?
There was a problem hiding this comment.
It is kind of arbitrary. You really shouldn't be using both features together. So are long as we are deterministic and documented it shouldn't matter. Keeping the per-transaction as the higher precedence would complicate the implementation.
There was a problem hiding this comment.
Hrrmmm. A bit inconsistent with standard practice, we may want to fix that later.
| if (txn_conf->ssl_client_verify_server) { | ||
| verifyServer = txn_conf->ssl_client_verify_server; | ||
| } else { | ||
| REC_EstablishStaticConfigByte(verifyServer, "proxy.config.ssl.client.verify.server"); |
There was a problem hiding this comment.
This is done potentially on every upstream connection? That seems somewhat risky.
There was a problem hiding this comment.
For performance concerns? Or reload concerns? We could move it into the HttpConfig object I suppose.
There was a problem hiding this comment.
Alan clarified that I was using a call that read the value and set a callback. Changed the call to just read the value. No callback setting.
shinrich
force-pushed
the
make-verify-server-settings-overridable
branch
2 times, most recently
from
12d877b to
cb1937f
Compare
shinrich
force-pushed
the
make-verify-server-settings-overridable
branch
from
cb1937f to
a7a3a94
Compare
|
Pushed changes to allow the per-transaction override version to take precedence. Also removed verify.server (the original version) from being overridable. |
3 participants
In PR #4414 I made changes to clean up the verify_server* actions. As part of that PR I moved all the overridable options to ssl_server_name. When I brought those changes back to our environment, Dave Carlin pointed out that we needed access to the path to determine whether we could enforce server verification yet. In theory it could be done via the ssl_server_name file, but it was much more natural to be done via the remap rules.
However, in other cases, making exceptions for one or two origins, the ssl_server_name option makes much more sense.
This PR adds back in the logic to conf_remap override the verify_server* actions. If action is overridden in both locations, the value in ssl_server_name will win.
As part of the PR, I added a test to exercise the conf_remap option.