Merge pull request #599 from rod-hynes/ntlm-fixes

go-ntlm fixes

psiphon/upstreamproxy/go-ntlm/README.md

@@ -1,3 +1,8 @@
+
+This is a fork of the package that previously existed at https://github.com/ThomsonReutersEikon/go-ntlm/ntlm. It contains several bug fixes, tagged with "[Psiphon]".
+
+Original github.com/ThomsonReutersEikon/go-ntlm/ntlm README:
+
 # NTLM Implementation for Go
 
 This is a native implementation of NTLM for Go that was implemented using the Microsoft MS-NLMP documentation available at http://msdn.microsoft.com/en-us/library/cc236621.aspx.

psiphon/upstreamproxy/go-ntlm/ntlm/av_pairs.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"encoding/hex"
+	"errors"
 	"fmt"
 )
 
@@ -51,13 +52,16 @@ func (p *AvPairs) AddAvPair(avId AvPairType, bytes []byte) {
 	p.List = append(p.List, *a)
 }
 
-func ReadAvPairs(data []byte) *AvPairs {
+func ReadAvPairs(data []byte) (*AvPairs, error) {
 	pairs := new(AvPairs)
 
 	// Get the number of AvPairs and allocate enough AvPair structures to hold them
 	offset := 0
 	for i := 0; len(data) > 0 && i < 11; i++ {
-		pair := ReadAvPair(data, offset)
+		pair, err := ReadAvPair(data, offset)
+		if err != nil {
+			return nil, err
+		}
 		offset = offset + 4 + int(pair.AvLen)
 		pairs.List = append(pairs.List, *pair)
 		if pair.AvId == MsvAvEOL {
@@ -65,7 +69,7 @@ func ReadAvPairs(data []byte) *AvPairs {
 		}
 	}
 
-	return pairs
+	return pairs, nil
 }
 
 func (p *AvPairs) Bytes() (result []byte) {
@@ -131,12 +135,26 @@ type AvPair struct {
 	Value []byte
 }
 
-func ReadAvPair(data []byte, offset int) *AvPair {
+func ReadAvPair(data []byte, offset int) (*AvPair, error) {
+
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(data) < offset+4 {
+		return nil, errors.New("invalid AvPair")
+	}
+
 	pair := new(AvPair)
 	pair.AvId = AvPairType(binary.LittleEndian.Uint16(data[offset : offset+2]))
 	pair.AvLen = binary.LittleEndian.Uint16(data[offset+2 : offset+4])
+
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(data) < offset+4+int(pair.AvLen) {
+		return nil, errors.New("invalid AvPair")
+	}
+
 	pair.Value = data[offset+4 : offset+4+int(pair.AvLen)]
-	return pair
+	return pair, nil
 }
 
 func (a *AvPair) UnicodeStringValue() string {

psiphon/upstreamproxy/go-ntlm/ntlm/challenge_responses.go

@@ -21,6 +21,13 @@ func (n *NtlmV1Response) String() string {
 }
 
 func ReadNtlmV1Response(bytes []byte) (*NtlmV1Response, error) {
+
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(bytes) < 24 {
+		return nil, errors.New("invalid NTLM v1 response")
+	}
+
 	r := new(NtlmV1Response)
 	r.Response = bytes[0:24]
 	return r, nil
@@ -84,6 +91,13 @@ func (n *NtlmV2Response) String() string {
 }
 
 func ReadNtlmV2Response(bytes []byte) (*NtlmV2Response, error) {
+
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(bytes) < 45 {
+		return nil, errors.New("invalid NTLM v2 response")
+	}
+
 	r := new(NtlmV2Response)
 	r.Response = bytes[0:16]
 	r.NtlmV2ClientChallenge = new(NtlmV2ClientChallenge)
@@ -103,7 +117,11 @@ func ReadNtlmV2Response(bytes []byte) (*NtlmV2Response, error) {
 	c.ChallengeFromClient = bytes[32:40]
 	// Ignoring - 4 bytes reserved
 	// c.Reserved3
-	c.AvPairs = ReadAvPairs(bytes[44:])
+	var err error
+	c.AvPairs, err = ReadAvPairs(bytes[44:])
+	if err != nil {
+		return nil, err
+	}
 	return r, nil
 }
 
@@ -114,10 +132,17 @@ type LmV1Response struct {
 	Response []byte
 }
 
-func ReadLmV1Response(bytes []byte) *LmV1Response {
+func ReadLmV1Response(bytes []byte) (*LmV1Response, error) {
+
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(bytes) < 24 {
+		return nil, errors.New("invalid LM v1 response")
+	}
+
 	r := new(LmV1Response)
 	r.Response = bytes[0:24]
-	return r
+	return r, nil
 }
 
 func (l *LmV1Response) String() string {
@@ -136,11 +161,18 @@ type LmV2Response struct {
 	ChallengeFromClient []byte
 }
 
-func ReadLmV2Response(bytes []byte) *LmV2Response {
+func ReadLmV2Response(bytes []byte) (*LmV2Response, error) {
+
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(bytes) < 24 {
+		return nil, errors.New("invalid LM v2 response")
+	}
+
 	r := new(LmV2Response)
 	r.Response = bytes[0:16]
 	r.ChallengeFromClient = bytes[16:24]
-	return r
+	return r, nil
 }
 
 func (l *LmV2Response) String() string {

psiphon/upstreamproxy/go-ntlm/ntlm/message_authenticate.go

@@ -38,7 +38,7 @@ type AuthenticateMessage struct {
 	/// MS-NLMP 2.2.1.3 - In connectionless mode, a NEGOTIATE structure that contains a set of bit flags (section 2.2.2.5) and represents the
 	// conclusion of negotiation—the choices the client has made from the options the server offered in the CHALLENGE_MESSAGE.
 	// In connection-oriented mode, a NEGOTIATE structure that contains the set of bit flags (section 2.2.2.5) negotiated in
-	// the previous 
+	// the previous
 	NegotiateFlags uint32 // 4 bytes
 
 	// Version (8 bytes): A VERSION structure (section 2.2.2.10) that is present only when the NTLMSSP_NEGOTIATE_VERSION
@@ -56,6 +56,12 @@ type AuthenticateMessage struct {
 func ParseAuthenticateMessage(body []byte, ntlmVersion int) (*AuthenticateMessage, error) {
 	am := new(AuthenticateMessage)
 
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(body) < 12 {
+		return nil, errors.New("invalid authenticate message")
+	}
+
 	am.Signature = body[0:8]
 	if !bytes.Equal(am.Signature, []byte("NTLMSSP\x00")) {
 		return nil, errors.New("Invalid NTLM message signature")
@@ -74,9 +80,12 @@ func ParseAuthenticateMessage(body []byte, ntlmVersion int) (*AuthenticateMessag
 	}
 
 	if ntlmVersion == 2 {
-		am.LmV2Response = ReadLmV2Response(am.LmChallengeResponse.Payload)
+		am.LmV2Response, err = ReadLmV2Response(am.LmChallengeResponse.Payload)
 	} else {
-		am.LmV1Response = ReadLmV1Response(am.LmChallengeResponse.Payload)
+		am.LmV1Response, err = ReadLmV1Response(am.LmChallengeResponse.Payload)
+	}
+	if err != nil {
+		return nil, err
 	}
 
 	am.NtChallengeResponseFields, err = ReadBytePayload(20, body)
@@ -90,7 +99,6 @@ func ParseAuthenticateMessage(body []byte, ntlmVersion int) (*AuthenticateMessag
 	} else {
 		am.NtlmV1Response, err = ReadNtlmV1Response(am.NtChallengeResponseFields.Payload)
 	}
-
 	if err != nil {
 		return nil, err
 	}
@@ -124,11 +132,24 @@ func ParseAuthenticateMessage(body []byte, ntlmVersion int) (*AuthenticateMessag
 		}
 		offset = offset + 8
 
+		// [Psiphon]
+		// Don't panic on malformed remote input.
+		if len(body) < offset+4 {
+			return nil, errors.New("invalid authenticate message")
+		}
+
 		am.NegotiateFlags = binary.LittleEndian.Uint32(body[offset : offset+4])
 		offset = offset + 4
 
 		// Version (8 bytes): A VERSION structure (section 2.2.2.10) that is present only when the NTLMSSP_NEGOTIATE_VERSION flag is set in the NegotiateFlags field. This structure is used for debugging purposes only. In normal protocol messages, it is ignored and does not affect the NTLM message processing.<9>
 		if NTLMSSP_NEGOTIATE_VERSION.IsSet(am.NegotiateFlags) {
+
+			// [Psiphon]
+			// Don't panic on malformed remote input.
+			if len(body) < offset+8 {
+				return nil, errors.New("invalid authenticate message")
+			}
+
 			am.Version, err = ReadVersionStruct(body[offset : offset+8])
 			if err != nil {
 				return nil, err
@@ -144,12 +165,25 @@ func ParseAuthenticateMessage(body []byte, ntlmVersion int) (*AuthenticateMessag
 		// there is a MIC and read it out.
 		var lowestOffset = am.getLowestPayloadOffset()
 		if lowestOffset > offset {
+
+			// [Psiphon]
+			// Don't panic on malformed remote input.
+			if len(body) < offset+16 {
+				return nil, errors.New("invalid authenticate message")
+			}
+
 			// MIC - 16 bytes
 			am.Mic = body[offset : offset+16]
 			offset = offset + 16
 		}
 	}
 
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(body) < offset {
+		return nil, errors.New("invalid authenticate message")
+	}
+
 	am.Payload = body[offset:]
 
 	return am, nil

psiphon/upstreamproxy/go-ntlm/ntlm/message_challenge.go

@@ -56,6 +56,12 @@ type ChallengeMessage struct {
 func ParseChallengeMessage(body []byte) (*ChallengeMessage, error) {
 	challenge := new(ChallengeMessage)
 
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(body) < 40 {
+		return nil, errors.New("invalid challenge message")
+	}
+
 	challenge.Signature = body[0:8]
 	if !bytes.Equal(challenge.Signature, []byte("NTLMSSP\x00")) {
 		return challenge, errors.New("Invalid NTLM message signature")
@@ -84,11 +90,21 @@ func ParseChallengeMessage(body []byte) (*ChallengeMessage, error) {
 		return nil, err
 	}
 
-	challenge.TargetInfo = ReadAvPairs(challenge.TargetInfoPayloadStruct.Payload)
+	challenge.TargetInfo, err = ReadAvPairs(challenge.TargetInfoPayloadStruct.Payload)
+	if err != nil {
+		return nil, err
+	}
 
 	offset := 48
 
 	if NTLMSSP_NEGOTIATE_VERSION.IsSet(challenge.NegotiateFlags) {
+
+		// [Psiphon]
+		// Don't panic on malformed remote input.
+		if len(body) < offset+8 {
+			return nil, errors.New("invalid challenge message")
+		}
+
 		challenge.Version, err = ReadVersionStruct(body[offset : offset+8])
 		if err != nil {
 			return nil, err
@@ -96,6 +112,12 @@ func ParseChallengeMessage(body []byte) (*ChallengeMessage, error) {
 		offset = offset + 8
 	}
 
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(body) < offset {
+		return nil, errors.New("invalid challenge message")
+	}
+
 	challenge.Payload = body[offset:]
 
 	return challenge, nil

psiphon/upstreamproxy/go-ntlm/ntlm/ntlmv1.go

@@ -343,7 +343,12 @@ func (n *V1ClientSession) GenerateAuthenticateMessage() (am *AuthenticateMessage
 	am.NtChallengeResponseFields, _ = CreateBytePayload(n.ntChallengeResponse)
 	am.DomainName, _ = CreateStringPayload(n.userDomain)
 	am.UserName, _ = CreateStringPayload(n.user)
-	am.Workstation, _ = CreateStringPayload("SQUAREMILL")
+
+	// [Psiphon]
+	// Set a blank workstation value, which is less distinguishable than the previous hard-coded value.
+	// See also: https://github.com/Azure/go-ntlmssp/commit/5e29b886690f00c76b876ae9ab8e31e7c3509203.
+
+	am.Workstation, _ = CreateStringPayload("")
 	am.EncryptedRandomSessionKey, _ = CreateBytePayload(n.encryptedRandomSessionKey)
 	am.NegotiateFlags = n.NegotiateFlags
 	am.Version = &VersionStruct{ProductMajorVersion: uint8(5), ProductMinorVersion: uint8(1), ProductBuild: uint16(2600), NTLMRevisionCurrent: uint8(15)}

psiphon/upstreamproxy/go-ntlm/ntlm/ntlmv1_test.go

@@ -42,14 +42,14 @@ func checkV1Value(t *testing.T, name string, value []byte, expected string, err
 // would authenticate. This was due to a bug in the MS-NLMP docs. This tests for that issue
 func TestNtlmV1ExtendedSessionSecurity(t *testing.T) {
 	// NTLMv1 with extended session security
-  challengeMessage := "TlRMTVNTUAACAAAAAAAAADgAAABVgphiRy3oSZvn1I4AAAAAAAAAAKIAogA4AAAABQEoCgAAAA8CAA4AUgBFAFUAVABFAFIAUwABABwAVQBLAEIAUAAtAEMAQgBUAFIATQBGAEUAMAA2AAQAFgBSAGUAdQB0AGUAcgBzAC4AbgBlAHQAAwA0AHUAawBiAHAALQBjAGIAdAByAG0AZgBlADAANgAuAFIAZQB1AHQAZQByAHMALgBuAGUAdAAFABYAUgBlAHUAdABlAHIAcwAuAG4AZQB0AAAAAAA="
-  authenticateMessage := "TlRMTVNTUAADAAAAGAAYAJgAAAAYABgAsAAAAAAAAABIAAAAOgA6AEgAAAAWABYAggAAABAAEADIAAAAVYKYYgUCzg4AAAAPMQAwADAAMAAwADEALgB3AGMAcABAAHQAaABvAG0AcwBvAG4AcgBlAHUAdABlAHIAcwAuAGMAbwBtAE4AWQBDAFMATQBTAEcAOQA5ADAAOQBRWAK3h/TIywAAAAAAAAAAAAAAAAAAAAA3tp89kZU1hs1XZp7KTyGm3XsFAT9stEDW9YXDaeYVBmBcBb//2FOu"
+	challengeMessage := "TlRMTVNTUAACAAAAAAAAADgAAABVgphiRy3oSZvn1I4AAAAAAAAAAKIAogA4AAAABQEoCgAAAA8CAA4AUgBFAFUAVABFAFIAUwABABwAVQBLAEIAUAAtAEMAQgBUAFIATQBGAEUAMAA2AAQAFgBSAGUAdQB0AGUAcgBzAC4AbgBlAHQAAwA0AHUAawBiAHAALQBjAGIAdAByAG0AZgBlADAANgAuAFIAZQB1AHQAZQByAHMALgBuAGUAdAAFABYAUgBlAHUAdABlAHIAcwAuAG4AZQB0AAAAAAA="
+	authenticateMessage := "TlRMTVNTUAADAAAAGAAYAJgAAAAYABgAsAAAAAAAAABIAAAAOgA6AEgAAAAWABYAggAAABAAEADIAAAAVYKYYgUCzg4AAAAPMQAwADAAMAAwADEALgB3AGMAcABAAHQAaABvAG0AcwBvAG4AcgBlAHUAdABlAHIAcwAuAGMAbwBtAE4AWQBDAFMATQBTAEcAOQA5ADAAOQBRWAK3h/TIywAAAAAAAAAAAAAAAAAAAAA3tp89kZU1hs1XZp7KTyGm3XsFAT9stEDW9YXDaeYVBmBcBb//2FOu"
 
 	challengeData, _ := base64.StdEncoding.DecodeString(challengeMessage)
 	c, _ := ParseChallengeMessage(challengeData)
 
-  authenticateData, _ := base64.StdEncoding.DecodeString(authenticateMessage)
-  msg, err := ParseAuthenticateMessage(authenticateData, 1)
+	authenticateData, _ := base64.StdEncoding.DecodeString(authenticateMessage)
+	msg, err := ParseAuthenticateMessage(authenticateData, 1)
 	if err != nil {
 		t.Errorf("Could not process authenticate message: %s", err)
 	}
@@ -62,7 +62,7 @@ func TestNtlmV1ExtendedSessionSecurity(t *testing.T) {
 	context.SetServerChallenge(c.ServerChallenge)
 	err = context.ProcessAuthenticateMessage(msg)
 	if err == nil {
-		t.Errorf("This message should have failed to authenticate, but it passed", err)
+		t.Errorf("This message should have failed to authenticate, but it passed")
 	}
 }
 

psiphon/upstreamproxy/go-ntlm/ntlm/ntlmv2.go

@@ -88,7 +88,6 @@ func (n *V2Session) Sign(message []byte) ([]byte, error) {
 	return nil, nil
 }
 
-//Mildly ghetto that we expose this
 func NtlmVCommonMac(message []byte, sequenceNumber int, sealingKey, signingKey []byte, NegotiateFlags uint32) []byte {
 	var handle *rc4P.Cipher
 	// TODO: Need to keep track of the sequence number for connection oriented NTLM
@@ -378,7 +377,12 @@ func (n *V2ClientSession) GenerateAuthenticateMessage() (am *AuthenticateMessage
 	am.NtChallengeResponseFields, _ = CreateBytePayload(n.ntChallengeResponse)
 	am.DomainName, _ = CreateStringPayload(n.userDomain)
 	am.UserName, _ = CreateStringPayload(n.user)
-	am.Workstation, _ = CreateStringPayload("SQUAREMILL")
+
+	// [Psiphon]
+	// Set a blank workstation value, which is less distinguishable than the previous hard-coded value.
+	// See also: https://github.com/Azure/go-ntlmssp/commit/5e29b886690f00c76b876ae9ab8e31e7c3509203.
+
+	am.Workstation, _ = CreateStringPayload("")
 	am.EncryptedRandomSessionKey, _ = CreateBytePayload(n.encryptedRandomSessionKey)
 	am.NegotiateFlags = n.NegotiateFlags
 	am.Mic = make([]byte, 16)

psiphon/upstreamproxy/go-ntlm/ntlm/ntlmv2_test.go

@@ -172,7 +172,7 @@ func TestNTLMv2WithDomain(t *testing.T) {
 
 	err := server.ProcessAuthenticateMessage(a)
 	if err != nil {
-		t.Error("Could not process authenticate message: %s\n", err)
+		t.Errorf("Could not process authenticate message: %s\n", err)
 	}
 }
 

psiphon/upstreamproxy/go-ntlm/ntlm/payload.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"encoding/hex"
+	"errors"
 )
 
 const (
@@ -80,6 +81,12 @@ func ReadBytePayload(startByte int, bytes []byte) (*PayloadStruct, error) {
 func ReadPayloadStruct(startByte int, bytes []byte, PayloadType int) (*PayloadStruct, error) {
 	p := new(PayloadStruct)
 
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(bytes) < startByte+8 {
+		return nil, errors.New("invalid payload")
+	}
+
 	p.Type = PayloadType
 	p.Len = binary.LittleEndian.Uint16(bytes[startByte : startByte+2])
 	p.MaxLen = binary.LittleEndian.Uint16(bytes[startByte+2 : startByte+4])
@@ -87,6 +94,13 @@ func ReadPayloadStruct(startByte int, bytes []byte, PayloadType int) (*PayloadSt
 
 	if p.Len > 0 {
 		endOffset := p.Offset + uint32(p.Len)
+
+		// [Psiphon]
+		// Don't panic on malformed remote input.
+		if len(bytes) < int(endOffset) {
+			return nil, errors.New("invalid payload")
+		}
+
 		p.Payload = bytes[p.Offset:endOffset]
 	}
 

psiphon/upstreamproxy/go-ntlm/ntlm/version.go

@@ -5,6 +5,7 @@ package ntlm
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 )
 
@@ -19,6 +20,12 @@ type VersionStruct struct {
 func ReadVersionStruct(structSource []byte) (*VersionStruct, error) {
 	versionStruct := new(VersionStruct)
 
+	// [Psiphon]
+	// Don't panic on malformed remote input.
+	if len(structSource) < 8 {
+		return nil, errors.New("invalid version struct")
+	}
+
 	versionStruct.ProductMajorVersion = uint8(structSource[0])
 	versionStruct.ProductMinorVersion = uint8(structSource[1])
 	versionStruct.ProductBuild = binary.LittleEndian.Uint16(structSource[2:4])