On Wed, 1 Nov 2023 13:38:08 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> 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? When no system property is set, previously max inbound length is 10, now it's 8. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15163#discussion_r1378838176
