Hi.

I think you should reuse option names used by other protocols, we
already have far too much variation and abbreviation styles for common
option names - see my comments inline below.

> This patch adds protocol support for PPP over SSH.  The protocol name is
> 'pppossh' with the following options.
> 
>  - sshserver, required, SSH server name

Should be "server", as used by pptp already.

>  - sshport, SSH server port

Should be just "port" to follow the naming style of the other opts.

>  - sshuser, required, SSH login username

Should be "username" as used by pppoe, 6in4, dhcpv6, pptp, ...

>  - identityfile, required, client private key file.

You could name that just "identity" or "privkey", but no preference here.

>  - localip, local ip address to be assigned.

Should be "ipaddr" as used by static, 6in4 etc.

>  - remoteip, peer ip address to be assigned.

Should be "peeraddr" as used by 6in4 , pptp, 6rd, ...

>  - acceptunknown, accept the connection if the remote host key is
>    unknown.  This option is only avaiable in dropbear client.  OpenSSH
>    client must NOT use it.

No preference here either.


> Because the protocol script file ppp.sh will be called with $HOME set to
> '/', so we use 'env -u HOME' to let dropbear client to get correct HOME
> directory from /etc/passwd file so that it can read '~/known_hosts'
> correctly.
> 
> Signed-off-by: Yousong Zhou <yszhou4t...@gmail.com>
> ---
>  package/network/services/ppp/files/ppp.sh |   51 
> +++++++++++++++++++++++++++++
>  1 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/package/network/services/ppp/files/ppp.sh 
> b/package/network/services/ppp/files/ppp.sh
> index 8824409..1cf4ab0 100755
> --- a/package/network/services/ppp/files/ppp.sh
> +++ b/package/network/services/ppp/files/ppp.sh
> @@ -206,10 +206,61 @@ proto_pptp_teardown() {
>       ppp_generic_teardown "$@"
>  }
>  
> +proto_pppossh_init_config() {
> +     ppp_generic_init_config
> +     proto_config_add_string "sshserver"
> +     proto_config_add_string "sshport"
> +     proto_config_add_string "sshuser"
> +     proto_config_add_string "identityfile"
> +     proto_config_add_string "localip"
> +     proto_config_add_string "remoteip"
> +     proto_config_add_string "acceptunknown"
> +     available=1
> +     no_device=1
> +}
> +
> +proto_pppossh_setup() {
> +     local config="$1"
> +     local iface="$2"
> +     local ip serv_addr
> +     local errmsg
> +
> +     json_get_vars sshport sshuser identityfile localip remoteip 
> acceptunknown
> +     json_get_var sshserver sshserver && {
> +             for ip in $(resolveip -t 5 "$sshserver"); do
> +                     ( proto_add_host_dependency "$config" "$ip" )
> +                     serv_addr=1
> +             done
> +     }
> +     [ -n "$serv_addr" ] || errmsg="${errmsg}Could not resolve $sshserver.\n"
> +     [ -n "$sshuser" ] || errmsg="${errmsg}Missing sshuser option.\n"
> +     [ -f "$identityfile" ] || errmsg="${errmsg}Invalid identityfile 
> option.\n"

Maybe you should probe some well known locations here, like
/root/.ssh/id_rsa, /home/$username/.ssh/id_rsa etc.

> +     [ -n "$errmsg" ] && {
> +             echo -e "$errmsg"
> +             sleep 5
> +             proto_setup_failed "$config"
> +             exit 1
> +     }
> +     sshport=${sshport:+-p \"$sshport\"}
> +     sshhost="$sshuser@$sshserver"
> +     acceptunknown="${acceptunknown:+-y}"
> +     pty="env -u HOME /usr/bin/ssh "$acceptunknown" -i '$identityfile' 
> $sshport '$sshhost'"
> +     pty="$pty pppd nodetach notty noauth"
> +     ippair="$localip:$remoteip"
> +
> +     ppp_generic_setup "$config" \
> +             noauth pty "$pty" "$ippair"
> +}
> +
> +proto_pppossh_teardown() {
> +     ppp_generic_teardown "$@"
> +}
> +
>  [ -n "$INCLUDE_ONLY" ] || {
>       add_protocol ppp
>       [ -f /usr/lib/pppd/*/rp-pppoe.so ] && add_protocol pppoe
>       [ -f /usr/lib/pppd/*/pppoatm.so ] && add_protocol pppoa
>       [ -f /usr/lib/pppd/*/pptp.so ] && add_protocol pptp
> +     [ -x /usr/bin/ssh ] && add_protocol pppossh
>  }
>  
> 


~ Jow

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Reply via email to