ppp-project / ppp

Paul's PPP Package: PPP daemon and associated utilities | Official GitHub repo: https://github.com/ppp-project/ppp
https://github.com/ppp-project/ppp
Other
384 stars 231 forks source link

EAP-TLS: When failing initialization, sending NAK suggesting itself #210

Closed enaess closed 3 years ago

enaess commented 3 years ago

1) The server (Win2K19) responds with EAP-TLS 2) PPPd runs without EAP-TLS configuration, and thus fails initialization 3) Responds with NAK: Use EAP-TLS 4) Server terminates the connection ..

At this point, we'd expect the client to send a NAK with any of the other EAP mechanisms it supports, e.g. EAPT_MSCHAPv2. However, it never gets this far as the server terminates the connection.

Output: Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP ConfReq id=0x1 <asyncmap 0x0> <magic 0x9b3ac54b> ] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP ConfReq id=0x0 <mru 4091> <magic 0x3f174a62> ] Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP ConfAck id=0x0 <mru 4091> <magic 0x3f174a62> ] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP ConfAck id=0x1 <asyncmap 0x0> <magic 0x9b3ac54b> ] Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP EchoReq id=0x0 magic=0x9b3ac54b] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [EAP Request id=0x0 Identity ] Jan 1 11:44:46 optiplex pppd[407928]: sent [EAP Response id=0x0 Identity <Name "SSTP-TEST\test">] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP EchoRep id=0x0 magic=0x3f174a62] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [EAP Request id=0x1 TLS --S] Jan 1 11:44:46 optiplex pppd[407928]: MTU = 1486 Jan 1 11:44:46 optiplex pppd[407928]: calling get_eaptls_secret Jan 1 11:44:46 optiplex pppd[407928]: Can't open eap-tls secret file /etc/ppp/eaptls-client: No such file or directory Jan 1 11:44:46 optiplex pppd[407928]: EAP-TLS: Cannot get secret/password for client "SSTP-TEST\test", server "sstp-test" Jan 1 11:44:46 optiplex pppd[407928]: cannot init ssl Jan 1 11:44:46 optiplex pppd[407928]: sent [EAP Response id=0x1 Nak <Suggested-type 0d (TLS)>] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [EAP Failure id=0x1] Jan 1 11:44:46 optiplex pppd[407928]: EAP: peer reports authentication failure Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP TermReq id=0x2 "Failed to authenticate ourselves to peer"] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP TermReq id=0x3 "?\027Jb\000<\315t\000\000\003,"] Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP TermAck id=0x3] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP TermAck id=0x2 "Failed to authenticate ourselves to peer"]

A small fix is to insert the following:

diff --git a/pppd/eap.c b/pppd/eap.c
index 2e0905b..8e4fd1d 100644
--- a/pppd/eap.c
+++ b/pppd/eap.c
@@ -1894,7 +1894,7 @@ eap_request(eap_state *esp, u_char *inp, int id, int len)
                                /* Init ssl session */
                                if(!eaptls_init_ssl_client(esp)) {
                                        dbglog("cannot init ssl");
-                                       eap_send_nak(esp, id, EAPT_TLS);
+                                       eap_send_nak(esp, id, EAPT_MSCHAPV2);
                                        esp->es_client.ea_using_eaptls = 0;
                                        break;
                                }
@@ -1906,7 +1906,7 @@ eap_request(eap_state *esp, u_char *inp, int id, int len)
                        }

                        /* The server has sent a bad start packet. */
-                       eap_send_nak(esp, id, EAPT_TLS);
+                       eap_send_nak(esp, id, EAPT_MSCHAPV2);
                        break;

                case eapTlsRecvAck:

However, this isn't correct either. We need to figure out which other mechanisms are supported and try them, e.g. EAP-TLS, EAP-MSCHAPv2, EAP-SRP, etc ...

@jjkeijser ??

enaess commented 3 years ago

@Neustradamus - I added this "fix" as a part of #211 pull request. The EAP code is littered with eap_send_nak() suggesting EAP_SRP, but it may not be enabled / supported. The handling of which protocol to nak with needs an improvement. Seems like a more exhaustive fix is necessary.

