Skip to content

Bumped secrecy crate from 0.8.0 to 0.10.3 #287

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 13, 2025
Merged

Bumped secrecy crate from 0.8.0 to 0.10.3 #287

merged 2 commits into from
Jun 13, 2025

Conversation

ptdecker
Copy link
Contributor

Bumped secrecy crate from 0.8.8 to 0.10.3.

Addresses issue Update Secrecy Crate.

The change secrecy from 0.8.0 to 0.10.3 is a minor breaking change. To correct for this, a single line change was needed in cryptoki/src/types.rs L283 to change:

pub type RawAuthPin = SecretVec<u8>;

to

pub type RawAuthPin = SecretBox<Vec<u8>>;

The other changes included in this PR are mainly cosmetic. Specifically:

  • Modified cryptoki/Cargo.toml to bump version.
  • Added INSTALL_NOTES.md to start capturing installation notes for specific development environments.
  • Grammar fixes.

@ptdecker ptdecker mentioned this pull request Jun 11, 2025
Closed
Jakuje
Jakuje previously approved these changes Jun 11, 2025
View reviewed changes
Copy link
Collaborator

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

Sounds like the update was quite easy in the end. Ack for the change. Inline comment only for the readme, but I am not a mac user.

INSTALL_NOTES.md Outdated
Comment on lines 20 to 32
```bash
$ cd ~/code
$ gh repo clone softhsm/SoftHSMv2
$ brew install automake cppunit
$ brew reinstall autoconf automake
$ export CPPFLAGS="-I/opt/homebrew/opt/openssl@3/include -I/opt/homebrew/opt/cppunit/include"
$ export LDFLAGS="-L/opt/homebrew/opt/openssl@3/lib -L/opt/homebrew/opt/cppunit/lib"
$ mkdir -p ~/softhsm2/tokens
$ echo "directories.tokendir = $HOME/softhsm2/tokens" > ~/softhsm2/softhsm2.conf
$ export SOFTHSM2_CONF=$HOME/softhsm2/softhsm2.conf
$ ./src/bin/util/softhsm2-util --init-token --slot 0 --label "TestToken" --so-pin abcdef --pin fedcba --module ./src/lib/.libs/libsofthsm2.so
$ vi openssl.cnf
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes me wonder whey the softhsm was not installed directly from brew? Is the package there broken in some way? Or what would be the reason not to use the brew provided version?

https://formulae.brew.sh/formula/softhsm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! @Jakuje , I didn't even thing about installing from brew! I just tried to build it from source. Damn, that probably would have saved me some time.

@wiktor-k
Copy link
Collaborator

You can fix the sign-off via git commit --amend --signoff --no-edit && git push --force-with-lease (phew, hopefully I got all of that right 😅 ).

@ptdecker
Bumped secrecy crate from 0.8.0 to 0.10.3
b29c6a5 
- Modified cryptoki/Cargo.toml to bump version.
- Added INSTALL_NOTES.md to start capturing installation notes for specific development environments.
- Changed deprecated SecretVec<u8> to SecretBox<Vec<u8>>.
- Grammar fixes.

Signed-off-by: Todd Decker <[email protected]>
@ptdecker ptdecker force-pushed the ptdecker/secrecy-upgrade branch from 707a7b2 to b29c6a5 Compare June 12, 2025 15:50
@ptdecker ptdecker marked this pull request as ready for review June 12, 2025 15:51
@ptdecker
Copy link
Contributor Author

You can fix the sign-off via git commit --amend --signoff --no-edit && git push --force-with-lease (phew, hopefully I got all of that right 😅 ).

This is done @wiktor-k . Looks like "two approving reviews" are required and I have one so far from @Jakuje

@Jakuje
Copy link
Collaborator

Jakuje commented Jun 12, 2025

To clarify, I am ok with the changes to the code, but I am not sure if we need the documentation, given that it can be replaced with a oneliner "install softhsm from brew".

@ptdecker
Copy link
Contributor Author

To clarify, I am ok with the changes to the code, but I am not sure if we need the documentation, given that it can be replaced with a oneliner "install softhsm from brew".

No worries, @Jakuje . I'll drop that file

@ptdecker
Removing INSTALL_NOTES.md
e783630 
Signed-off-by: Todd Decker <[email protected]>
@ptdecker ptdecker force-pushed the ptdecker/secrecy-upgrade branch from 70a9105 to e783630 Compare June 12, 2025 20:15
@ptdecker ptdecker requested a review from Jakuje June 12, 2025 20:33
wiktor-k
wiktor-k approved these changes Jun 13, 2025
View reviewed changes
Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks!

@wiktor-k wiktor-k merged commit 52daf5f into parallaxsecond:main Jun 13, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wiktor-k wiktor-k wiktor-k approved these changes

@Jakuje Jakuje Jakuje approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ptdecker @wiktor-k @Jakuje