Initial Scaffold for EAP Messages #34

sempervictus · 2022-07-28T12:45:34Z

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:

MD5 Message Authenticator generation & validation
Data segment processing
EAP transaction state management (this merits discussion as state handling is generally done in the server but message identifiers in EAP segments have to be deconflicted)
Identity access-request/eap-response
MD5-Challenge authentication

TODO's for completion:

OTP authentication
SmartCard authentication

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,
]

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

Write basic field tests for EAP elements, implement a to_bytes() method, misc touchups. Testing: EAP parsing tests pass

Moved the segments of code which produce validators for message authentication into their own methods, using them inside existing is_authentic_* methods in Packet. Implemented EAP generation length fixes. Added mashalling test. Implemented an MD5-Challenge response using the EAP message struct defined in earlier commits in the test server, but failing in the client-side with "Conflicting response authenticator for reply ..." This seems to indicate that either the Message-Authenticator value being produced is incorrect, or, more likely, that i am using the code incorrectly. Testing: Passes all tests currently defined

sempervictus · 2022-07-29T00:32:03Z

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 · 2022-07-30T11:53:29Z

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

moznion · 2022-07-31T02:15:12Z

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

I really appreciate your contribution!
I left some comments, but I know this is still WIP status :)

moznion · 2022-07-31T06:45:17Z

radius/src/core/eap.rs

+    }
+    /// Create an EAP message structure from a slice of bytes
+    pub fn from_bytes(eap_bytes: &[u8]) -> Self {
+        let code = EAPCode::from(eap_bytes[0]);


Here could be panic because when the eap_bytes doesn't have sufficient length of data.
Would it be better to check the length at the entry of this method?

Yes - i haven't implemented safety latches for stuff yet as i'm just trying to get valid content on the wire.

moznion · 2022-07-31T06:53:42Z

radius/src/core/eap.rs

+
+impl EAP {
+    /// Create an (invalid) EAP message structure
+    pub fn new() -> Self {


I suppose this constructor should be split into two methods. For example:

pub fn new(code: EAPCode, id: u8, typ: EAPType, data: &Vec<u8>) to make a EAP object as immutable and calculate the length in the constructor

pub fn make_invalid() to make an invalid, i.e. empty, object

I mean the normal constructor should afford the immutable operation.

Same as above, definitely all for being able to instantiate prepared immutable structures as responses, new ones, etc, but trying to get wire-level correctness first.

moznion · 2022-07-31T07:00:33Z

examples/eap_server.rs

+}
+
+struct MyRequestHandler {
+    secret_provider: MySecretProvider


[nit] is this secret provider needed?

Copy-pasta, with pesto :)

moznion · 2022-07-31T07:02:05Z

examples/eap_server.rs

+//                                                 Access-Accept/
+//                                                 EAP-Message/EAP-Success
+//                                                 (other attributes)
+//                         <- EAP-Success


just a question: does this flow diagram has the original document? For example, RFC document.

Yeah, all of the dry stuff in the comments strewn about is copy-pasted from RFCs. This bloody process is described in like 3 - the two i mentioned and the actual EAP RFC, plus HMAC-MD5 has its own (stdby for proto-code)

moznion · 2022-07-31T07:07:55Z

examples/eap_server.rs

+                println!("No eap message found");
+                EAP::new()
+            }
+        };


It seems the result of this match expression is discarded. What does this code intend?

It is, but it satisfies the constraints of the match (maybe thats not needed anymore, been doing it this way for a few years).

moznion · 2022-07-31T07:24:32Z

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 · 2022-07-31T07:27:31Z

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 · 2022-07-31T11:03:43Z

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 · 2022-07-31T11:23:29Z

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 · 2022-08-02T11:48:52Z

@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. 😄

sempervictus · 2022-08-03T10:40:53Z

@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 · 2022-08-03T20:42:05Z

@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 😄 .
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?

Implement a new type using the hmac and md-5 crates, namespacing md-5 as hmac_md5 to avoid collision with existing crate. Stub out MessageAuthenticator struct & implementation to create HMACMD5 signature buffers from Packet's encode() byte vector. Implement Message-Authenticator input structure for starting the chain of HMAC exchanges and verifying Access-Request messages. Testing: Implemented test to verify Access-Request message HMACMD5 - pass

sempervictus · 2022-08-04T13:06:41Z

@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 · 2022-08-04T23:44:59Z

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?

Small amount of cleanup and progress on responding to an Identity Access-Request. Generating bad Message-Authenticator values in for_response, likely not grokking the requisite field value correctly

sempervictus · 2022-08-05T02:29:45Z

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 · 2022-08-05T04:03:17Z

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

moznion · 2022-09-20T06:20:36Z

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.

RageLtMan added 3 commits July 28, 2022 08:35

Initial Scaffold for EAP Messages

c307bcb

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

Enhance EAP processing

7a08fe0

Write basic field tests for EAP elements, implement a to_bytes() method, misc touchups. Testing: EAP parsing tests pass

moznion reviewed Jul 31, 2022

View reviewed changes

moznion added the enhancement New feature or request label Jul 31, 2022

sempervictus force-pushed the feature/eap_authentication branch from b57ed86 to 10bf7fc Compare August 3, 2022 10:30

sempervictus force-pushed the feature/eap_authentication branch 2 times, most recently from 947e8c7 to 93c29ee Compare August 3, 2022 20:39

sempervictus force-pushed the feature/eap_authentication branch from 93c29ee to ae0382b Compare August 4, 2022 00:02

Incremental Message-Authenticator improvements

93d80f6

Small amount of cleanup and progress on responding to an Identity Access-Request. Generating bad Message-Authenticator values in for_response, likely not grokking the requisite field value correctly

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial Scaffold for EAP Messages #34

Initial Scaffold for EAP Messages #34

sempervictus commented Jul 28, 2022 •

edited

Loading

sempervictus commented Jul 29, 2022

sempervictus commented Jul 30, 2022

moznion commented Jul 31, 2022

moznion left a comment •

edited

Loading

moznion Jul 31, 2022 •

edited

Loading

sempervictus Jul 31, 2022

moznion Jul 31, 2022

sempervictus Jul 31, 2022

moznion Jul 31, 2022

sempervictus Jul 31, 2022

moznion Jul 31, 2022

sempervictus Jul 31, 2022

moznion Jul 31, 2022

sempervictus Jul 31, 2022

moznion commented Jul 31, 2022

moznion commented Jul 31, 2022 •

edited

Loading

sempervictus commented Jul 31, 2022 •

edited

Loading

sempervictus commented Jul 31, 2022 •

edited

Loading

sempervictus commented Aug 2, 2022

sempervictus commented Aug 3, 2022

sempervictus commented Aug 3, 2022 •

edited

Loading

sempervictus commented Aug 4, 2022

moznion commented Aug 4, 2022

sempervictus commented Aug 5, 2022

moznion commented Aug 5, 2022

moznion commented Sep 20, 2022

Initial Scaffold for EAP Messages #34

Are you sure you want to change the base?

Initial Scaffold for EAP Messages #34

Conversation

sempervictus commented Jul 28, 2022 • edited Loading

sempervictus commented Jul 29, 2022

sempervictus commented Jul 30, 2022

moznion commented Jul 31, 2022

moznion left a comment • edited Loading

Choose a reason for hiding this comment

moznion Jul 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moznion commented Jul 31, 2022

moznion commented Jul 31, 2022 • edited Loading

sempervictus commented Jul 31, 2022 • edited Loading

sempervictus commented Jul 31, 2022 • edited Loading

sempervictus commented Aug 2, 2022

sempervictus commented Aug 3, 2022

sempervictus commented Aug 3, 2022 • edited Loading

sempervictus commented Aug 4, 2022

moznion commented Aug 4, 2022

sempervictus commented Aug 5, 2022

moznion commented Aug 5, 2022

moznion commented Sep 20, 2022

sempervictus commented Jul 28, 2022 •

edited

Loading

moznion left a comment •

edited

Loading

moznion Jul 31, 2022 •

edited

Loading

moznion commented Jul 31, 2022 •

edited

Loading

sempervictus commented Jul 31, 2022 •

edited

Loading

sempervictus commented Jul 31, 2022 •

edited

Loading

sempervictus commented Aug 3, 2022 •

edited

Loading