Neustradamus commented 3 years ago

@enaess: Ok but please edit your issue here, it is deformed, difficult to read.

Look the <> "Insert code" function.

enaess commented 3 years ago

Looking into RFC3748 (EAP), section 5.3 describes the NAK message and it can hold one or more alternative authentication methods that the peer supports. Should have some kind of state mechanism that accounts for which ones we have tried, and then remove them as they fail, no?

paulusmack commented 3 years ago

I'd like to hear @carlsonj 's opinion...

carlsonj commented 3 years ago

Withdrawing ones that've failed sounds fair to me. All that's necessarily required, though, is that you nak with one or more methods that you (locally) expect may work, and then give up and disconnect when too many naks occur. It's not expected that a large number of alternatives will be viable and that we'll be sequencing through them. That'd be a bad configuration.

enaess commented 3 years ago

I can see if I can give it a whirl for the client side, and if necessary remove the options we have tried. Not sure about the server side.

enaess commented 3 years ago

Looks like the Microsoft server will only take the first method suggested and not selectively pick one that it thinks will work. Consider the following patch, the server will reject the nak with suggested type EAPT_CHAPMD5 and terminate the connection. Somehow ranking these may be necessary.

diff --git a/pppd/eap.c b/pppd/eap.c
index 0cffa8b..e4e2812 100644
--- a/pppd/eap.c
+++ b/pppd/eap.c
@@ -1559,10 +1559,28 @@ eap_send_nak(eap_state *esp, u_char id, u_char type)
        PUTCHAR(EAP_RESPONSE, outp);
        PUTCHAR(id, outp);
        esp->es_client.ea_id = id;
-       msglen = EAP_HEADERLEN + 2 * sizeof (u_char);
+       msglen = EAP_HEADERLEN + 2;
+       u_char *lenp = outp;
        PUTSHORT(msglen, outp);
        PUTCHAR(EAPT_NAK, outp);
-       PUTCHAR(type, outp);
+       PUTCHAR(EAPT_MD5CHAP, outp); // RFC3748: All implementations must support type 1-4
+
+#ifdef USE_EAPTLS
+       PUTCHAR(EAPT_TLS, outp);
+       msglen += 1;
+#endif
+
+#ifdef USE_SRP
+       PUTCHAR(EAPT_SRP, outp);
+       msglen += 1;
+#endif
+
+#ifdef CHAPMS
+       PUTCHAR(EAPT_MSCHAPV2, outp);
+       msglen += 1;
+#endif
+
+       PUTSHORT(msglen, lenp);

        output(esp->es_unit, outpacket_buf, PPP_HDRLEN + msglen);
 }
enaess commented 3 years ago

@paulusmack - I gave up on a more elaborate solution, and fixed the one place we were still using it in error. I created pull request #217 to fix that error.

jjkeijser commented 3 years ago

Hi Eivind,

sorry for the delay , but I just started working again (happy New Year, everybody!) and I do have other responsibilities next to this ;)

As for your report/problem, I did read it and have thought about it a little. A proper solution will require more work and coordination, but I propose roughly the following:

This will require some coding and a lot of testing, of course - your thoughts?

On 01/01/21 20:58, Eivind Næss wrote:

  1. The server (Win2K19) responds with EAP-TLS
  2. PPPd runs without EAP-TLS configuration, and thus fails initialization
  3. Responds with NAK: Use EAP-TLS
  4. Server terminates the connection ..

At this point, we'd expect the client to send a NAK with any of the other EAP mechanisms it supports, e.g. EAPT_MSCHAPv2. However, it never gets this far as the server terminates the connection.

