Skip to content

build(deps): bump openssl from 0.10.71 to 0.10.72 in the cargo group across 1 directory #1924

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

Merged
merged 1 commit into from
Apr 5, 2025

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Apr 4, 2025

Bumps the cargo group with 1 update in the / directory: openssl.

Updates openssl from 0.10.71 to 0.10.72

Release notes

Sourced from openssl's releases.

openssl-v0.10.72

What's Changed

New Contributors

Full Changelog: sfackler/rust-openssl@openssl-v0.10.71...openssl-v0.10.72

Commits
  • 87085bd Merge pull request #2390 from alex/uaf-fix
  • d1a12e2 Fixed two UAFs and bumped versions for release
  • 7c7b2e6 Merge pull request #2389 from skmcgrail/aws-lc-follow-up
  • 34a477b Use --experimental with bindgen-cli with aws-lc build
  • d4bf071 Merge pull request #2386 from skmcgrail/aws-lc-follow-up
  • a86bf67 Remove comment
  • 705dbfb Fix test
  • e0df413 Skip final call for LibreSSL 4.1.0 for CCM mode
  • 2f1164b Enable additional capabilities for AWS-LC
  • dde9ffb Merge pull request #1805 from skmcgrail/aws-lc-support-final
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore <dependency name> major version will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself)
  • @dependabot ignore <dependency name> minor version will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself)
  • @dependabot ignore <dependency name> will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself)
  • @dependabot unignore <dependency name> will remove all of the ignore conditions of the specified dependency
  • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions
    You can disable automated security fix PRs for this repo from the Security Alerts page.

@dependabot dependabot bot added dependencies Pull requests that update a dependency file rust Pull requests that update Rust code labels Apr 4, 2025
Copy link
Member

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This updates openssl in Cargo.lock to a version unaffected by RUSTSEC-2025-0022 (GHSA-4fcv-w3qc-ppgg). The cargo-deny-advisories job fails here due to an unrelated advisory. That job deliberately does not block auto-merge, and it fails when rerun on main in the same way it fails here. So I plan to merge this PR.

However, it is strange that the job fails the same way here as when rerun on main, because I would expect the vulnerability this is fixing to be revealed by rerunning cargo-deny-advisories on main (such that its absence here would constitute a difference). That did not happen: this vulnerable dependency was only ever detected by Dependabot, even though it has a RUSTSEC advisory.

That's a gap in the functionality of our cargo-deny-advisories job. I'm going to see if I can fix that quickly. If so, then waiting to integrate this PR until after it is fixed will help verify that I have managed to fix it. So I'm going to hold off on merging this for a short time (up to a few hours, probably less) in order to attempt that.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this pull request Apr 5, 2025
When `cargo deny` commands are run from the top-level directory of
a workspace, and no `--manifest-path` option is pased, they
implicitly use the top-level `Cargo.toml` file as their manifest.

In workspaces where this is a virtual manifest, i.e. those where
the top-level `Cargo.toml` defines only the workspace and not also
a specific crate, running `cargo deny` without specifying any roots
implicitly acts as though `--workspace` has been passed. (See
https://embarkstudios.github.io/cargo-deny/cli/common.html for
details.) In that situation, `--workspace` can be omitted and all
crates in the workspace are still treated as roots for the scan.

But the gitoxide repository's top-level workspace's `Cargo.toml`
file is not a virtual manifest. It defines a workspace, but it also
defines the `gitoxide` crate. As a result, `cargo deny` commands,
as run on CI in the `cargo-deny` and `cargo-deny-advisories` jobs,
and locally via `just audit`, were not guaranteed to check the
entire workspace. They used only the `gitoxide` crate as a root for
scanning, rather than using all crates defined in the workspace as
roots as when `--workspace` is used or implied.

It looks like this was not detected for three reasons:

1. Most of the workspace was usually covered, especially on CI
   where, since no `arguments` were passed, `cargo-deny-action`
   used the defualt of `--all-features`. *Most* transitive
   dependencies inside and outside of the gitoxide project seem to
   be used by the `gitoxide` crate by in some feature
   configuration.

2. The distinction between virtual and non-virtual top-level
   manifests of workspaces is subtle and not highlighted in the
   `cargo deny` documentation.

3. In `EmbarkStudios/cargo-deny-action`, the default value of
   `arguments` contains `--all-features` but not `--workspace` (nor
   any other options). Intuitively it feels like the default value
   would scan all crates in the repository that the action is being
   run on. That does happen in the perhaps more common case that
   the top-level `Cargo.toml` defines no crate, but not here.

This was discovered in connection with GitoxideLabs#1924. It is why the
`cargo-deny-advisories` job didn't catch RUSTSEC-2025-0022
(GHSA-4fcv-w3qc-ppgg) even as Dependabot did.

To fix it, this adds `--workspace` explicitly in both `cargo deny`
jobs run on CI, as well as in the `justfile`.

This also adds `--all-features`:

- On CI, adding `--all-features` is itself no change from before,
  since it is the implicit value of `arguments`. It has to be
  passed explicitly now that `arguments` has an explicit value.

- In the `justfile`, adding `--all-features` does (at least in
  principle) make a difference.
@Byron
Copy link
Member

Byron commented Apr 5, 2025

And on top of that, we are probably looking at quite some time until the failure due to gix-testtools can be fixed, which is unable to use the latest version or publishes break.

I think what I will have to do is to dance around it - set it to the latest version, create a new release, then set it back to allow a follow-up release of gitoxide.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this pull request Apr 5, 2025
When `cargo deny` commands are run from the top-level directory of
a workspace, and no `--manifest-path` option is pased, they
implicitly use the top-level `Cargo.toml` file as their manifest.

In workspaces where this is a virtual manifest, i.e. those where
the top-level `Cargo.toml` defines only the workspace and not also
a specific crate, running `cargo deny` without specifying any roots
implicitly acts as though `--workspace` has been passed. (See
https://embarkstudios.github.io/cargo-deny/cli/common.html for
details.) In that situation, `--workspace` can be omitted and all
crates in the workspace are still treated as roots for the scan.

But the gitoxide repository's top-level workspace's `Cargo.toml`
file is not a virtual manifest. It defines a workspace, but it also
defines the `gitoxide` crate. As a result, `cargo deny` commands,
as run on CI in the `cargo-deny` and `cargo-deny-advisories` jobs,
and locally via `just audit`, were not guaranteed to check the
entire workspace. They used only the `gitoxide` crate as a root for
scanning, rather than using all crates defined in the workspace as
roots as when `--workspace` is used or implied.

It looks like this was not detected for three reasons:

1. Most of the workspace was usually covered, especially on CI
   where, since no `arguments` were passed, `cargo-deny-action`
   used the defualt of `--all-features`. *Most* transitive
   dependencies inside and outside of the gitoxide project seem to
   be used by the `gitoxide` crate by in some feature
   configuration.

2. The distinction between virtual and non-virtual top-level
   manifests of workspaces is subtle and not highlighted in the
   `cargo deny` documentation.

3. In `EmbarkStudios/cargo-deny-action`, the default value of
   `arguments` contains `--all-features` but not `--workspace` (nor
   any other options). Intuitively it feels like the default value
   would scan all crates in the repository that the action is being
   run on. That does happen in the perhaps more common case that
   the top-level `Cargo.toml` defines no crate, but not here.

This was discovered in connection with GitoxideLabs#1924. It is why the
`cargo-deny-advisories` job didn't catch RUSTSEC-2025-0022
(GHSA-4fcv-w3qc-ppgg) even as Dependabot did.

To fix it, this adds `--workspace` explicitly in both `cargo deny`
jobs run on CI, as well as in the `justfile`.

This also adds `--all-features`:

- On CI, adding `--all-features` is itself no change from before,
  since it is the implicit value of `arguments`. It has to be
  passed explicitly now that `arguments` has an explicit value.

