Merged
Conversation
If we tried to transfer SecretBytes via Remoting then the remote side would end up with the encrypted byte[] but not the key needed to decrypt the data. Sending the decryption key would be a bad thing, but we can send the data in plain text to the agent and then re-encrypt it with the remoting (agent) sides key. As generally the remoting side would not be a Jenkins the mock encryption key would be used in this case. Whilst the mock key is static, it is not expected that ian agent persists these SecretBytes. fixes: jenkinsci#1013
jtnord
force-pushed
the
secretBytes-remoting
branch
from
95622b3 to
1d420ac
Compare
jtnord
commented
Feb 1, 2026
|
|
||
| @Override | ||
| public Void call() throws AssertionError { | ||
| assertThat(sb.getPlainData(), is(expectedUnencryptedValue)); |
Member
Author
There was a problem hiding this comment.
prior to the fix this errored with
java.io.IOException: Remote call on slave0 failed
at hudson.remoting.Channel.call(Channel.java:1112)
at com.cloudbees.plugins.credentials.SecretBytesTest.serialisationOverRemoting(SecretBytesTest.java:176)
Caused by: java.lang.Error: javax.crypto.BadPaddingException: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
at app//com.cloudbees.plugins.credentials.SecretBytes.getPlainData(SecretBytes.java:141)
at app//com.cloudbees.plugins.credentials.SecretBytesTest$CheckSecretBytesCallable.call(SecretBytesTest.java:191)
at app//com.cloudbees.plugins.credentials.SecretBytesTest$CheckSecretBytesCallable.call(SecretBytesTest.java:179)
at hudson.remoting.UserRequest.perform(UserRequest.java:225)
at hudson.remoting.UserRequest.perform(UserRequest.java:50)
at hudson.remoting.Request$2.run(Request.java:391)
at hudson.remoting.InterceptingExecutorService.lambda$wrap$0(InterceptingExecutorService.java:81)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
Suppressed: hudson.remoting.Channel$CallSiteStackTrace: Remote call to slave0
at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1916)
at hudson.remoting.UserRequest$ExceptionResponse.retrieve(UserRequest.java:384)
at hudson.remoting.Channel.call(Channel.java:1108)
at com.cloudbees.plugins.credentials.SecretBytesTest.serialisationOverRemoting(SecretBytesTest.java:176)
Caused by: javax.crypto.BadPaddingException: Given final block not properly padded. Such issues can arise if a bad key is used during decryption.
at java.base/com.sun.crypto.provider.CipherCore.unpad(CipherCore.java:861)
at java.base/com.sun.crypto.provider.CipherCore.fillOutputBuffer(CipherCore.java:941)
at java.base/com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:734)
at java.base/com.sun.crypto.provider.AESCipher.engineDoFinal(AESCipher.java:446)
at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2244)
at app//com.cloudbees.plugins.credentials.SecretBytes.getPlainData(SecretBytes.java:139)
... 10 more
6 tasks
Contributor
|
For posterity and cross-linking: this PR and some earlier landed commits may borrow from work started and stalled in #391 and altogether this should finally solve issue https://issues.jenkins.io/browse/JENKINS-70101 and https://issues.jenkins.io/browse/JENKINS-67778 (#913) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Any attempt to serialize
SecretBytesover a remoting channel was doomed to failure as the remote side would have a differentCredentialsConfidentialKeyleading to either a failure to decrypt or garbage data.This change fixes this by transferring the data in plan text and re-encrypting on the receiving side (if and only if serialisation is being performed over a remoting channel)
Testing done
Introduced new unit test, that fails before the fix and passes after the fix, existing unit tests still pass
fixes: #1013
Submitter checklist