-
Notifications
You must be signed in to change notification settings - Fork 508
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
🌱 Secure workflow stale.yml #1326
🌱 Secure workflow stale.yml #1326
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NACK- I am not in favor of sending data to a remote server agent.api.stepsecurity.io:443
. By putting this in we would be indirectly recommending this to the larger community.
I like the adding additional permissions.
.github/workflows/stale.yml
Outdated
runs-on: ubuntu-latest | ||
steps: | ||
- uses: step-security/harden-runner@7206db2ec98c5538323a6d70e51f965d55c11c87 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What value add does this step-security/harden-runner
? So that it restricts access the action to only certain FQDN?
What if there is a vulnerability within the step-security/harden-runner
? What kind of data is being sent to agent.api.stepsecurity.io:443
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @naveensrinivasan the value add as of now is to prevent exfiltration of credentials and detect compromised components in the supply chain. As an example, when the Codecov bash uploader was compromised, credentials were sent out to 2 IP addresses and were not detected for months. Or as in the dependency confusion attack, DNS exfiltration was used to send metadata out. As more features are added, the value add would be to provide runtime security for GitHub Action hosted runner...
step-security/harden-runner
and the step-security/agent
it installs are both open source. The data it sends out is network information and process information. You can see the source code and the output here. W.r.t vulnerability, yes, that is a risk, but that is true for all GitHub Actions and all components used in the supply chain. To reduce the risk, the goal of step-security/harden-runner
is to set a baseline and restrict network, process, and file activity on the runner.
Please let me know if this addresses your concerns. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @naveensrinivasan the value add as of now is to prevent exfiltration of credentials and detect compromised components in the supply chain. As an example, when the Codecov bash uploader was compromised, credentials were sent out to 2 IP addresses and were not detected for months. Or as in the dependency confusion attack, DNS exfiltration was used to send metadata out. As more features are added, the value add would be to provide runtime security for GitHub Action hosted runner...
Could you tell us more about how the restrictions put in place by step-security/harden-runner
are guaranteed not to be undone by code that manages to get RCE in the action? Say, in the firewalling rules: can the malicious code undo the rules? Why not?
The firewalling code does allow 8.8.8.8
: why can the malicious code not send packets to it, and only the agent can?
step-security/harden-runner and the step-security/agent it installs are both open source. The data it sends out is network information and process information. You can see the source code and the output here. W.r.t vulnerability, yes, that is a risk, but that is true for all GitHub Actions and all components used in the supply chain. To reduce the risk, the goal of step-security/harden-runner is to set a baseline and restrict network, process, and file activity on the runner.
Please let me know if this addresses your concerns. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @laurentsimon w.r.t restrictions being undone, to undo the firewall rules or to stop the agent itself, a malicious action would need to run sudo
. One of the future features would be to recommend if sudo can be disabled for that job (because it is not used) and do so. If it cannot be disabled, then detect the use of sudo
in a way not in the baseline.
W.r.t sending to 8.8.8.8, that is a great question. Malicious code can do that, and will not be stopped, but it will get detected. The service side creates a map of process activities per step, and if a step was not supposed to call 8.8.8.8, and does, it will get detected. In fact, I have been thinking about whether to have a detect and alert only system or also do restrictions for outbound calls. Restrictions are not going to be per step though, they can only be for the whole job...
Hope that answers your questions.
I'm on the fence about this and leaning towards not sending data to a remote server. Will let @laurentsimon chime in. |
Hi @azeemshaikh38 @naveensrinivasan would love to better understand the concern with sending data to remote server, given that the code is open source. Once I understand the concern, I may be able to solve it in a different way. Lot of steps/ actions in a GitHub workflow make outbound calls, to download code, download dependencies, upload coverage reports, deployments, publishing packages etc, so not able to understand concern with sending metadata to remote server. With this at least you will know which all steps are making calls to what all remote servers. Otherwise, that is completely unknown to the developers today. |
what other policies do you have in mind for the hardener? The firewall is mostly useful for workflows where the |
My concern is not technical but mostly inertia about a new service. Also adding @asraa here in case she's interested in giving this a look. |
I agree,Also making sure going forward the step-security is secure.The repository is maintained by a single individual and doesn't have the traction from community. So my vote is NO. |
Hi @laurentsimon my goal is to prioritize policies based on previous supply chain attacks. The outbound traffic restriction/ detection can detect reconnaissance (e.g. the DNS exfiltration in the dependency confusion attack) and also prevent credential exfiltration beyond the |
Thanks @naveensrinivasan that is fair feedback. The repo and components are fairly new and my intention was to pilot it. In fact I already got some really good feedback in this discussion. I would be happy to remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Integration tests success for |
FYI, in terms of token exfiltration, the GitHub APIs provide many covert channels. An attacker can theoretically exfiltrate sensitive data by, for example, create an issue on an attacker-controlled GitHub issue. |
Hi @laurentsimon - this would have then raised the bar from the current, where attacker just has to do simple http call to any IP address. Moreover, with a per-step policy in place, that step would normally need to call api.github.com. If not, this will be detected. In the future, there should also be support for application level policy e.g. only allow https://api.github.com/repos/allowed-owner/allowed-repo/*. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
PR secures the stale.yml workflow by adding minimum token permissions. It also adds the
step-security/harden-runner
GitHub action that provides runtime security for GitHub hosted runners. By adding this, outbound calls will be limited to allowed domains, to prevent credential exfiltration. You can see the results from running this on a fork here.What is the current behavior? (You can also link to an open issue here)
Currently stale.yml does not have token permissions set and outbound calls are not restricted.
What is the new behavior (if this is a feature change)?
Minimum tokens are set and outbound calls are restricted
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No
Other information:
@laurentsimon would love to pilot the step-security/harden-runner on this workflow to get feedback and improve the experience. Do let me know if you have any questions. Thanks!