crypto/internal/fips140/nistec: p256NegCond is variable time on ppc64le

[rolandshoemaker]

Due to usage of a conditional branching instruction in the ppc64le implementation of p256NegCond, the function is variable time rather than constant time.

This is a security issue, as it leaks a very small number of bits, but we're treating it as PUBLIC track per the Go Security policy, as it affects a rather niche architecture, and because we're unaware of any protocols this directly affects.

[gopherbot]

Change https://go.dev/cl/643735 mentions this issue: crypto/internal/fips140/nistec: make p256NegCond constant time on ppc64le

[gabyhelp]

Related Code Changes

• crypto/internal/fips140/nistec: make p256NegCond constant time on ppc64le

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[jkrishmys]

@rolandshoemaker Thank you so much for the observation and quick fix.

[qinlonglong123]

@rolandshoemaker @jkrishmys I see that the relevant code in the CL643735 is also present in Go 1.22, Go 1.23, and other versions. Does this vulnerability affect the lower versions? Does it need to be patched? Thanks.

[jkrishmys]

Hi @qinlonglong123. Indeed you are right, thanks. It is tagged under PUBLIC track of Go Security Policy. I am little new to it. If @rolandshoemaker can advice, that will be great.

[rolandshoemaker]

We'll backport this to the next minor releases.

@gopherbot please open backport issues, this is a minor security issue.

[gopherbot]

Backport issue(s) opened: #71422 (for 1.22), #71423 (for 1.23).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[qinlonglong123]

Hi @qinlonglong123. Indeed you are right, thanks. It is tagged under PUBLIC track of Go Security Policy. I am little new to it. If @rolandshoemaker can advice, that will be great.

@jkrishmys Thanks for your reply，I noticed a slight difference in the implementation of the related function p256NegCond between Go 1.18 and Go 1.22. Does Go 1.18 involve this vulnerability?

[qinlonglong123]

@rolandshoemaker @jkrishmys For Go 1.18, can the following commit change resolve this vulnerability?
qinlonglong123@9e017ff
Some of our products are still using Go 1.18, which is very important to us. I look forward to your response. Thank you.

[qinlonglong123]

@rolandshoemaker @jkrishmys For Go 1.18, can the following commit change resolve this vulnerability? qinlonglong123@9e017ff Some of our products are still using Go 1.18, which is very important to us. I look forward to your response. Thank you.

In Go 1.19, the declaration of p256NegCond is as follows:

type p256Element [4]uint64
func p256NegCond(val *p256Element, cond int)

However, in Go 1.18, the declaration is as follows:

func p256NegCond(val []uint64, cond int)

In Go 1.19, the first parameter is a pointer to an array, while in Go 1.18, the first parameter is a slice.

So I'm not quite sure if what I’m seeing is:

MOVD \$40, R17
LXVDSX (R1)(R17), SEL

Is this fetching the second parameter cond value?

[jkrishmys]

Hi @qinlonglong123. Indeed you are right, thanks. It is tagged under PUBLIC track of Go Security Policy. I am little new to it. If @rolandshoemaker can advice, that will be great.

@jkrishmys Thanks for your reply，I noticed a slight difference in the implementation of the related function p256NegCond between Go 1.18 and Go 1.22. Does Go 1.18 involve this vulnerability?

@qinlonglong123, thank you so much, as observed, one of the difference is the need for a byte-swap vector (SWAP) and VPERM instruction (used in Go 1.18) being replaced with the more direct XXPERMDI instruction (in Go 1.22), which is more efficient for the use-case. Further, the conditional branching in Go 1.18 version, makes the function p256NegCond variable time.

[jkrishmys]

@rolandshoemaker @jkrishmys For Go 1.18, can the following commit change resolve this vulnerability? [qinlonglong123/go@9e017ff]

The changes seem to eliminate the conditional branch and ensure constant-time execution. The parameter val is accessed via MOVD val+0(FP), P1ptr. This is suggesting that val is treated as a pointer, as in Go 1.19 (*p256Element). So is the change proposed for Go 1.19 or Go 1.18 ? Because, In Go 1.18, the val is a slice, and this might cause issues, if the data pointer is not extracted correctly. In Go 1.18, the slice descriptor is passed in the shadow space as follows and we may have to fetch the data pointer accordingly:

offset+0: pointer to slice data
offset+8: length of the slice
offset+16: capacity of the slice

(qinlonglong123@9e017ff) Some of our products are still using Go 1.18, which is very important to us. I look forward to your response. Thank you.

We can wait for the comments of @rolandshoemaker about the patch proposed[qinlonglong123/go@9e017ff].

In Go 1.19, the declaration of p256NegCond is as follows:

type p256Element [4]uint64
func p256NegCond(val *p256Element, cond int)

However, in Go 1.18, the declaration is as follows:

func p256NegCond(val []uint64, cond int)

In Go 1.19, the first parameter is a pointer to an array, while in Go 1.18, the first parameter is a slice.

So I'm not quite sure if what I’m seeing is:

MOVD \$40, R17
LXVDSX (R1)(R17), SEL

Is this fetching the second parameter cond value?

According to PowerPC64 ELFv2ABI, R1 points to the current stack frame. R1 + 0 contains the shadowed value of arg1, R1 + 8 contains the shadowed value of arg2 (32+8). Hence, the offset $40 is used in fetching the second parameter cond from its shadowed stack list. If the first argument is a slice (as in Go 1.18), then R3 contains the pointer to the data portion of the slice, and R4 contains the second parameter cond. Whereas in Go 1.19, the first argument being a pointer to an array, the R3 contains the pointer to the array, and R4 still contains second parameter cond. Hence, the difference in the first parameter type ([]uint64 vs. *p256Element) should not directly affect this specific instruction as it is fetching the second parameter (cond).

[qinlonglong123]

@jkrishmys Thank you very much for your reply. Regarding the changes in this CL https://go-review.googlesource.com/c/go/+/643735, I don't fully understand why the second parameter cond can be accessed with an offset of 40 from R1. The comment states "cond is R1 + 8 (cond offset) + 32." I guess that 8 is the offset for the first pointer parameter, but what exactly does the 32 refer to?

[jkrishmys]

Hi @qinlonglong123, according to PowerPC64 ABI document, The Parameter Save Area is a reserved region in the stack frame where the values of parameters passed in registers are shadowed. This ensures that the parameters can be accessed consistently throughout the function, even if the registers are repurposed for other tasks. In the use case, func p256NegCond(val *p256Element, cond int), each parameter (val and cond) is 8 bytes in size. When these parameters are shadowed on the stack, each occupies 8 bytes as well. Here, is a simplified stack frame layout, where the value of register R3 (val) and R4 (cond) is shadowed in the stack frame:

Stack Frame Base (R1)
+-------------------------------+
| Non-volatile Registers Save    
|         Area  (32 bytes)               
|                                                    
+-------------------------------+
| Parameter Save Area (Shadow  
| Space)                                          
|                                                       
| R3 (val)   at R1 + 32                     
| R4 (cond)  at R1 + 40                  
+-------------------------------+
| Local Variables and Other          
| Data                                              
+-------------------------------+

I hope you can now visualise the comment "cond is R1 + 8 (cond offset) + 32."

[gopherbot]

Change https://go.dev/cl/645515 mentions this issue: [release-branch.go1.23] crypto/internal/fips140/nistec: make p256NegCond constant time on ppc64le

[gopherbot]

Change https://go.dev/cl/645535 mentions this issue: [release-branch.go1.22] crypto/internal/fips140/nistec: make p256NegCond constant time on ppc64le

[archanaravindar]

Hi @qinlonglong123, if my understanding is right,
slice descriptor occupies three places as explained by @jkrishmys in here, #71383 (comment)
so if the first value is a slice as in func p256NegCond(val []uint64, cond int)
then val occupies offset+0, offset+8, offset+16, cond starts from offset+24 but we need to add 32 to get the first offset for parameter area in case of ppc64le
so in this case cond can be accessed from R1+24+32 which is R1+56
you can refer to p256Select and see how idx is being accessed that occurs right after table []p256Point
the first pointer is at offset+0, slice p256Point begins at offset+8 .. offset+24 and idx begins at offset+32
The total offset becomes 32+32 = 64 hence R19 is loaded with 64

// func p256Select(point *p256Point, table []p256Point, idx int)

TEXT ·p256Select(SB), NOSPLIT, $0-40        
..
MOVD $64, R19
..
LXVDSX   (R1)(R19), SEL1_ // VLREPG idx+32(FP), SEL1

[archanaravindar]

Hi @qinlonglong123, if my understanding is right, slice descriptor occupies three places as explained by @jkrishmys in here, #71383 (comment) so if

Hi @qinlonglong123 i investigated this code further in my local repository and it appears that the val parameter is being treated as a pointer internally in this case which means cond is still accessible at 8+FP which 8+32 = 40 (R1)
so the offset you use in your code is correct !
The case in p256Select seemed to work at offset 64 because p256Point has three x,y,z integers, apologies for my previous comment, pls ignore that.
The only thing i noticed in your code which can cause an issue is that you use R17
for both loading SEL and accessing Y1L
if you change the register to something like R19 to load SEL it should work, i was able to verify that the test passed with that change on my end

[jkrishmys]

Thanks @archanaravindar , for clarifying it further.