Log successful CHAP authentication

[Giulio-xyz]

Years ago pppd used to log successful authentication, then it lost it. Could you re-add it?

--- ppp/pppd/chap-new.c.orig    2009-05-14 17:54:42.000000000 +0200
+++ ppp/pppd/chap-new.c 2009-05-14 17:56:06.000000000 +0200
@@ -349,6 +349,9 @@
                        ss->flags |= AUTH_FAILED;
                        warn("Peer %q failed CHAP authentication", name);
                }
+               else {
+                       warn("Peer %q CHAP authentication succeeded", name);
+               }
        } else if ((ss->flags & AUTH_DONE) == 0)
                return;

Thanks.

[Neustradamus]

@tisj, @enaess, @jjkeijser: For you ;)

@Giulio-xyz: Here: https://github.com/ppp-project/ppp/blob/f1a34da3b2f5336e4993a729e5ac2130d0e0595a/pppd/chap-new.c#L548

[Giulio-xyz]

I use pppd with pptpd.

In my case, it doesn't hit that spot.

When failing, it hits https://github.com/ppp-project/ppp/blob/f1a34da3b2f5336e4993a729e5ac2130d0e0595a/pppd/chap-new.c#L391 logging the failure and the peer name. When succeeding it doesn't log anything.

In the past (15+ years ago) it used to log the peer name even when successful, see for instance ppp-2.3.11/pppd/chap.c:

  if (cstate->chal_interval != 0)
	    TIMEOUT(ChapRechallenge, cstate, cstate->chal_interval);
	notice("CHAP peer authentication succeeded for %q", rhostname);       <=========== THIS WENT MISSING

   } else {
	error("CHAP peer authentication failed for remote host %q", rhostname);
	cstate->serverstate = CHAPSS_BADAUTH;
	auth_peer_fail(cstate->unit, PPP_CHAP);`

[enaess]

Wouldn't it be better to log authentication success in a more common place independent of which authentication method was used? Should be in auth.c and not in chap-new.c.

Also, typically common criteria specifications require this sort of notifications ...

[paulusmack]

This changed when chap.c got replaced by chap-new.c. There is a message printed in auth_withpeer_success() (auth.c):

notice("%s authentication succeeded", prot);

So it should log "CHAP authentication succeeded" at level LOG_NOTICE. If you're not seeing that at all then check your logging configuration. If the complaint is that the remote hostname isn't included in the message, that would be easy to fix in auth.c.

[Giulio-xyz]

Let's say there are 2 peers A and B.

It seems to me auth.c/auth_withpeer_success() is logged, in A logs, when A authenticates successfully to B.

In this ticket I'm talking about what is logged, in B logs, when A authenticates to B:

• when A fails, in B logs I see https://github.com/ppp-project/ppp/blob/919b28b82f08d7552e2f0febb715ddce6f7208d4/pppd/chap-new.c#L391
• when A succeeds, i B logs I see nothing

This is why I added the successful log close to the already present failed log.

In auth.c there's also a auth_withpeer_fail() with "Failed to authenticate ourselves to peer", but I don't see this in B logs when A fails to authenticate to B; in B I see the log from chap-new.c which includes the A peername.

Thanks

[paulusmack]

OK fair enough. I think the logging should be added in auth_peer_success() rather than chap_handle_response() though.