Skip to content

Commit 2c4521c

Browse files
committed
Return X500Principal from getPeerPrincipal() and getLocalPrincipal()
1 parent 276eff7 commit 2c4521c

20 files changed

Lines changed: 4106 additions & 26 deletions

claude_review_reply.txt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
Thanks for the detailed review. Here is how each Claude item was handled on the current branch:
2+
3+
1. `pendingAppData` can go stale on error paths: Fixed. `WolfSSLEngine` now uses `clearPendingAppData()` to clear `pendingAppData`, `pendingAppDataLen`, and `pendingNetConsumed` on `unwrap()` exception paths and in `closeInbound()` / `closeOutbound()`. That prevents stale consumed offsets from being applied on a later successful `unwrap()`.
4+
5+
2. C89 declaration-after-statement in DTLS drop count functions: Fixed. The `word32 dropCount` declarations in `com_wolfssl_WolfSSLSession.c` were moved before the `(void)` statements so the code remains valid for stricter C89/C90 compilers.
6+
7+
3. SNI parsing allocates on every server-side `unwrap()` during handshake: Addressed. `WolfSSLEngine` now uses `WolfSSL.getSNIFromBuffer()` first for pending-input SNI extraction, so the old Java parser is no longer the default path on supported builds. The old parser is retained only as a `NOT_COMPILED_IN` fallback for builds where the native SNI parser is unavailable, including the current FIPS build path.
8+
9+
4. `verifyHostnameForExternalTM` allocates a new `WolfSSLCertificate` from DER each time: Reviewed, no change in this pass. The concern is valid, but the proper fix changes verification flow by moving hostname verification earlier and relying more on native hostname setup/checking. That is larger than this bug-fix pass.
10+
11+
5. `setEnabledCipherSuites()` / `setEnabledProtocols()` throw unchecked `IllegalArgumentException`: Reviewed, no change. This matches normal JSSE behavior, including SunJSSE, where invalid setter input is surfaced as `IllegalArgumentException`.
12+
13+
6. `setNativeTimeout` catch block swallows exception silently: Fixed. Added `WolfSSLDebug.log()` in that catch path before invalidating the session entry so the failure is diagnosable.
14+
15+
7. README wrong class name in three places: Intentionally not changed. This is documentation-only and was left out to keep the branch focused on code and test fixes.
16+
17+
8. README trailing whitespace: Intentionally not changed. This is documentation-only and was left out to keep the branch focused on code and test fixes.
18+
19+
9. `getBytes()` validates `len < 0` after allocation: Fixed. The bounds check now runs before `new byte[len]`, removing the `NegativeArraySizeException` footgun.
20+
21+
10. `getPeerPrincipal()` now calls heavier `getPeerCertificates()`: Reviewed, no change. The extra cost is intentional because the change preserves standard Java certificate handling and `X500Name.equals()` compatibility.
22+
23+
11. Session context `getIds()` / `getSession()` now call `updateTimeouts()` eagerly: Partially addressed. The eager timeout invalidation remains by design for JSSE correctness, but `getIds()` no longer does the extra N+1 session lookups. Valid-ID filtering now happens inside `WolfSSLAuthStore.getAllIDs()` in one store pass, and `WolfSSLSessionContext.getIds()` returns that directly.
24+
25+
12. Trailing newline removals: No functional issue; no change needed.
26+
27+
Native reuse suggestions:
28+
29+
13. Replace Java SNI parsing with `wolfSSL_SNI_GetFromBuffer()`: Addressed. `WolfSSLEngine` now uses `WolfSSL.getSNIFromBuffer()` first. The JNI helper itself comes from upstream PR `#339`; this branch updates the JSSE-side call site to use it.
30+
31+
14. Lean more on native cipher-list handling for TLS 1.3 vs legacy partitioning: Reviewed, no change in this pass. This would require additional native API surface and broader retesting.
32+
33+
15. Replace manual hostname verification with pre-handshake native hostname setup: Reviewed, no change in this pass. Valid long-term cleanup, but it changes handshake setup and verification ordering and is larger than the fixes here.
34+
35+
16. TLS record completeness check should be documented as TLS-only: Partially addressed. The optimization was kept, and a clarifying comment was added that it is TLS-only and that native `WANT_READ` remains the correctness fallback.
36+
37+
17. Session timeout expiration could delegate more to native: Reviewed, no change. The Java session cache is distinct from the native session cache, so native `flush_sessions` does not replace the current Java-side invalidation behavior.
38+
39+
In addition to the items above, two related performance/code-quality issues raised in Copilot review were also addressed during this pass: `RecvAppData()` now reuses a per-engine scratch buffer instead of allocating a new `byte[]` on every call, and `cacheRequestedServerNamesFromNetData()` now uses an explicit local session lookup with a null guard before reading or caching SNI names.
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
#Ant JUnitTask generated properties file
2+
#Mon Mar 02 17:02:50 UTC 2026
3+
env.TERM=dumb
4+
java.specification.version=17
5+
examples.build.dir=examples/build
6+
java.debug=true
7+
sun.arch.data.model=64
8+
env.GIT_PAGER=cat
9+
java.vendor.url=https\://tracker.debian.org/openjdk-17
10+
sun.boot.library.path=/usr/lib/jvm/java-17-openjdk-amd64/lib
11+
sun.java.command=org.apache.tools.ant.launch.Launcher -cp test
12+
jdk.debug=release
13+
java.specification.vendor=Oracle Corporation
14+
have-nativeheaderdir=true
15+
java.version.date=2026-01-20
16+
java.home=/usr/lib/jvm/java-17-openjdk-amd64
17+
basedir=/home/claude/wolfssl-containers/java/wolfssl-openjdk-fips-root/wolfssljni
18+
java.specification.name=Java Platform API Specification
19+
java.vm.specification.vendor=Oracle Corporation
20+
doc.dir=docs
21+
env.NO_COLOR=1
22+
sun.management.compiler=HotSpot 64-Bit Tiered Compilers
23+
java.runtime.version=17.0.18+8-Debian-1deb12u1
24+
java.optimize=false
25+
env.PATH=/home/claude/.local/bin\:/usr/local/bin\:/usr/bin\:/bin\:/usr/local/games\:/usr/games
26+
implementation.title=wolfSSL JNI/JSSE
27+
env.LS_COLORS=rs\=0\:di\=01;34\:ln\=01;36\:mh\=00\:pi\=40;33\:so\=01;35\:do\=01;35\:bd\=40;33;01\:cd\=40;33;01\:or\=40;31;01\:mi\=00\:su\=37;41\:sg\=30;43\:ca\=00\:tw\=30;42\:ow\=34;42\:st\=37;44\:ex\=01;32\:*.tar\=01;31\:*.tgz\=01;31\:*.arc\=01;31\:*.arj\=01;31\:*.taz\=01;31\:*.lha\=01;31\:*.lz4\=01;31\:*.lzh\=01;31\:*.lzma\=01;31\:*.tlz\=01;31\:*.txz\=01;31\:*.tzo\=01;31\:*.t7z\=01;31\:*.zip\=01;31\:*.z\=01;31\:*.dz\=01;31\:*.gz\=01;31\:*.lrz\=01;31\:*.lz\=01;31\:*.lzo\=01;31\:*.xz\=01;31\:*.zst\=01;31\:*.tzst\=01;31\:*.bz2\=01;31\:*.bz\=01;31\:*.tbz\=01;31\:*.tbz2\=01;31\:*.tz\=01;31\:*.deb\=01;31\:*.rpm\=01;31\:*.jar\=01;31\:*.war\=01;31\:*.ear\=01;31\:*.sar\=01;31\:*.rar\=01;31\:*.alz\=01;31\:*.ace\=01;31\:*.zoo\=01;31\:*.cpio\=01;31\:*.7z\=01;31\:*.rz\=01;31\:*.cab\=01;31\:*.wim\=01;31\:*.swm\=01;31\:*.dwm\=01;31\:*.esd\=01;31\:*.avif\=01;35\:*.jpg\=01;35\:*.jpeg\=01;35\:*.mjpg\=01;35\:*.mjpeg\=01;35\:*.gif\=01;35\:*.bmp\=01;35\:*.pbm\=01;35\:*.pgm\=01;35\:*.ppm\=01;35\:*.tga\=01;35\:*.xbm\=01;35\:*.xpm\=01;35\:*.tif\=01;35\:*.tiff\=01;35\:*.png\=01;35\:*.svg\=01;35\:*.svgz\=01;35\:*.mng\=01;35\:*.pcx\=01;35\:*.mov\=01;35\:*.mpg\=01;35\:*.mpeg\=01;35\:*.m2v\=01;35\:*.mkv\=01;35\:*.webm\=01;35\:*.webp\=01;35\:*.ogm\=01;35\:*.mp4\=01;35\:*.m4v\=01;35\:*.mp4v\=01;35\:*.vob\=01;35\:*.qt\=01;35\:*.nuv\=01;35\:*.wmv\=01;35\:*.asf\=01;35\:*.rm\=01;35\:*.rmvb\=01;35\:*.flc\=01;35\:*.avi\=01;35\:*.fli\=01;35\:*.flv\=01;35\:*.gl\=01;35\:*.dl\=01;35\:*.xcf\=01;35\:*.xwd\=01;35\:*.yuv\=01;35\:*.cgm\=01;35\:*.emf\=01;35\:*.ogv\=01;35\:*.ogx\=01;35\:*.aac\=00;36\:*.au\=00;36\:*.flac\=00;36\:*.m4a\=00;36\:*.mid\=00;36\:*.midi\=00;36\:*.mka\=00;36\:*.mp3\=00;36\:*.mpc\=00;36\:*.ogg\=00;36\:*.ra\=00;36\:*.wav\=00;36\:*.oga\=00;36\:*.opus\=00;36\:*.spx\=00;36\:*.xspf\=00;36\:*~\=00;90\:*\#\=00;90\:*.bak\=00;90\:*.old\=00;90\:*.orig\=00;90\:*.part\=00;90\:*.rej\=00;90\:*.swp\=00;90\:*.tmp\=00;90\:*.dpkg-dist\=00;90\:*.dpkg-old\=00;90\:*.ucf-dist\=00;90\:*.ucf-new\=00;90\:*.ucf-old\=00;90\:*.rpmnew\=00;90\:*.rpmorig\=00;90\:*.rpmsave\=00;90\:
28+
file.encoding=UTF-8
29+
env._=/usr/bin/ant
30+
env.SHLVL=1
31+
java.io.tmpdir=/tmp
32+
java.version=17.0.18
33+
java.vm.specification.name=Java Virtual Machine Specification
34+
lib.dir=lib/
35+
native.encoding=UTF-8
36+
java.library.path=/usr/java/packages/lib\:/usr/lib/x86_64-linux-gnu/jni\:/lib/x86_64-linux-gnu\:/usr/lib/x86_64-linux-gnu\:/usr/lib/jni\:/lib\:/usr/lib
37+
java.vendor=Debian
38+
java.specification.maintenance.version=1
39+
ant.file.type.wolfssl=file
40+
sun.io.unicode.encoding=UnicodeLittle
41+
junit4=junit-4.13.2.jar
42+
env.CODEX_CI=1
43+
ant.file.wolfssl=/home/claude/wolfssl-containers/java/wolfssl-openjdk-fips-root/wolfssljni/build.xml
44+
ant.file.type=file
45+
env.LANG=C.UTF-8
46+
test.build.dir=build/test
47+
env.SSH_CLIENT=153.90.84.228 37056 22
48+
os.name=Linux
49+
java.vm.specification.version=17
50+
test.dir=src/test/
51+
user.home=/home/claude
52+
env.SSH_CONNECTION=153.90.84.228 37056 153.90.84.135 22
53+
env.COLORTERM=
54+
env.CODEX_MANAGED_BY_NPM=1
55+
native.dir=native
56+
examples.dir=examples/
57+
java.security.manager=allow
58+
ant.file=/home/claude/wolfssl-containers/java/wolfssl-openjdk-fips-root/wolfssljni/build.xml
59+
path.separator=\:
60+
os.version=6.17.2-1-pve
61+
java.vm.name=OpenJDK 64-Bit Server VM
62+
env.SHELL=/bin/bash
63+
implementation.version=1.16
64+
os.arch=amd64
65+
env.CODEX_THREAD_ID=019c96f4-7cd2-78c0-bcae-334f384d6344
66+
java.debuglevel=source,lines,vars
67+
ant-junit4=ant/ant-junit4.jar
68+
env.GH_PAGER=cat
69+
java.vm.info=mixed mode, sharing
70+
java.class.version=61.0
71+
env.USER=claude
72+
src.dir=src/java/
73+
ant.project.default-target=build
74+
ant.library.dir=/usr/share/ant/lib
75+
ant.project.name=wolfssl
76+
sun.jnu.encoding=UTF-8
77+
implementation.vendor=wolfSSL Inc.
78+
hamcrest-core=hamcrest-all-1.3.jar
79+
file.separator=/
80+
java.vm.compressedOopsMode=Zero based
81+
line.separator=\n
82+
env.SSH_TTY=/dev/pts/3
83+
java.source=1.8
84+
user.name=claude
85+
env.LOGNAME=claude
86+
isJava9Plus=true
87+
have-SNIHostName=true
88+
build.dir=build
89+
ant.home=/usr/share/ant
90+
env.MOTD_SHOWN=pam
91+
ant.version=Apache Ant(TM) version 1.10.13 compiled on February 6 2023
92+
env.PWD=/home/claude/wolfssl-containers/java/wolfssl-openjdk-fips-root/wolfssljni
93+
java.deprecation=true
94+
env.LC_CTYPE=C.UTF-8
95+
env.PAGER=cat
96+
java.class.path=/usr/share/ant/lib/ant-launcher.jar\:/usr/share/ant/lib/ant-antlr.jar\:/usr/share/ant/lib/ant-jsch.jar\:/usr/share/ant/lib/ant-swing.jar\:/usr/share/ant/lib/ant-apache-regexp.jar\:/usr/share/ant/lib/ant-apache-log4j.jar\:/usr/share/ant/lib/ant-junitlauncher.jar\:/usr/share/ant/lib/ant-jmf.jar\:/usr/share/ant/lib/ant-commons-net.jar\:/usr/share/ant/lib/ant-apache-resolver.jar\:/usr/share/ant/lib/ant-xz.jar\:/usr/share/ant/lib/ant-apache-bcel.jar\:/usr/share/ant/lib/ant-testutil.jar\:/usr/share/ant/lib/ant-apache-bsf.jar\:/usr/share/ant/lib/ant.jar\:/usr/share/ant/lib/ant-jdepend.jar\:/usr/share/ant/lib/ant-junit.jar\:/usr/share/ant/lib/ant-junit4.jar\:/usr/share/ant/lib/ant-launcher.jar\:/usr/share/ant/lib/ant-javamail.jar\:/usr/share/ant/lib/ant-commons-logging.jar\:/usr/share/ant/lib/ant-apache-oro.jar\:/usr/share/ant/lib/ant-apache-xalan2.jar
97+
env.HOME=/home/claude
98+
java.vm.vendor=Debian
99+
reports.dir=build/reports
100+
java.target=1.8
101+
sun.java.launcher=SUN_STANDARD
102+
sun.cpu.endian=little
103+
user.language=en
104+
ant.java.version=17
105+
env.LC_ALL=C.UTF-8
106+
src.java9.dir=src/java9/
107+
java.runtime.name=OpenJDK Runtime Environment
108+
ant.project.invoked-targets=test
109+
env.JUNIT_HOME=/tmp/wolfssljni-junit
110+
ant.core.lib=/usr/share/ant/lib/ant.jar
111+
java.vendor.url.bug=https\://bugs.debian.org/openjdk-17
112+
user.dir=/home/claude/wolfssl-containers/java/wolfssl-openjdk-fips-root/wolfssljni
113+
java.vm.version=17.0.18+8-Debian-1deb12u1
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
testSetWantNeedClientAuth_ClientNoKeyManager

0 commit comments

Comments
 (0)