diff mbox

[netctl] wifi-menu: Support UTF-8 encoded SSIDs (FS#45384)

Message ID 20180623112535.GA2331@Mindship-03
State New
Headers show

Commit Message

Emil Velikov via arch-projects June 23, 2018, 11:25 a.m. UTC 
Strictly speaking, we should check with the SSIDEncoding value sent out
by the station, as specified in the 2012 version of 802.11 (page 566),
but wpa_supplicant does not (yet) expose this information and stations
may not set the field to UTF-8 and still encode their SSID accordingly.

We therefore assume SSIDs to be UTF-8 encoded (the only real alternative
is treating SSIDs as raw byte string) in wifi-menu. Note that wifi-menu
is only provided as a convenience and many/most use cases involve
writing profile files manually.

When the current character map is not set to UTF-8 , wifi-menu will only
use ASCII characters in its screen output.
This can be forced using for example `LC_CTYPE=C wifi-menu`.
---
 src/wifi-menu | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

Comments

Emil Velikov via arch-projects June 26, 2018, 10:17 a.m. UTC | #1 
Hi Jouke,

Thanks for fixing this!
Sharing a couple of ideas that come to mind.

On 23 June 2018 at 12:25, Jouke Witteveen via arch-projects
<arch-projects@archlinux.org> wrote:
> Strictly speaking, we should check with the SSIDEncoding value sent out
> by the station, as specified in the 2012 version of 802.11 (page 566),
> but wpa_supplicant does not (yet) expose this information and stations
> may not set the field to UTF-8 and still encode their SSID accordingly.
>
I wonder if iwd (meant to be wpa_supplicant replacement) could help here.
Be that by exposing SSIDEncoding or setting it, as it detects UTF-8 in the SSID.


> @@ -38,6 +43,9 @@ init_profiles() {
>              unset Interface ESSID
>              source "$PROFILE_DIR/$profile" > /dev/null
>              if [[ "$Interface" = "$1" && -n "$ESSID" ]]; then
> +                if [[ "$ESSID" = \"\"*\" ]]; then
> +                    ESSID=${ESSID:2:-1}
> +                fi
FYI, I will send out a small patch which simplifying init_profiles.
It will cause a trivial conflict so I can rebase it on top of this
patch, if you prefer.

> @@ -221,6 +237,8 @@ ensure_root "$(basename "$0")"
>  if ! type dialog &> /dev/null; then
>      exit_error "Please install 'dialog' to use wifi-menu"
>  fi
> +CHARMAP=$(locale charmap)
> +cd /  # We do not want to spawn anything that can block unmounting
>
Was the "cd /" line moved by mistake?

Other than the cd nitpick, the patch looks great. FWIW
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>

HTH
Emil
Emil Velikov via arch-projects June 26, 2018, 10:34 a.m. UTC | #2 
On Tue, Jun 26, 2018 at 12:17 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 23 June 2018 at 12:25, Jouke Witteveen via arch-projects
> <arch-projects@archlinux.org> wrote:
> > Strictly speaking, we should check with the SSIDEncoding value sent out
> > by the station, as specified in the 2012 version of 802.11 (page 566),
> > but wpa_supplicant does not (yet) expose this information and stations
> > may not set the field to UTF-8 and still encode their SSID accordingly.
> >
> I wonder if iwd (meant to be wpa_supplicant replacement) could help here.
> Be that by exposing SSIDEncoding or setting it, as it detects UTF-8 in the SSID.

As far as I can tell, only developers of iwd think of iwd as a
wpa_supplicant replacement. Currently, it is lacking a CLI and
documentation. Moreover, wpa_supplicant is tried and tested. While I
try to not tie netctl in too strongly with any specific client,
wpa_supplicant will likely be the client of choice for netctl for the
foreseeable future.

I submitted a patch to wpa_supplicant for exposing the UTF-8 SSID bit:
http://lists.infradead.org/pipermail/hostap/2018-June/038658.html
However, experimentation showed that it is not really used (yet?).

>
> > @@ -38,6 +43,9 @@ init_profiles() {
> >              unset Interface ESSID
> >              source "$PROFILE_DIR/$profile" > /dev/null
> >              if [[ "$Interface" = "$1" && -n "$ESSID" ]]; then
> > +                if [[ "$ESSID" = \"\"*\" ]]; then
> > +                    ESSID=${ESSID:2:-1}
> > +                fi
> FYI, I will send out a small patch which simplifying init_profiles.
> It will cause a trivial conflict so I can rebase it on top of this
> patch, if you prefer.

Note that the above code is more about matching multiple encodings of
ESSIDs than about the UTF-8 conversion per se.

Thanks for being so quick with your patch!

>
> > @@ -221,6 +237,8 @@ ensure_root "$(basename "$0")"
> >  if ! type dialog &> /dev/null; then
> >      exit_error "Please install 'dialog' to use wifi-menu"
> >  fi
> > +CHARMAP=$(locale charmap)
> > +cd /  # We do not want to spawn anything that can block unmounting
> >
> Was the "cd /" line moved by mistake?

No. It was moved because it fits better (semantically) in this section
of the code.

> Other than the cd nitpick, the patch looks great. FWIW
> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
>

I'll reply to your patch in the other mail.

Regards,
- Jouke
Emil Velikov via arch-projects June 26, 2018, 11:01 a.m. UTC | #3 
On 26 June 2018 at 11:34, Jouke Witteveen <j.witteveen@gmail.com> wrote:
> On Tue, Jun 26, 2018 at 12:17 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> On 23 June 2018 at 12:25, Jouke Witteveen via arch-projects
>> <arch-projects@archlinux.org> wrote:
>> > Strictly speaking, we should check with the SSIDEncoding value sent out
>> > by the station, as specified in the 2012 version of 802.11 (page 566),
>> > but wpa_supplicant does not (yet) expose this information and stations
>> > may not set the field to UTF-8 and still encode their SSID accordingly.
>> >
>> I wonder if iwd (meant to be wpa_supplicant replacement) could help here.
>> Be that by exposing SSIDEncoding or setting it, as it detects UTF-8 in the SSID.
>
> As far as I can tell, only developers of iwd think of iwd as a
> wpa_supplicant replacement. Currently, it is lacking a CLI and
> documentation. Moreover, wpa_supplicant is tried and tested. While I
> try to not tie netctl in too strongly with any specific client,
> wpa_supplicant will likely be the client of choice for netctl for the
> foreseeable future.
>
I've noticed the iwctl and iwmon utils provided, although did not play
around with them.
You're on point though, sticking with wpa_supplicant for the
foreseeable future is fine.

> I submitted a patch to wpa_supplicant for exposing the UTF-8 SSID bit:
> http://lists.infradead.org/pipermail/hostap/2018-June/038658.html
> However, experimentation showed that it is not really used (yet?).
>
Vendors may start using it... one day

Thanks again for the comprehensive answers!
Emil
diff mbox

Patch

diff --git a/src/wifi-menu b/src/wifi-menu
index 42c4c53..525830f 100755
--- a/src/wifi-menu
+++ b/src/wifi-menu
@@ -20,6 +20,11 @@  Arguments:
 END
 }
 
+# Undo printf escaping in $1
+printf_decode() {
+    printf -- "${1//%/%%}"
+}
+
 # Prepare $1 for use in a special quoting context
 quote_safe() {
     if [[ "$1" = \"* ]]; then
@@ -38,6 +43,9 @@  init_profiles() {
             unset Interface ESSID
             source "$PROFILE_DIR/$profile" > /dev/null
             if [[ "$Interface" = "$1" && -n "$ESSID" ]]; then
+                if [[ "$ESSID" = \"\"*\" ]]; then
+                    ESSID=${ESSID:2:-1}
+                fi
                 printf "%s" "$ESSID"
                 if [[ "$Description" =~ "Automatically generated" ]]; then
                     return 2
@@ -62,14 +70,19 @@  init_profiles() {
 
 # Build ENTRIES as an argument list for dialog based on scan results in $1
 init_entries() {
-    local i=0 sep=$'\t' flags signal ssid
+    local i=0 sep=$'\t' decoded flags signal ssid
     while IFS=$'\t' read -r signal flags ssid; do
+        decoded=$(printf_decode "$ssid")
         ENTRIES[i++]="--"  # the SSID might look like an option to dialog
-        ENTRIES[i++]=$ssid
-        if is_yes "${CONNECTED:-no}" && [[ "$ssid" = "$CONNECTION" ]]; then
+        if [[ "$CHARMAP" = "UTF-8" ]]; then
+            ENTRIES[i++]=$decoded
+        else
+            ENTRIES[i++]=$ssid
+        fi
+        if is_yes "${CONNECTED:-no}" && [[ "$decoded" = "$CONNECTION" ]]; then
             ENTRIES[i]="*"  # Currently connected
-        elif in_array "$ssid" "${ESSIDS[@]}"; then
-            if in_array "$(ssid_to_profile "$ssid")" "${GENERATED[@]}"; then
+        elif in_array "$decoded" "${ESSIDS[@]}"; then
+            if in_array "$(ssid_to_profile "$decoded")" "${GENERATED[@]}"; then
                 ENTRIES[i]="."  # Automatically generated
             else
                 ENTRIES[i]=":"  # Handmade
@@ -115,11 +128,14 @@  Do you want to overwrite it?"
 
 # Create a profile for ssid $1
 create_profile() {
-    local box flags key msg security
-    PROFILE="$INTERFACE-${1//\//_}"
+    local box flags key msg security signal ssid
+    PROFILE=$(iconv -c -f UTF-8 -t //TRANSLIT <<< "$1")
+    PROFILE="$INTERFACE-${PROFILE//[?\/]/_}"
     [[ -e "$PROFILE_DIR/$PROFILE" ]] && PROFILE+=".wifi-menu"
     confirm_profile || return $?
-    flags=$(grep -m 1 $'\t'"$1\$" "$NETWORKS" | cut -f 2)
+    while IFS=$'\t' read -r signal flags ssid; do
+        [[ "$(printf_decode "$ssid")" != "$1" ]] || break
+    done < "$NETWORKS"
     if [[ "$flags" =~ WPA|WEP ]]; then
         security=${BASH_REMATCH[0],,}
     else
@@ -221,6 +237,8 @@  ensure_root "$(basename "$0")"
 if ! type dialog &> /dev/null; then
     exit_error "Please install 'dialog' to use wifi-menu"
 fi
+CHARMAP=$(locale charmap)
+cd /  # We do not want to spawn anything that can block unmounting
 
 INTERFACE=$1
 if [[ -z "$INTERFACE" ]]; then
@@ -237,7 +255,6 @@  elif ! is_interface "$INTERFACE"; then
 fi
 load_interface_config "$INTERFACE"
 
-cd /  # We do not want to spawn anything that can block unmounting
 if [[ "$RFKill" && "$(rf_status "$INTERFACE" "$RFKill")" ]]; then
     if ! rf_enable "$INTERFACE" "$RFKill"; then
         exit_error "Could not unblock transmission on interface '$INTERFACE'"
@@ -247,7 +264,7 @@  fi
 
 echo -n "Scanning for networks... "
 if CONNECTION=$(wpa_call "$INTERFACE" status 2> /dev/null | grep -m 1 "^ssid="); then
-    CONNECTION=${CONNECTION#ssid=}
+    CONNECTION=$(printf_decode "${CONNECTION#ssid=}")
     CONNECTED=yes
 fi
 NETWORKS=$(wpa_supplicant_scan "$INTERFACE" 3,4,5)
@@ -270,6 +287,9 @@  Flags description:
     CHOICE=$(dialog --menu "$MSG" 24 50 12 "${ENTRIES[@]}" --stdout)
     RETURN=$?
     if (( RETURN == 0 )); then
+        if [[ "$CHARMAP" != "UTF-8" ]]; then
+            CHOICE=$(printf_decode "$CHOICE")
+        fi
         connect_to_ssid "$CHOICE"
         RETURN=$?
     fi