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

Replace AES-CBC with AES-GCM in SecureDataManager #627

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

rgerganov
Copy link
Contributor

Use AES-GCM for encrypting the master key (derived from the PIN) and
get rid of the magic bytes.
Use AES-GCM for encrypting object attributes and thus guarantee the
integrity of the object files (i.e. detect if the object store has been
tampered).

Issue: #623

Use AES-GCM for encrypting the master key (derived from the PIN) and
get rid of the magic bytes.
Use AES-GCM for encrypting object attributes and thus guarantee the
integrity of the object files (i.e. detect if the object store has been
tampered).

Issue: opendnssec#623
@rijswijk
Copy link
Contributor

Thanks for the contribution! This indeed looks like a minimal intervention, although I think it should be a configurable option rather than an immediate change, and it is likely also good to detect what type of encryption an existing token uses, since users that upgrade SoftHSM versions will want to keep using their existing tokens. I realise this may be quite a bit of work, if you don't have time to do that, I suggest we put the PR on hold for now.

@rgerganov
Copy link
Contributor Author

Agree, this needs more work for backward compatibility with existing tokens. I just wanted to push my changes so other people can review and provide feedback. I hope I will be able to find time to complete this in the near future. Also if anyone is willing to collaborate they are free to jump in.

@rijswijk
Copy link
Contributor

OK, thanks, I will tag it as "on hold". I appreciate your patience and contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants