On Wed, 1 Nov 2023 07:35:48 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> This section of comments was taken from the CSR. I updated the comments as >> follows. If it looks fine, I will update the related doc. Thanks! >> >> /* >> * If either jdk.tls.server.maxInboundCertificateChainLength or >> * jdk.tls.client.maxInboundCertificateChainLength is set, it will >> * override jdk.tls.maxCertificateChainLength, regardless of whether >> * jdk.tls.maxCertificateChainLength is set or not. >> * If neither jdk.tls.server.maxInboundCertificateChainLength nor >> * jdk.tls.client.maxInboundCertificateChainLength is set, the >> behavior >> * depends on the setting of jdk.tls.maxCertificateChainLength. If >> * jdk.tls.maxCertificateChainLength is set, it falls back to that >> * value; otherwise, it defaults to 8 for >> * jdk.tls.server.maxInboundCertificateChainLength >> * and 10 for jdk.tls.client.maxInboundCertificateChainLength. >> * Usesrs can independently set either >> * jdk.tls.server.maxInboundCertificateChainLength or >> * jdk.tls.client.maxInboundCertificateChainLength. >> */ > > Sorry, I did not get time to review this behavior update. > >> This section of comments was taken from the CSR. I updated the comments as >> follows. If it looks fine, I will update the related doc. Thanks! >> >> ``` >> /* >> * If either jdk.tls.server.maxInboundCertificateChainLength or >> * jdk.tls.client.maxInboundCertificateChainLength is set, it will >> * override jdk.tls.maxCertificateChainLength, regardless of whether >> * jdk.tls.maxCertificateChainLength is set or not. > I'm not sure the statement is clear enough. I think there are two points > that need to clarify. The 1st one is that there is a default value of > jdk.tls.maxCertificateChainLength, which is 10. The 2nd one is that > jdk.tls.maxCertificateChainLength works for both client and server mode. > jdk.tls.server.maxInboundCertificateChainLength works on server mode, and it > does not work for client mode, and therefore it cannot override client mode > behavior for jdk.tls.maxCertificateChainLength. The same for > jdk.tls.client.maxInboundCertificateChainLength. > > To be clear, the release note or the comment might be placed in different > code block like: > > > /* If jdk.tls.server.maxInboundCertificateChainLength is set, it will > override jdk.tls.maxCertificateChainLength behavior for server side. > */ > > Integer inboundClientLen = ... > > /* If jdk.tls.client.maxInboundCertificateChainLength is set, it will > override jdk.tls.maxCertificateChainLength behavior for client side. > */ > Integer inboundServerLen = GetIntegerAction.privilegedGetProperty( > > > >> * If neither jdk.tls.server.maxInboundCertificateChainLength nor >> * jdk.tls.client.maxInboundCertificateChainLength is set, the >> behavior >> * depends on the setting of jdk.tls.maxCertificateChainLength. If >> * jdk.tls.maxCertificateChainLength is set, it falls back to that >> * value; otherwise, it defaults to 8 for >> * jdk.tls.server.maxInboundCertificateChainLength > > Previously, the jdk.tls.maxCertificateChainLength default value is 10. Now > it is changed to 8. This is a behavior change, please document it in release > note, or just use 10 in the implementation. > > >> * and 10 for jdk.tls.client.maxInboundCertificateChainLength. >> * Usesrs can independently set either >> * jdk.tls.server.maxInboundCertificateChainLength or >> * jdk.tls.client.maxInboundCertificateChainLength. >> */ >> ``` I don't see a behavior change that conflicts with the CSR. I think it is a wording issue, let me suggest some improvements in another comment. There is no longer a default value for `jdk.tls.maxCertificateChainLength`. Where is it set to 8 in the code? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15163#discussion_r1378814738
