refraction-networking/utls

[BUG] ? StatusRequestV2Extension

VeNoMouS opened this issue · 7 comments

Ran into a situation when using raw bytes, are we sure this was written correctly?

Shouldn't this

read more like...

// Write is a no-op for StatusRequestV2Extension. No data for this extension.
func (e *StatusRequestV2Extension) Write(b []byte) (int, error) {
	fullLen := len(b)
	extData := cryptobyte.String(b)
	// RFC 4366, Section 3.6
	var statusType uint8
	var ignored cryptobyte.String
	
	if !extData.ReadUint16LengthPrefixed(&ignored) ||
		!ignored.ReadUint8(&statusType) ||
		!ignored.ReadUint16LengthPrefixed(&ignored) ||
		!ignored.ReadUint16LengthPrefixed(&ignored) {
		//!extData.ReadUint16LengthPrefixed(&ignored) {
		return fullLen, errors.New("unable to read status request v2 extension data")
	}

	if statusType != statusV2TypeOCSP {
		return fullLen, errors.New("status request v2 extension statusType is not statusV2TypeOCSP(2)")
	}

	return fullLen, nil
}

extData becomes empty after the first ReadUint16LengthPrefixed... so you should traverse ignored after that...

Example

	fmt.Println()
	fmt.Println("Dumping b...")
	fmt.Println(b)
	fmt.Println("Dumping extData")
	fmt.Println(extData)
	fmt.Println()
	
	fmt.Println("Proccessing..")
	fmt.Println(ignored)
	extData.ReadUint16LengthPrefixed(&ignored)
	fmt.Println(ignored)
	ignored.ReadUint8(&statusType)
	fmt.Println(ignored)
	ignored.ReadUint16LengthPrefixed(&ignored)
	fmt.Println(ignored)
	ignored.ReadUint16LengthPrefixed(&ignored) 
	fmt.Println(ignored)
	fmt.Println()
Dumping b...
[0 7 2 0 4 0 0 0 0]
Dumping extData
[0 7 2 0 4 0 0 0 0]

Proccessing..
[]
[2 0 4 0 0 0 0]
[0 4 0 0 0 0]
[0 0 0 0]
[]
gaukas commented

Good catch. I agree that this is a bug. Would you love to submit a pull request addressing this issue?

We did not implement enough testcases to cover all the extensions, and I basically reversed the Read() function for the Write(). So testcases would benefit this project as well.

Yea that's not a problem, unsure why ReadUint16LengthPrefixed reads the last element tho,,, as there should be one more read left... 07, 2, 04, 00, 00

I'll look into that (unless it treating it as \0)

gaukas commented

why ReadUint16LengthPrefixed reads the last element

Can you be more specific on that? Like, which bytes are you referring to as the last element?

why ReadUint16LengthPrefixed reads the last element

Can you be more specific on that? Like, which bytes are you referring to as the last element?

with the last ignored.ReadUint16LengthPrefixed(&ignored) there should still be 0, 0 left in that array to read, but its empty, cryptobyte appears to do something with it... i'll run some tests

[2 0 4 0 0 0 0] <-- 8 
[0 4 0 0 0 0] <-- 16
[0 0 0 0] <-- 16
[] <--- missing
gaukas commented

When you call ignored.ReadUint16LengthPrefixed(&ignored) when ignored == [0 0 0 0] you are reading (consuming) next two bytes from ignored as length (which is 0), then put ignored[0:length] == [] into &ignored. Basically that's where you lose the following bytes by overwriting ignored == [0 0] with [].

ah right , my bad

@gaukas PR sent #247