credentials-plugin
credentials-plugin copied to clipboard
[JENKINS-70101] Revive ability to snapshot() the CertificateCredentials so they can be used on remote agents
As detailed in https://issues.jenkins.io/browse/JENKINS-70101 I've hit a problem using certificate-based user authentication in http-request-plugin (noticed as part of my work on https://issues.jenkins.io/browse/JENKINS-70000 in https://github.com/jenkinsci/http-request-plugin/pull/120 but also reproduced with latest 1.16 release of that plugin).
After a week of reproductions and tracing across different plugins, the root of the problem crystallized as the use of SecretBytes
in UploadedKeyStoreSource
and effective lack of special snapshot()
support. As a consequence, serialized copies of the key store used the same SecretBytes
as on the Jenkins Controller, but being on a different JVM in the remote agent case, they used a different instance of the static KEY
field with its own secret
field. Although the SecretBytes
are diligently copied (took a lot of effort to confirm that as seen in sibling branch https://github.com/jimklimov/credentials-plugin/tree/JENKINS-70101-trace), they are unreadable on the remote agent.
During investigation I found that credentials-plugin did have an implementation for the Certificate Credentials Snapshot Taker, but it was removed by https://github.com/jenkinsci/credentials-plugin/commit/40d0b5cc53c265b601ffaa4469310fad390a80fb as part of deprecation of FileOnMasterKeyStoreSource
subclass. Also an earlier history of the plugin involved the Secret
class which effectively stores a base64 string and only encodes/decodes it on demand -- this was superseded by SecretBytes
but some handling for conversion from older versions remained. This code proved to be a good starting point for fixing the problem, although not without some further work:
- originally it consulted
isSnapshotSource
(forcedtrue
forUploadedKeyStoreSource
subclass) and just returned the original credential if so; - even with that check disabled, the snapshot used
SecretBytes
for the copied key store and suffered the same fate - not usable on agent.
This PR adds tests (failing at first to confirm the problem), makes use of older codebases and new verified fixes:
-
UploadedKeyStoreSource.isSnapshotSource()
depends onChannel.current() == null
(so for remoting-related snapshots it isfalse
and shortcuts toreturn this
are not taken) -
CertificateCredentialsSnapshotTaker
was revived as a separate source file and standalone class, similar to existingUsernamePasswordCredentialsSnapshotTaker
-
UploadedKeyStoreSource
was modified to handle again aSecret uploadedKeyStore
field (and provide data from it ifSecretBytes uploadedKeyStoreBytes
are currentlynull
). As part of that,transient
andfinal
modifiers on these fields had to go. -
CertificateCredentialsSnapshotTaker
was modified to create the newUploadedKeyStoreSource
instance withSecret
version rather thanSecretBytes
(asCredentialsSnapshotTaker
docs stipulate, "all the details are captured within the credential")
Thanks to @mawinter69 and @slide for bright ideas and pointers, and general sympathy as I woed on the chat :)
Separately note that this PR adds tests using a separate JVM for the build agent to reproduce the problem. This code may be worth exporting into some JenkinsAgentRule
if someone is up for it.
- [x] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue
NOTE: I'll post commits in several phases, so CI has its chance to fail with the tests and show the original problem in logs.