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

Server and client do not verify presence or validity of certain attributes #43

Open
ghost opened this issue Nov 15, 2018 · 4 comments
Open
Labels

Comments

@ghost
Copy link

ghost commented Nov 15, 2018

hi, patch #41 don't consider that Message-Authenticator should be calculated
rfc5997:
If a server supports Status-Server packets, and is configured to respond to them, and receives a packet from a known client, it MUST validate the Message-Authenticator attribute as defined in [RFC3579], Section 3.2. Packets failing that validation MUST be silently discarded.
rfc3579:
When present in an Access-Request packet, Message-Authenticator is an HMAC-MD5 [RFC2104] hash of the entire Access-Request packet, including Type, ID, Length and Authenticator, using the shared secret as the key, as follows. Message-Authenticator = HMAC-MD5 (Type, Identifier, Length, Request Authenticator, Attributes)
thanks,

@ghost
Copy link

ghost commented Nov 15, 2018

Thanks for opening this.

Both the client and server code do not perform any attribute verification that is laid out by the RFCs. This is obviously wrong, and needs to be fixed. The plan of action is to read each of the RFCs we want to conform to and create test cases for each of the the RFC "key words" (MUST, MUST NOT, etc.).

@ghost ghost added the bug label Nov 15, 2018
@gen2brain
Copy link

Hi,

I am trying to query status server and collect statistics, not sure if I am affected by this bug. Can you please take a look:

func main() {
	packet := radius.New(radius.CodeStatusServer, []byte("testing123"))

	//packet.Authenticator[0] = 0x00
	rfc2869.MessageAuthenticator_Set(packet, []byte{0x00})
	PacketType_Set(packet, PacketType_Value_AccessAccept)
	FreeRADIUSStatisticsType_Set(packet, FreeRADIUSStatisticsType_Value_Accounting)

	response, err := radius.Exchange(context.Background(), packet, "127.0.0.1:18121")
	if err == nil {
		fmt.Println(response.Code)
	} else {
		log.Println(err)
	}
}

Whole file with some parts generated from dictionary.freeradius and dictionary.freeradius.internal is here https://gist.github.com/gen2brain/762f6115ff3688e9d016046ccd7c440d . With this it just waits for response, and when I start radiusd with -Xx it says got a packet which isn't RADIUS .

With echo "Message-Authenticator = 0x00, FreeRADIUS-Statistics-Type = 2, Response-Packet-Type = Access-Accept" | radclient -x 127.0.0.1:18121 status testing123 it works, also if I use port 1812.

Thanks

@ghost
Copy link

ghost commented Nov 26, 2018

@gen2brain This library does not auto-populate Message-Authenticator for you; you need to perform the HMAC calculation yourself (see #27 (comment)). So it's possible that you are seeing that error message because freeradius is expecting a Message-Authenticator attribute that has a 16-byte-long value, but you are only providing 0x00.

@gen2brain
Copy link

@bontibon Yes, it works with just a empty 16 byte array. Thanks, and sorry for the noise.

@ghost ghost changed the title CodeStatusServer type packet and Message-Authenticator attribute Server and client do not verify presence or validity of certain attributes Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

1 participant