search

moznion / radius-rs

An async/await native implementation of the RADIUS server and client for Rust.
MIT License
88 stars 14 forks source link

Initial Scaffold for EAP Messages #34

Open sempervictus opened 2 years ago

sempervictus commented 2 years ago

Implement EAP struct with Enums defined from RFC2284 and RFC3579. Implement basic parsing of data into the struct and human-readable output of contents.

This is very much WIP, seeking to address #32, working off of dry RFCs trying to put them together into something coherent for structs and impls. TODO's for MVP:

TODO's for completion:

Testing: Using radtest -t eap-md5 test me localhost 1 somesecretval i am seeing proper output for the EAP packet fields in

$ target/debug/examples/server
EAP Message: Code: EAP-Response, ID: 114, Length: 9, Type: EAP-Identity, Data Length: 4
Found authenticator:
[
    252,
    83,
    114,
    108,
    37,
    126,
    67,
    232,
    150,
    151,
    217,
    166,
    4,
    83,
    120,
    138,
]
sempervictus commented 2 years ago

ping @moznion - could you please take a peek at my last commit? i'm messing something up here, the test client says that my response message authenticator is wrong:

Sent Access-Request Id 52 from 0.0.0.0:43630 to 127.0.0.1:1812 length 67
    User-Name = "test"
    Cleartext-Password = "me"
    NAS-IP-Address = 127.0.1.1
    NAS-Port = 1
    Message-Authenticator = 0x00
    EAP-Code = Response
    EAP-Type-Identity = 0x74657374
    EAP-Message = 0x02e200090174657374
Conflicting response authenticator for reply from 127.0.0.1 (sockfd: 5, id: 52)
Main loop: done.
sempervictus commented 2 years ago

@moznion - is it possible that md5::compute is not correct and what we actually need is Hmac<Md5> as a new type here?

moznion commented 2 years ago

Hi, thank you for your contribution! First of all, I have to catch up on the RADIUS support for EAP, so please hang on a moment. I'll get back it to you today.

moznion commented 2 years ago

ping @moznion - could you please take a peek at my last commit? i'm messing something up here, the test client says that my response message authenticator is wrong:

Who/Where does it raise this message?

moznion commented 2 years ago

is it possible that md5::compute is not correct and what we actually need is Hmac as a new type here?

If you said about Message-Authenticator attribute, that's true. AFAIK that attribute must be a hash value of HMAC-MD5 (ref: https://datatracker.ietf.org/doc/html/rfc2869#section-5.14).

But the MD5 hash calculations in the current implementation, I think that would be legit.

The NAS and RADIUS server share a secret. That shared secret followed by the Request Authenticator is put through a one-way MD5 hash to create a 16 octet digest value which is xored with the password entered by the user, and the xored result placed in the User-Password attribute in the Access-Request packet. See the entry for User-Password in the section on Attributes for a more detailed description. ... The value of the Authenticator field in Access-Accept, Access- Reject, and Access-Challenge packets is called the Response Authenticator, and contains a one-way MD5 hash calculated over a stream of octets consisting of: the RADIUS packet, beginning with the Code field, including the Identifier, the Length, the Request Authenticator field from the Access-Request packet, and the response Attributes, followed by the shared secret. That is, ResponseAuth = MD5(Code+ID+Length+RequestAuth+Attributes+Secret) where + denotes concatenation. https://datatracker.ietf.org/doc/html/rfc2865#section-3

What do you think about that, @sempervictus?

sempervictus commented 2 years ago

ping @moznion - could you please take a peek at my last commit? i'm messing something up here, the test client says that my response message authenticator is wrong:

Who/Where does it raise this message?

That message is thrown by radtest See below for prototypical authenticator impl

sempervictus commented 2 years ago

So looks like other dedicated souls with apparent inclinations toward masochism have done this, but there's not exactly a boatload of example code for us to use. Erlang's eradius seems to have a template for this - i included the relevant snippet in the code comments. Oh and the reason for EAP::new() there is because i take the message out of the match arms result for this commit, but i pulled this work from the branch before my last push because i am failing to pass tests (or possibly to write them correctly)

sempervictus commented 2 years ago

@moznion - any luck on your end divining what i may have done wrong here? I should have a couple of cycles today to get back into it, hoping for some insight from a pair of fresh eyes on the code to help me move forward on the correct line of approach. :smile:

sempervictus commented 2 years ago

@moznion: i've tried a few approaches pulled from Erlang, Python, and C++ code found on GH but they all use different inputs and structural trickery to create the HMAC, they also don't seem to finalize the vector like Rust does (or its implicit), and we can't get bytes from ours until we do. I've attempted to recreate the MessageAuthenticator semantic from eradius' implementation but clearly doing something wrong here - test-case fails abysmally with various inputs which makes me think i'm using the Packet attributes incorrectly. I've also exposed attributes as pub(crate) for now since it looks like i might need them for this, but all ears on how to do this better since trying to implement Packet.get_attributes() told me that Attributes don't implement Copy while being in a shared reference and i'm not looking to make copies of data to confuse the flow.

sempervictus commented 2 years ago

@moznion - is there a way to enforce the order of attribute fields? Not a problem for RFC-compliant packet-parsers, but not great for the HMAC piece when messing with attributes to recreate the data initially checksummed :smile: . Authentication-Request HMAC-MD5 generation works and validates the input from radtest.

I can work on getting this sorted for other types, any chance you might be able to tackle the "business-logic" around response creation for the various states of EAP inner messages and RADIUS outers?

sempervictus commented 2 years ago

@moznion: i'm dealing with the field ordering mess by way of byte-slice replacement for the 16B of the Message-Authenticator for the time being, but i think that this concern is at the Packet level and not so much in the individual fields with which i am working or their internal logic.

moznion commented 2 years ago

Hmm, actually I'm not sure how to fix the order of the struct fields. I'm guessing making a dedicated method to serde would be a way to do that. Anyways, what is the problem in the concrete?

sempervictus commented 2 years ago

When a struct such as [field1,field2,field3] is hashed and then field2 is removed and a replacement appended after field3, the struct containing the same fields in a different order will not sum correctly. I've hacked around this somewhat, but it may be related to the problem i'm seeing in the MessageAuthenticator.for_response() mess - its producing the wrong signature and i'm not sure if there's something structural about how the response is formatted or something i'm doing wrong in terms of the input to that process. Pushed current state for review

moznion commented 2 years ago

Thank you @sempervictus, I understand the current status. I'll take a look at that.

moznion commented 2 years ago

I apologize for my late reply.

I took a look at this problem and I recognize the right way to apply HMAC-MD5. In my understanding, HMAC-MD5 should be applied for the entire RADIUS packet that has 0 filled Message-Authenticator AVP. And it has to implant the result of HMAC-MD5 into the Message-Authenticator placeholder. I'm working on modifying the code to make it pass the radtest chck.