Re: RFR: 8264948: Check for TLS extensions total length [v2]
On Fri, 9 Apr 2021 19:29:50 GMT, Xue-Lei Andrew Fan wrote: >> To improve the readability, it would be nice to check the TLS extensions >> total length while parsing. >> >> No new regression test, trial update. > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > Change to use decode_error for incorrect extension length Looks good to me. - Marked as reviewed by jnimeh (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3405
Re: RFR: 8264948: Check for TLS extensions total length [v2]
On Fri, 9 Apr 2021 05:55:40 GMT, Jamil Nimeh wrote: >> Xue-Lei Andrew Fan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Change to use decode_error for incorrect extension length > > src/java.base/share/classes/sun/security/ssl/SSLExtensions.java line 68: > >> 66: Alert.ILLEGAL_PARAMETER, >> 67: "Insufficient extensions data"); >> 68: } > > For both of these blocks the checks themselves look OK, but illegal_parameter > I thought was more for cases where a field value is out of range or > inconsistent with already negotiated parameters. I would think that > decode_error would be more appropriate to cases like this where the encoding > is structurally incorrect and the length doesn't match the actual data size. Good catch! Updated to use decode_error. - PR: https://git.openjdk.java.net/jdk/pull/3405
Re: RFR: 8264948: Check for TLS extensions total length [v2]
> To improve the readability, it would be nice to check the TLS extensions > total length while parsing. > > No new regression test, trial update. Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Change to use decode_error for incorrect extension length - Changes: - all: https://git.openjdk.java.net/jdk/pull/3405/files - new: https://git.openjdk.java.net/jdk/pull/3405/files/5332d35d..1fa255c3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3405&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3405&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3405.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3405/head:pull/3405 PR: https://git.openjdk.java.net/jdk/pull/3405
Re: RFR: 8264948: Check for TLS extensions total length
On Fri, 9 Apr 2021 04:55:14 GMT, Xue-Lei Andrew Fan wrote: > To improve the readability, it would be nice to check the TLS extensions > total length while parsing. > > No new regression test, trial update. src/java.base/share/classes/sun/security/ssl/SSLExtensions.java line 68: > 66: Alert.ILLEGAL_PARAMETER, > 67: "Insufficient extensions data"); > 68: } For both of these blocks the checks themselves look OK, but illegal_parameter I thought was more for cases where a field value is out of range or inconsistent with already negotiated parameters. I would think that decode_error would be more appropriate to cases like this where the encoding is structurally incorrect and the length doesn't match the actual data size. - PR: https://git.openjdk.java.net/jdk/pull/3405
RFR: 8264948: Check for TLS extensions total length
To improve the readability, it would be nice to check the TLS extensions total length while parsing. No new regression test, trial update. - Commit messages: - 8264948: Check for TLS extensions total length Changes: https://git.openjdk.java.net/jdk/pull/3405/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3405&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264948 Stats: 12 lines in 1 file changed: 12 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3405.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3405/head:pull/3405 PR: https://git.openjdk.java.net/jdk/pull/3405