Output: Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP ConfReq id=0x1 <asyncmap 0x0> <magic 0x9b3ac54b> ] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP ConfReq id=0x0 <mru 4091> <magic 0x3f174a62> ] Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP ConfAck id=0x0 <mru 4091> <magic 0x3f174a62> ] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP ConfAck id=0x1 <asyncmap 0x0> <magic 0x9b3ac54b> ] Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP EchoReq id=0x0 magic=0x9b3ac54b] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [EAP Request id=0x0 Identity ] Jan 1 11:44:46 optiplex pppd[407928]: sent [EAP Response id=0x0 Identity <Name "SSTP-TEST\test">] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP EchoRep id=0x0 magic=0x3f174a62] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [EAP Request id=0x1 TLS --S] Jan 1 11:44:46 optiplex pppd[407928]: MTU = 1486 Jan 1 11:44:46 optiplex pppd[407928]: calling get_eaptls_secret Jan 1 11:44:46 optiplex pppd[407928]: Can't open eap-tls secret file /etc/ppp/eaptls-client: No such file or directory Jan 1 11:44:46 optiplex pppd[407928]: EAP-TLS: Cannot get secret/password for client "SSTP-TEST\test", server "sstp-test" Jan 1 11:44:46 optiplex pppd[407928]: cannot init ssl Jan 1 11:44:46 optiplex pppd[407928]: sent [EAP Response id=0x1 Nak <Suggested-type 0d (TLS)>] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [EAP Failure id=0x1] Jan 1 11:44:46 optiplex pppd[407928]: EAP: peer reports authentication failure Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP TermReq id=0x2 "Failed to authenticate ourselves to peer"] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP TermReq id=0x3 "?\027Jb\000<\315t\000\000\003,"] Jan 1 11:44:46 optiplex pppd[407928]: sent [LCP TermAck id=0x3] Jan 1 11:44:46 optiplex pppd[407928]: rcvd [LCP TermAck id=0x2 "Failed to authenticate ourselves to peer"]

A small fix is to insert the following:

iff --git a/pppd/eap.c b/pppd/eap.c index 2e0905b..8e4fd1d 100644 --- a/pppd/eap.c +++ b/pppd/eap.c @@ -1894,7 +1894,7 @@ eap_request(eap_state esp, u_char /inp, int id, int len) // Init ssl session / if(!eaptls_init_ssl_client(esp)) { dbglog("cannot init ssl");

* |eap_send_nak(esp, id, EAPT_TLS); |

* |eap_send_nak(esp, id, EAPT_MSCHAPV2); esp->es_client.ea_using_eaptls = 0; break; } |

@@ -1906,7 +1906,7 @@ eap_request(eap_state esp, u_char inp, int id, int len) }

|/ The server has sent a bad start packet. / |

* |eap_send_nak(esp, id, EAPT_TLS); |

* |eap_send_nak(esp, id, EAPT_MSCHAPV2); break; case eapTlsRecvAck: |

However, this isn't correct either. We need to figure out which other mechanisms are supported and try them, e.g. EAP-TLS, EAP-MSCHAPv2, EAP-SRP, etc ...

@jjkeijser https://github.com/jjkeijser ??

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/paulusmack/ppp/issues/210, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWR3ZMHJJHDBJFHVZW63RLSXYSMVANCNFSM4VQPXUKQ.

enaess commented 3 years ago

Hi Jan Just,

Happy New Year to you too! And yes, I am actually out on paternity leave until mid-March which means the child is my primary responsibility.

I think your suggestion is a reasonable one. However, as I found out, we can't reply with a NAK with multiple type and have the server select one that the client support. This means we need to "rank" the ones we want to try.

To make it more complicated, some of these mechanisms can be left out of the compile time. And with others in the feature pipeline, e.g. PEAP. I will need to review some of the implementation of wpa_supplicant which seems to be the defac-to standard for many project and how they do EAP. The other part of me feel like the EAP code needs to further be refactored to keep it maintainable.

paulusmack commented 3 years ago

I merged #217, but I agree a more comprehensive fix is needed. I don't want to hold the release for the more comprehensive fix though.

enaess commented 3 years ago

Yes, acknowledged!

Neustradamus commented 3 years ago

@enaess: This issue has been solved?

enaess commented 3 years ago

@Neustradamus I think there might be one more place in the code that needs an update. Let me check on that tomorrow

We should probably address the issue of priority / order in which the types are selected. That would be a different issue though

enaess commented 3 years ago

@Neustradamus Fix #217 took care of the one place missing from #211. You can go ahead and close this issue. If someone needs to be able to configure an priority then let them create a new issue.