- In the `justfile`, adding `--all-features` does (at least in
  principle) make a difference.
@EliahKagan
Copy link
Member

Once the #1925 auto-merge completes, I plan to rebase this onto main, inspect the cargo-deny-advisories output here just in case, and merge it.

@EliahKagan
Copy link
Member

@dependabot rebase

@dependabot dependabot bot force-pushed the dependabot/cargo/cargo-5b4ceb88c4 branch from cd5c940 to 773fb04 Compare April 5, 2025 01:53
Copy link
Member

@EliahKagan EliahKagan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since #1925 was merged, the vulnerability this Dependabot security update is for is found by cargo-deny-advisories on main, as intended. This is rebased onto main, and it goes away here, as intended, since unaffected versions of openssl and openssl-sys are listed instead. This is all as intended, so this Dependabot PR is in good shape. The job still of course still fails because of the other unrelated advisory in the old version of gix-features pulled in via gix-testtools as you note in #1924 (comment).

What might be a temporary blocker is that the full test job is now breaking in a completely unrelated way. I observed this on my fork, including on its main branch, already. For a short window of time it seems to not be happening in this upstream repository, but now is. I have a suspicion as to the cause and fix; if I can't fix it soon then I'll let you know.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this pull request Apr 5, 2025
The CI `test` job's "Setup dependencies" step had `apt-get install`
without having previously run `apt-get update`. This often does not
work, and in a CI environment one should not typically expect it to
work because package indexes in a virtual machine just provisioned
from an image, if present at all, may be very outdated. But for
some reason it had been working until recently, when breakages were
observed, including in GitoxideLabs#1924 (though the breakage is not related to
the changes in that PR).

This runs `apt-get update` before `apt-get install` in that CI job,
as had already been done in the other CI jobs and as had likely
always been intended. This should make the "Setup dependencies"
step work again.
@EliahKagan
Copy link
Member

I've fixed the new apt-get problem in #1926, which is set to auto-merge. Ordinarily I would have just added a commit here for it, but I did it separately for diagnostic reasons as described there. When that's merged, I plan to rebase this Dependabot PR again and finally merge it.

@EliahKagan
Copy link
Member

@dependabot rebase

Bumps the cargo group with 1 update in the / directory: [openssl](https://github.com/sfackler/rust-openssl).


Updates `openssl` from 0.10.71 to 0.10.72
- [Release notes](https://github.com/sfackler/rust-openssl/releases)
- [Commits](sfackler/rust-openssl@openssl-v0.10.71...openssl-v0.10.72)

---
updated-dependencies:
- dependency-name: openssl
  dependency-version: 0.10.72
  dependency-type: indirect
  dependency-group: cargo
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/cargo/cargo-5b4ceb88c4 branch from 773fb04 to ee6ee99 Compare April 5, 2025 02:30
@EliahKagan EliahKagan enabled auto-merge April 5, 2025 02:30
@EliahKagan EliahKagan merged commit da1545f into main Apr 5, 2025
19 of 20 checks passed
@EliahKagan EliahKagan deleted the dependabot/cargo/cargo-5b4ceb88c4 branch April 5, 2025 02:47
@EliahKagan
Copy link
Member

EliahKagan commented Apr 5, 2025

Everything went through here. 🚀


And on top of that, we are probably looking at quite some time until the failure due to gix-testtools can be fixed, which is unable to use the latest version or publishes break.

Do you mean the underlying problem of gix-testtools depending on vulnerable crates, or the specific effect that the cargo-deny-advisories job fails because of it and is potentially distracting?

If wanted, I think I could put in a place some workaround for the latter, such as running cargo deny twice on CI for advisories, once with the committed deny.toml configuration but gix-testtools pruned, and once with a modified configuration with RUSTSEC-2025-0021 added to the ignore list, thereby letting everything pass while keeping our use of cargo deny fully robust. But that's separate from actually fixing it.

I think what I will have to do is to dance around it - set it to the latest version, create a new release, then set it back to allow a follow-up release of gitoxide.

I'm not sure I fully understand. Is this something that would be done in future releases of published crates other than gix-testtools, to allow the circular dev dependency described in #1510 (comment) while achieving the goal of #1510 and also fully working around #1886 (perhaps making the vendoring discused in #1886 (comment) unnecessary)?

@Byron
Copy link
Member

Byron commented Apr 5, 2025

If wanted, I think I could put in a place some workaround for the latter, such as running cargo deny twice on CI for advisories, once with the committed deny.toml configuration but gix-testtools pruned, and once with a modified configuration with RUSTSEC-2025-0021 added to the ignore list, thereby letting everything pass while keeping our use of cargo deny fully robust. But that's separate from actually fixing it.

I mean, this would be even better, as that way I wouldn't have to mess with publishing which is fragile enough. For instance, I noticed that a workspace dependency in conditional dependencies aren't updated automatically…

[target.'cfg(windows)'.dependencies]
gix-path = { version = "^0.10.15", path = "../gix-path" }

…and the publish will fail if the version isn't matching up to the patch level.

I think this answers the second question - my idea would just be a workaround, and not a permanent fix to this. Right now I simply can't justify working on cargo smart-release, a situation I don't think will change anytime soon.

@EliahKagan
Copy link
Member

I mean, this would be even better, as that way I wouldn't have to mess with publishing which is fragile enough.

To clarify--I think we may be on the same page, but I am not sure--what I am talking about doing will only affect the behavior of the cargo-deny-advisory CI job as run here in this repository. It won't cause anything to be updated automatically, nor cause any change of any kind for people to use gix-testtools in their own project.

That might be okay. I would guess that in most other uses of gix-testtools the risks from RUSTSEC-2025-0021 would be fairly low since gix-testtools would typically be operating on trusted test fixture data, assuming other projects use it in a similar way to how it is used here. But I wanted to check, because what I am talking about doing solves a different and much narrower problem than what I imagine would be solved by making a new release of anything.

If the problem that needs to be solved is just that the cargo-deny-advisories CI job in this repository is failing due to a condition that we don't wish to cause it to fail, then modifying the job is definitely a better solution than releasing any crates.

For instance, I noticed that a workspace dependency in conditional dependencies aren't updated automatically…

My first thought was that this is the latest published version so it looks fine, but then I looked at the blame and noticed that you had to manually bump it in fb1b71d (#1919). Was the need to bump it manually due to a bug in cargo-smart-release?

@EliahKagan
Copy link
Member

I've gone ahead with the change described in #1924 (comment) and #1924 (comment). It is in 67d9bf4; once auto-merge completes on #1927, cargo-deny-advisories should be passing on main again.

@Byron
Copy link
Member

Byron commented Apr 5, 2025

To clarify--I think we may be on the same page, but I am not sure--what I am talking about doing will only affect the behavior of the cargo-deny-advisory CI job as run here in this repository. It won't cause anything to be updated automatically, nor cause any change of any kind for people to use gix-testtools in their own project.

Yes, this is what I understand as well.

f the problem that needs to be solved is just that the cargo-deny-advisories CI job in this repository is failing due to a condition that we don't wish to cause it to fail, then modifying the job is definitely a better solution than releasing any crates.

I thought so too.

My first thought was that this is the latest published version so it looks fine, but then I looked at the blame and noticed that you had to manually bump it in fb1b71d (#1919). Was the need to bump it manually due to a bug in cargo-smart-release?

Yes, and now for the first time I seem to have realized what the problem is, or at least one of them. It sues toml-edit to maniplate the manifests, and simply doesn't look at these keyed dictionaries. That would certainly be a simple fix.
The second manual intervention was needed because Cargo publish considers ignored bare repositories untracked files, and aborts unless I remember to clear out these fixtures.
From that point of view I should really accelerate the gix integration into cargo so it will run the git status code instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants