Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-73471] Restore passing credentialsId to the GitSCM #867

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Dohbedoh
Copy link
Contributor

@Dohbedoh Dohbedoh commented Aug 1, 2024

Proposal to fix #862. Reinstore the credentialsId in the GitSCM configuration.
It would also guarantee that credentials usage is still tracked. Checking down the line, GitClient still uses the authenticator credentials reference.

@yaroslavafenkin Per my understanding, the issue that SECURITY-3363 fixes was the clone link of the OAuth Authenticator at https://github.com/jenkinsci/bitbucket-branch-source-plugin/blob/886.v44cf5e4ecec5/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/api/credentials/BitbucketOAuthAuthenticator.java#L48-L57 ? In which case instantiating the GitSCM with the credentialsId is fine ? I am not sure what is the scenario to validate that this does not bring back this security problem ?

Your checklist for this pull request

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

@dwnusbaum
Copy link
Member

I don't know if Yaroslav is still active in the Jenkins project. It is unfortunate that ad359b3 did not add any tests. Here are the reproduction steps from the security ticket though to check if it regresses the fix manually:

  • Create a Bitbucket repository with a Jenkinsfile and a PR editing this Jenkinsfile (You need at least 2 branches)
  • Create an OAuth credentials following this documentation
  • Create a MultiBranch Pipeline
  • As a Branch Sources, select "Bitbucket"
  • Select your previously created OAuth credentials
  • As Owner, put the username of your Bitbucket repository
    (With a valid OAuth credentials and owner, it should automatically fill the repository name dropdown)
  • Save
  • Navigate to the status page MultiBranch pipeline ([JENKINS_INSTANCE]/job/[JOB_NAME])
  • Click on "Pull request"
  • Navigate to the related job
  • Open the build console output [and check whether the access token is displayed in the clone URLs]

@dwnusbaum
Copy link
Member

dwnusbaum commented Aug 1, 2024

@Dohbedoh Yaroslav asked me to pass on some info from when he worked on the security ticket initially. The reason he made the one-line change that you are proposing to revert in this PR has to do with unusual behavior he saw while testing:

There's a bit of weird behaviour that I've just noticed when testing. On a fresh instance I've setup a project, pointed it to a repo that I've set up for my testing purposes in BitBucket Cloud. I've added Username and Password credentials that correspond to OAuth key and secret. Scanning is successful, build on the main branch too. PR build kept failing with the following:

Log
Started by user [Admin](http://localhost:8080/user/admin)
Checking out git https://[email protected]/yafenkin-org/test.git into /Users/yaroslavafenkin/projects/jenkinsci/jenkins/war/work/workspace/zxc_PR-4@script/7f84c2c049bbfafce90d9d2d0fe3b04ae23118ea5c2526236601377762ad2088 to read Jenkinsfile
The recommended git tool is: NONE
using credential whatever
 > git rev-parse --resolve-git-dir /Users/yaroslavafenkin/projects/jenkinsci/jenkins/war/work/workspace/zxc_PR-4@script/7f84c2c049bbfafce90d9d2d0fe3b04ae23118ea5c2526236601377762ad2088/.git # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://[email protected]/yafenkin-org/test.git # timeout=10
Fetching without tags
Fetching upstream changes from https://[email protected]/yafenkin-org/test.git
 > git --version # timeout=10
 > git --version # 'git version 2.39.3 (Apple Git-146)'
using GIT_ASKPASS to set credentials whatever
 > git fetch --no-tags --force --progress -- https://[email protected]/yafenkin-org/test.git +refs/heads/Yaroslav-Afenkin/jenkinsfile-edited-online-with-bitbucket-1709733248367:refs/remotes/origin/Yaroslav-Afenkin/jenkinsfile-edited-online-with-bitbucket-1709733248367 +refs/heads/main:refs/remotes/origin/main # timeout=10
ERROR: Error fetching remote repo 'origin'
hudson.plugins.git.GitException: Failed to fetch from https://[email protected]/yafenkin-org/test.git
	at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:999)
	at hudson.plugins.git.GitSCM.retrieveChanges(GitSCM.java:1241)
	at hudson.plugins.git.GitSCM.checkout(GitSCM.java:1305)
	at org.jenkinsci.plugins.workflow.steps.scm.SCMStep.checkout(SCMStep.java:136)
	at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:167)
	at org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:143)
	at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:311)
	at hudson.model.ResourceController.execute(ResourceController.java:101)
	at hudson.model.Executor.run(Executor.java:442)
