Skip to content

Conversation

@qmuntal
Copy link
Collaborator

@qmuntal qmuntal commented Aug 23, 2023

This PR adds support for the ciphers defined in crypto/des. Note that DES doesn't support CTR nor GCM modes, that's by design.

This is the new public API:

// SupportsDESCipher returns true if NewDESCipher is supported.
func SupportsDESCipher() bool

// SupportsTripleDESCipher returns true if NewTripleDESCipher is supported.
func SupportsTripleDESCipher() bool

func NewDESCipher(key []byte) (cipher.Block, error)

func NewTripleDESCipher(key []byte) (cipher.Block, error)

The PR contains a lot of changes because I've refactored AES so it can share almost all the code with DES.

Please review the PR commit by commit, it will be easier. The commit with more changes is 8cf7f70, in which I moved most of the generalized AES code into cipher.go. The commit ed65759 is not required, as we know the block size for the ciphers we support, but IMO it is more robust to take them from OpenSSL.

@qmuntal qmuntal marked this pull request as ready for review August 23, 2023 13:54
Copy link
Collaborator

@dagood dagood left a comment

Choose a reason for hiding this comment

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

A couple suggestions, but LGTM!

@qmuntal
Copy link
Collaborator Author

qmuntal commented Sep 8, 2023

@ueno @derekparker Please take a look

@qmuntal
Copy link
Collaborator Author

qmuntal commented Sep 14, 2023

Weekly remainder: @ueno @derekparker please take a look. Thanks!

Copy link
Collaborator

@ueno ueno left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@qmuntal qmuntal merged commit a4761ff into v2 Sep 14, 2023
@qmuntal qmuntal deleted the des branch September 14, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants