[PATCH] Notify clients of PAM error messages

[Matt Johnston]

Hi,

Thanks for the patch. It looks useful, though I plan to
update Dropbear to full PAM handling (with
keyboard-interactive mode) soon, which should handle this
case as well. If I don't get that done I'll put the patch in
the next release.

My one concern is how clients might deal with multiple
banner messages - have you tried with many clients?

Cheers,
Matt

On Thu, May 02, 2013 at 11:44:10AM +0100, Martin Donnelly wrote:
> This patch adds support for relaying PAM_ERROR_MSG to clients. This
> is useful when PAM_ERROR_MSG returns meaningful information, an
> example of this is when pam_nologin is being used and '/etc/nologin'
> contains a message for clients attempting to connect.
> 
> -Martin

> diff -ubNr dropbear-2013.58.orig/auth.h dropbear-2013.58/auth.h
> --- dropbear-2013.58.orig/auth.h	2013-04-18 15:58:14.000000000 +0100
> +++ dropbear-2013.58/auth.h	2013-04-30 17:37:33.317480220 +0100
> @@ -36,6 +36,7 @@
>  void recv_msg_userauth_request();
>  void send_msg_userauth_failure(int partial, int incrfail);
>  void send_msg_userauth_success();
> +void send_msg_userauth_banner(buffer **msg);
>  void svr_auth_password();
>  void svr_auth_pubkey();
>  void svr_auth_pam();
> diff -ubNr dropbear-2013.58.orig/common-session.c dropbear-2013.58/common-session.c
> --- dropbear-2013.58.orig/common-session.c	2013-04-18 15:58:14.000000000 +0100
> +++ dropbear-2013.58/common-session.c	2013-05-01 15:55:20.687368142 +0100
> @@ -122,6 +122,8 @@
>  
>  	ses.allowprivport = 0;
>  
> +	ses.pam_err = NULL;
> +
>  	TRACE(("leave session_init"))
>  }
>  
> diff -ubNr dropbear-2013.58.orig/session.h dropbear-2013.58/session.h
> --- dropbear-2013.58.orig/session.h	2013-04-18 15:58:14.000000000 +0100
> +++ dropbear-2013.58/session.h	2013-05-02 11:32:32.610072021 +0100
> @@ -197,6 +197,8 @@
>  	 * really belong here, but nowhere else fits nicely */
>  	int allowprivport;
>  
> +	/* buffer for storing PAM error messages */
> +	buffer * pam_err;
>  };
>  
>  struct serversession {
> diff -ubNr dropbear-2013.58.orig/svr-auth.c dropbear-2013.58/svr-auth.c
> --- dropbear-2013.58.orig/svr-auth.c	2013-04-18 15:58:14.000000000 +0100
> +++ dropbear-2013.58/svr-auth.c	2013-05-01 17:36:28.249791834 +0100
> @@ -37,7 +37,6 @@
>  
>  static void authclear();
>  static int checkusername(unsigned char *username, unsigned int userlen);
> -static void send_msg_userauth_banner();
>  
>  /* initialise the first time for a session, resetting all parameters */
>  void svr_authinitialise() {
> @@ -82,10 +81,10 @@
>  
>  /* Send a banner message if specified to the client. The client might
>   * ignore this, but possibly serves as a legal "no trespassing" sign */
> -static void send_msg_userauth_banner() {
> +void send_msg_userauth_banner(buffer **banner) {
>  
>  	TRACE(("enter send_msg_userauth_banner"))
> -	if (svr_opts.banner == NULL) {
> +	if ((banner == NULL) || (*banner == NULL)) {
>  		TRACE(("leave send_msg_userauth_banner: banner is NULL"))
>  		return;
>  	}
> @@ -93,13 +92,13 @@
>  	CHECKCLEARTOWRITE();
>  
>  	buf_putbyte(ses.writepayload, SSH_MSG_USERAUTH_BANNER);
> -	buf_putstring(ses.writepayload, buf_getptr(svr_opts.banner,
> -				svr_opts.banner->len), svr_opts.banner->len);
> +	buf_putstring(ses.writepayload, buf_getptr(*banner,
> +				(*banner)->len), (*banner)->len);
>  	buf_putstring(ses.writepayload, "en", 2);
>  
>  	encrypt_packet();
> -	buf_free(svr_opts.banner);
> -	svr_opts.banner = NULL;
> +	buf_free(*banner);
> +	*banner = NULL;
>  
>  	TRACE(("leave send_msg_userauth_banner"))
>  }
> @@ -121,7 +120,7 @@
>  
>  	/* send the banner if it exists, it will only exist once */
>  	if (svr_opts.banner) {
> -		send_msg_userauth_banner();
> +		send_msg_userauth_banner(&svr_opts.banner);
>  	}
>  
>  	
> diff -ubNr dropbear-2013.58.orig/svr-authpam.c dropbear-2013.58/svr-authpam.c
> --- dropbear-2013.58.orig/svr-authpam.c	2013-04-18 15:58:14.000000000 +0100
> +++ dropbear-2013.58/svr-authpam.c	2013-05-01 15:18:38.603729862 +0100
> @@ -142,6 +142,22 @@
>  			(*respp) = resp;
>  			break;
>  
> +		case PAM_ERROR_MSG:
> +
> +			if (msg_len > 0) {
> +				ses.pam_err  = buf_new(msg_len + 4);
> +				buf_setpos(ses.pam_err, 0);
> +				buf_putbytes(ses.pam_err, "\r\n", 2);
> +				buf_putbytes(ses.pam_err, (*msg)->msg, msg_len);
> +				buf_putbytes(ses.pam_err, "\r\n", 2);
> +				buf_setpos(ses.pam_err, 0);
> +
> +				send_msg_userauth_banner(&ses.pam_err);
> +			}
> +
> +			rc = PAM_AUTH_ERR;
> +			break;
> +
>  		default:
>  			TRACE(("Unknown message type"))
>  			rc = PAM_CONV_ERR;