Closed kalpshekhar closed 1 year ago
Security audit, information and commands
The security team is auditing all the hosting requests, to ensure a better security by default. This message informs you that the security team was notified about the request and will soon participate in this issue to assist. The team is usually starting by a quick superficial audit and if it's not sufficient, they are planning a deeper audit.
/audit-ok
=> the audit is complete, the hosting can continue :tada:./audit-skip
=> the audit is not necessary, the hosting can continue :tada:./audit-required
=> the superficial audit was not sufficient, a deeper look is necessary :mag:./audit-findings
=> the audit reveals some issues that require corrections :pencil2:./audit-review
=> the findings from the audits were corrected, this command will ping the security team to review the findings :eyes:.
It's only applicable when the previous audit required changes.(automatically generated message)
Hello from your friendly Jenkins Hosting Checker
It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.
You can re-trigger a check by editing your hosting request or by commenting /hosting re-check
Hey @kalpshekhar,
a few things I'd like to point out:
@v5
hudson.plugins
to io.jenkins.plugins.deployintegrationserver
https://github.com/kalpshekhar/deploy-integrationserver/tree/master/src/main/java/hudson/plugins<changelist>-SNAPSHOT</changelist>
inside your <properties>
block.Hi @NotMyFault, Thanks for your inputs. I have made all the changes.
/hosting re-check
Hello from your friendly Jenkins Hosting Checker
It looks like you have everything in order for your hosting request. A human volunteer will check over things that I am not able to check for (code review, README content, etc) and process the request as quickly as possible. Thank you for your patience.
Hosting team members can host this request with /hosting host
Hello @kalpshekhar,
we're able to perform a security audit and it seems you've misunderstood how distributed builds work. Indeed with your actual code, you'll not be able to retrieve files on agents, your search is only done on the controller. This prevents it from working properly, especially in the case of different OS between controller and agents. And worse, by creating an environment variable on the agent, it would be possible to upload arbitrary zip files from the controller to an arbitrary site.
Recommendation: Use the perform version which uses the FilePath argument, it will automatically define the workspace, depending on the agent where the build is executed.
I will let you correct it and ping us when you are done or if you have questions.
/audit-findings
Hi @Kevin-CB Thanks for you inputs. I have made the changes. I have changed the core implementation of the plugin. It no longer requires zip files to be uploaded but still uses workspace. It is now using FilePath argument for that.
/audit-review
A bigger analysis is undergoing, tracked internally as JENSEC-1916
Hello @kalpshekhar ,
We were able to perform a more complete audit on your plugin, here are the feedback:
At ProjectAutomatorUtils.java#L157 and DeployerUtils.java#L38, you're running command using the classic Runtime.getRuntime().exec
which is vulnerable to command injection and does not follow the environment where it'll be executed. (It will always be executed on the controller)
Recommendation:
Replace it by the Launcher provided by your perform method.
(Not a security issue) Same thing with ProjectAutomatorUtils.java#L138, to avoid it to be executed on the controller with an agent filepath, you should use method from the FilePath class.
At ProjectAutomatorUtils.java#L50-L51, you're storing credentials in plain text, I don't know what is our use case with it, but if possible, stores an encrypted version of this password, or at least use a temporary file to store this value.
I will let you correct the different findings and ping us when you are done or if you have questions.
/audit-findings
Please ignore. Found there is ps.quite(true) method that solves the below issue.
Hi @Kevin-CB When I use launcher to execute the .bat command, ProcStarter ps = launcher.launch(); Proc p = launcher.launch(ps.cmdAsSingleString(command).stdout(listener));
It prints the command itself in the build log file which contains the pwd argument. C:\SoftwareAG\IntegrationServer\instances\default\packages\WmDeployer\bin\Deployer.bat --deploy -dc mytestprojectDC -project mytestproject -host localhost -port 5555 -user Administrator -pwd mypassword -force -reportFilePath c:\eclipse-workspace-2\deploy-integrationserver\work\workspace\test
Is there a way I can set to not print the executed command? I do want to print other output generated by the .bat file.
Thanks!
Hi @Kevin-CB I have made the changes as per your recommendations.
Hello @kalpshekhar,
You're still vulnerable to a command injection, indeed you should not let the user provide the first variable at ProjectAutomatorUtils.java#L152 and DeployerUtils.java#L41, deployerHomeDirectory
can contain the path to an others executable.
What you can do here is to find and confirm the path to projectautomator
or Deployer
before calling the command, or to force a path.
projectautomator executable encrypts the password in the file automatically when its executed by the plugin.
In this case, you should delete the file in case projectautomator
executable fail.
Hello @Kevin-CB I have made the changes.
Hello @kalpshekhar, sorry for the delay.
Everything looks good to me, you can continue the hosting process 🙂
/audit-ok
Hey @kalpshekhar,
thanks for addressing the feedback. Apparently some of my last feedback got lost:
Hi @NotMyFault I have removed the unused dependencies.
/hosting re-check
Hello from your friendly Jenkins Hosting Checker
It appears you have some issues with your hosting request. Please see the list below and correct all issues marked Required. Your hosting request will not be approved until these issues are corrected. Issues marked with Warning or Info are just recommendations and will not stall the hosting process.
<jenkins.version>2.332.4</jenkins.version>
to at least 2.361.4 in your pom.xml. Take a look at the baseline recommendations.You can re-trigger a check by editing your hosting request or by commenting /hosting re-check
/hosting re-check
Hello from your friendly Jenkins Hosting Checker
It looks like you have everything in order for your hosting request. A human volunteer will check over things that I am not able to check for (code review, README content, etc) and process the request as quickly as possible. Thank you for your patience.
Hosting team members can host this request with /hosting host
/hosting host
Hosting request complete, the code has been forked into the jenkinsci project on GitHub as https://github.com/jenkinsci/deploy-integrationserver-plugin
A Jira component named deploy-integrationserver-plugin has also been created with kalpshekhar as the default assignee for issues.
A pull request has been created against the repository permissions updater to setup release permissions. Additional users can be added by modifying the created file.
Please delete your original repository (if there are no other forks), under 'Danger Zone', so that the jenkinsci organization repository is the definitive source for the code. If there are other forks, please contact GitHub support to make the jenkinsci repo the root of the fork network (mention that Jenkins approval was given in support request 569994). Also, please make sure you properly follow the documentation on documenting your plugin so that your plugin is correctly documented.
You will also need to do the following in order to push changes and release your plugin:
In order for your plugin to be built by the Jenkins CI Infrastructure and check pull requests, please add a Jenkinsfile to the root of your repository with the following content:
buildPlugin(useContainerAgent: true, jdkVersions: [8, 11])
Welcome aboard!
Repository URL
https://github.com/kalpshekhar/deploy-integrationserver
New Repository Name
deploy-integrationserver-plugin
Description
This plugin is to deploy webMethods Integration Server packages. There is no plugin at the moment that does this.
GitHub users to have commit permission
@kalpshekhar
Jenkins project users to have release permission
kalpshekhar
Issue tracker
Jira