Caused by: hudson.plugins.git.GitException: Command "git fetch --no-tags --force --progress -- https://[email protected]/yafenkin-org/test.git +refs/heads/Yaroslav-Afenkin/jenkinsfile-edited-online-with-bitbucket-1709733248367:refs/remotes/origin/Yaroslav-Afenkin/jenkinsfile-edited-online-with-bitbucket-1709733248367 +refs/heads/main:refs/remotes/origin/main" returned status code 128:
stdout: 
stderr: remote: Invalid credentials
fatal: Authentication failed for 'https://bitbucket.org/yafenkin-org/test.git/'

	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2872)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:2211)
	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:637)
	at hudson.plugins.git.GitSCM.fetchFrom(GitSCM.java:997)
	... 8 more
ERROR: Error fetching remote repo 'origin'
ERROR: Maximum checkout retry attempts reached, aborting
ERROR: Error fetching remote repo 'origin'
ERROR: Maximum checkout retry attempts reached, aborting
[Bitbucket] Notifying pull request build result
Can not determine Jenkins root URL or Jenkins URL is not a valid URL regarding Bitbucket API. Commit status notifications are disabled until a root URL is configured in Jenkins global configuration. 
IllegalStateException: Jenkins URL cannot start with http://localhost/
Finished: FAILURE

I switched to a terminal and did git clone https://[email protected]/yafenkin-org/test.git. Was asked for a password. Captured the token via the debugger, pasted it in and the repo was cloned successfully. Went back to Jenkins and restarted the build for the PR. It now succeeds (issuing a new token for every build, so it's a not a particular token issue). Doesn't seem to happen without the fix, so likely caused by it :/

After that comment he changed the line in question to stop passing credentialsId, which seemed to fix the issue for him when testing.

He doesn't think that your change would regress the security fix, the question is just whether the error that he saw when the code was like you have it in the PR now is representative of a real bug that would affect users or is just some kind of environmental or configuration issue.

@Dohbedoh
Copy link
Contributor Author

Dohbedoh commented Aug 2, 2024

Thanks!

  1. I can't reproduce the security problem.
  2. Reproducible. Actually that comment made me think about credentials.helper, and when getting rid of it, actually even a branch build checkout scm fails. Looking into this.

@Dohbedoh
Copy link
Contributor Author

Dohbedoh commented Aug 2, 2024

Allright, so actually this cannot work .. In GitSCM, the GitSCMExtension decorates the GitClient first and then it set the credentials if a credentials ID is provided https://github.com/jenkinsci/git-plugin/blob/git-5.2.2/src/main/java/hudson/plugins/git/GitSCM.java#L929. It's been the case since 2013...

I wonder why we don't decorate the client at the end of the GitClient#createClient method 🤔. We can maybe propose this.. I guess a way to solve this particular problem is to override the GitSCMExtension#beforeCheckout to override the credentials added to the GitClient... I tested this and it works.

If unit tests are required, I would need more time to provide them..

@Dohbedoh
Copy link
Contributor Author

Dohbedoh commented Aug 28, 2024

@Dohbedoh
Copy link
Contributor Author

git-plugin 5.5.0 has been released.
Added a temporary dependency in the pom until that version of git is included in the BOM.
Updated the BOM.

cc @jenkinsci/bitbucket-branch-source-plugin-developers

@Dohbedoh Dohbedoh force-pushed the JENKINS-73471 branch 2 times, most recently from 2f82472 to 6a23a48 Compare September 23, 2024 09:32
@jglick
Copy link
Member

jglick commented Sep 25, 2024

Alternately, revert ad359b3 and just use a https://javadoc.jenkins.io/plugin/workflow-api/org/jenkinsci/plugins/workflow/log/TaskListenerDecorator.Factory.html which is triggered by the use of this SCM source and which masks any x-token-auth:{…}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version 887 breaks usage of scm.userRemoteConfigs[].credentialsId
3 participants