Skip to content

crypto/aes: expandKeyGo inconsistent with expandKeyAsm in endiannes #34823

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
Closed
xujintao opened this issue Oct 10, 2019 · 3 comments
Closed

crypto/aes: expandKeyGo inconsistent with expandKeyAsm in endiannes #34823

xujintao opened this issue Oct 10, 2019 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@xujintao
Copy link 

$ go version
go version go1.12 linux/amd64

What did you do?

0, given key:

var key = [32]byte{0x12, 0x34, 0x56, 0x78}

1, cpu does not support AES, select expandKey with BigEndian, enc[0] is 0x12345678

func expandKeyGo(key []byte, enc, dec []uint32) {
	// Encryption key setup.
	var i int
	nk := len(key) / 4
	for i = 0; i < nk; i++ {
		enc[i] = binary.BigEndian.Uint32(key[4*i:])
	}
...

2, cpu supports AES, use expandKeyAsm and movups, enc[0] is 0x78563412 on x86

// func expandKeyAsm(nr int, key *byte, enc, dec *uint32) {
// Note that round keys are stored in uint128 format, not uint32
TEXT ·expandKeyAsm(SB),NOSPLIT,$0
	MOVQ nr+0(FP), CX
	MOVQ key+8(FP), AX
	MOVQ enc+16(FP), BX
	MOVQ dec+24(FP), DX
	MOVUPS (AX), X0
	// enc
	MOVUPS X0, (BX)
	ADDQ $16, BX
	PXOR X4, X4 // _expand_key_* expect X4 to be zero
	CMPL CX, $12
	JE Lexp_enc196
	JB Lexp_enc128
Lexp_enc256:
...

What did you expect to see?

consistency

What did you see instead?

not consistency
@andybons andybons changed the title expandKeyGo, Big-Endian or Little-Endian? crypto/aes: expandKeyGo inconsistent with expandKeyAsm in endiannes Oct 10, 2019
@andybons
Copy link
Member

Is this a user-visible problem or a code quality issue? If it's the former can you provide a repro that shows code breaking due to this behavior?

@FiloSottile

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 10, 2019
@andybons andybons added this to the Unplanned milestone Oct 10, 2019
@xujintao
Copy link
Author

Sorry, i'm wrong.
All the operation is bitwise opration, shift or xor. So endian makes no sense here.
Please close.

@ianlancetaylor
Copy link
Member

Thanks for following up.

@golang golang locked and limited conversation to collaborators Oct 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@andybons @ianlancetaylor @xujintao @gopherbot