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

Make SHA1 crate dependency feature gated #543

Closed
hkr1990 opened this issue Jan 11, 2024 · 5 comments · Fixed by #1039
Closed

Make SHA1 crate dependency feature gated #543

hkr1990 opened this issue Jan 11, 2024 · 5 comments · Fixed by #1039
Labels
zbus Issues/PRs related to zbus crate
Milestone

Comments

@hkr1990
Copy link

hkr1990 commented Jan 11, 2024

Zbus crate depends on a crate "SHA1" which is now flagged as "cryptographically broken" by most Rust Component governance tools in major software companies.

I see that SHA1 crate is used for DBUS_SHA1_COOKIE mechanism. I did some reading on the DBUS_SHA1_COOKIE mechanism and looks like it is only needed on legacy unix and windows systems. So, it should be possible to make the DBUS_SHA1_COOKIE feature as optional feature (gated by a feature flag). This will make zbus crate not being flagged by compliance tools and improve its adoption in production.

Thanks,
Hemendra

@hkr1990 hkr1990 changed the title Making SHA1 crate dependency optional Make SHA1 crate dependency optional Jan 11, 2024
@zeenix zeenix closed this as completed Jan 11, 2024
@zeenix zeenix reopened this Jan 11, 2024
@hkr1990 hkr1990 changed the title Make SHA1 crate dependency optional Make SHA1 crate dependency feature gated Jan 12, 2024
@zeenix
Copy link
Contributor

zeenix commented Feb 13, 2024

@hkr1990 zbus 4 already released so you missed the train on this one, I'm afraid. :( We'll have to wait till the next API break.

@zeenix zeenix added api break zbus Issues/PRs related to zbus crate labels Feb 13, 2024
@zeenix zeenix added this to the zbus-5.0 milestone Feb 13, 2024
@hkr1990
Copy link
Author

hkr1990 commented Feb 13, 2024

Sorry about that! When is the next API break expected? Also, I was planning to add the new gating feature as no-sha1, so by default (when no-sha1 feature isn't specified), zbus behavior will stay compatible to zbus 4. The SHA1_COOKIE mechanism will only get disabled when client explicitly specifies no-sha1 in their cargo.toml.

Will you still consider this behavior as API break?

@zeenix
Copy link
Contributor

zeenix commented Feb 13, 2024

When is the next API break expected?

When enough issues require API break are accumulated. :) I honestly hope it won't be anytime soon though. Every break is a burden on the users.

Also, I was planning to add the new gating feature as no-sha1, so by default (when no-sha1 feature isn't specified), zbus behavior will stay compatible to zbus 4. The SHA1_COOKIE mechanism will only get disabled when client explicitly specifies no-sha1 in their cargo.toml.

I thought of that but it will still break existing code that disables default features and rely on cookie auth (on Windows, it's currently the only authenticated mechanism you can use with tokio). This would especially apply to tokio users (I think most of our users) as they disable default features to avoid depending on a bunch of smol-rs crates.

Will you still consider this behavior as API break?

Unfortunately, yes.

@zeenix
Copy link
Contributor

zeenix commented Mar 19, 2024

Giving me thought to this, I don't think most people use cookies so even though it might break things for some people, I think it won't for most people and even for people for whom this will break, it's super easy to fix with a oneline change. So please do contribute the PR.

@zeenix zeenix removed the api break label Mar 19, 2024
@hkr1990
Copy link
Author

hkr1990 commented Mar 19, 2024 via email

zeenix added a commit to zeenix/zbus that referenced this issue Oct 3, 2024
* It drags the `sha1` crate as a dependency, which can be [problematic for some users].
* It makes the handshake more complex, not allowing to pipeline all the commands.
* It's not widely used. If `EXTERNAL` is not an option, you might as well just use `ANONYMOUS`.

Fixes dbus2#727.

[problematic for some users]: dbus2#543
zeenix added a commit to zeenix/zbus that referenced this issue Oct 3, 2024
This was only needed for `DBUS_COOKIE_SHA1` auth mechansim, which was
just dropped so we can happily drop this.

Fixes dbus2#543.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
zbus Issues/PRs related to zbus crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants