Skip to content

Conversation

@shinrich
Copy link
Member

Registered callback and added autest.

This closes #9563

@shinrich shinrich self-assigned this Mar 31, 2023
@shinrich shinrich force-pushed the reloadable-server-policy branch from 40cc654 to 73f9032 Compare March 31, 2023 19:28
@shinrich shinrich requested review from bneradt and brbzull0 March 31, 2023 21:42
@brbzull0
Copy link
Contributor

brbzull0 commented Apr 3, 2023

[approve ci cmake]

brbzull0
brbzull0 previously approved these changes Apr 3, 2023
Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a minitor comment that can be fixed in another PR probably, no biggie.

} 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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} 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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I would mention that this is now DISABLED

@shinrich
Copy link
Member Author

shinrich commented Apr 3, 2023

@brbzull0 good points. I pushed up another commit to address.

@shinrich shinrich force-pushed the reloadable-server-policy branch from 8dcfc2d to a385c9c Compare April 3, 2023 13:49
Copy link
Contributor

@brbzull0 brbzull0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@shinrich shinrich merged commit 6d1382d into master Apr 3, 2023
@zwoop
Copy link
Contributor

zwoop commented Aug 28, 2023

@shinrich @bneradt Do we want this back ported to 9.2.x ?

@shinrich
Copy link
Member Author

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.

@zwoop zwoop added this to the 10.0.0 milestone Aug 29, 2023
zwoop pushed a commit that referenced this pull request Aug 29, 2023
…loadable (#9572)

(cherry picked from commit 6d1382d)

 Conflicts:
	iocore/net/P_SSLConfig.h
@zwoop
Copy link
Contributor

zwoop commented Aug 29, 2023

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.3 Aug 29, 2023
@shinrich shinrich deleted the reloadable-server-policy branch September 19, 2023 13:33
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* asf/master: (42 commits)
  Add logic to make the server.policy and server.properties settings reloadable (apache#9572)
  Add CMake to the required PR CI builds (apache#9575)
  fixup cmake build for master and add conditional for io_uring support (apache#9571)
  Cleanup: Use swoc::meta instead of ts::meta. (apache#9566)
  codeql 24: Multiplication result converted to larger type (apache#9569)
  Drop support for old quiche (apache#9561)
  QUIC: Ignore default_inactivity_timeout in favour of proxy.config.quic.no_activity_timeout_in. (apache#9564)
  Fix log format specifications (apache#9568)
  Add `current_time_epoch_ms` stat to be appended before the server version. This allows computation of stats externally based on the cache time frame. This can help alleviate issues with sliding windows between various stats programs that generate discrepencies (apache#9567)
  Define BIO macros in ink_ssl.h (apache#9557)
  combine UDPPacket and UDPPacketInternal (apache#9424)
  Update codeql.yml (apache#9560)
  Http2 to origin (apache#9366)
  coverity 1497413: Use of 32-bit time_t (apache#9556)
  Add support for multiple yaml config files for wasm plugin (apache#9483)
  Add TS_HAS_QUICHE feature variable. (apache#9547)
  mime header field parsing fix trailing quote handlling (apache#9513)
  Make magick plugin buildable with BoringSSL (apache#9554)
  QUIC: Test basic scenarios around the ts.quic.no_activity_timeout_in config. (apache#9543)
  Fix records events deps (apache#9511)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

proxy.config.ssl.client.verify.server.policy and proxy.config.ssl.client.verify.server.properties are not reloadable

3 participants