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

Jump to bottom

crypto/cipher: move xorBytes function to internal #28465

Open
mengzhuo opened this issue Oct 29, 2018 · 8 comments
Open

crypto/cipher: move xorBytes function to internal #28465

mengzhuo opened this issue Oct 29, 2018 · 8 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Unplanned

Comments

@mengzhuo
Copy link
Contributor

Some packages need an efficient xor function bytes, for example:

gogrep -x 'for $*_ { $m[i] = $k[i] ^ $l[i] }' std cmd
src/crypto/rand/rand_unix.go:149:3: for i := 0; i < aes.BlockSize; i++ { r.dst[i] = r.time[i] ^ r.seed[i]; }
src/crypto/rand/rand_unix.go:153:3: for i := 0; i < aes.BlockSize; i++ { r.seed[i] = r.time[i] ^ r.dst[i]; }

And about 18 code pieces in golang.org/x/crypto https://github.com/golang/crypto/search?l=Go&q=xor

If this CL( https://go-review.googlesource.com/c/go/+/125316 ) get merged in master we will get a SIMD xorBytes which will be a benefit for all other package that use xor function and reduce redundancy.
@gopherbot gopherbot added this to the Proposal milestone Oct 29, 2018
@agnivade
Copy link
Contributor

Please see this #28113, which was already rejected.

@mengzhuo
Copy link
Contributor Author

mengzhuo commented Oct 29, 2018 via email •
edited

@randall77
Copy link
Contributor

What did you want to use it for, exactly? math/big has an xor operation but I'd be very surprised if that function shows up in profiles of typical math/big operations.

If you want to use xor within the stdlib, we could move such a function to internal/bytealg and redirect current uses to there. That would not involve any new API.

@mengzhuo
Copy link
Contributor Author

Moving xor into internal sounds great, I will change title to that.
But I found that math/big using xor for []Word(uint) instead of []byte.

replace target:
math/big

go/src/math/big/nat.go

Line 951 in d1836e6

func (z nat) xor(x, y nat) nat {

crypto/rand

go/src/crypto/rand/rand_unix.go

Line 152 in d1836e6

r.dst[i] = r.time[i] ^ r.seed[i]

@mengzhuo mengzhuo changed the title proposal: crypto/cipher: export xorBytes function proposal: crypto/cipher: move xorBytes function to internal Oct 30, 2018
@agnivade
Copy link
Contributor

/cc @FiloSottile

@andybons
Copy link
Member

This appears to be an internal implementation detail, not a user-facing change. @FiloSottile should still take a look, but removing the proposal tag.

@andybons andybons added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Proposal labels Oct 31, 2018
@andybons andybons added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 31, 2018
@andybons andybons changed the title proposal: crypto/cipher: move xorBytes function to internal crypto/cipher: move xorBytes function to internal Oct 31, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 31, 2018
@andybons andybons removed the Proposal label Oct 31, 2018
@andybons andybons removed this from the Proposal milestone Oct 31, 2018
@andybons andybons added the Proposal-Crypto Proposal related to crypto packages or other security issues label Oct 31, 2018
@crvv crvv mentioned this issue Apr 22, 2019
Closed
@bcmills
Copy link
Contributor

bcmills commented Mar 11, 2020

See also #35381, #25111, #37644.

@mengzhuo
Copy link
Contributor Author

mengzhuo commented Mar 24, 2020 •
edited

Hi everyone

I propose that adding new Xor function to internal/bytealg/xor_generic.go

// Xor dst must greater or equal to minimal length of a or b.
// return with bytes count that had been XOR with.
func Xor(dst, a, b []byte) (n int)

For native implement

package bytealg                                               
                                                              
import "unsafe"                                               
                                                              
func Xor(dst, a, b []byte) (n int) {                          
        n = len(a)                                            
        if n > len(b) {                                       
                n = len(b)                                    
        }                                                     
        memxor(unsafe.Pointer(&dst[0]), unsafe.Pointer(&a[0]),
                unsafe.Pointer(&b[0]), uintptr(n))            
        return                                                
}                                                             
                                                              
//go:noescape                                                 
func memxor(dstp, ap, bp unsafe.Pointer, n uintptr)

Any suggestion?

@josharian josharian mentioned this issue May 21, 2022
Closed
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@andybons @mengzhuo @agnivade @bcmills @randall77 @gopherbot @seankhliao