Skip to content

Commit

Permalink
Revert commit #2bf46d9
Browse files Browse the repository at this point in the history 
Revert [commit \#2bf46d9](2bf46d9), because `OPTIONS+="string_escape=none"` should only affect "*strings used for device naming*" [per **udev** documentation](https://www.freedesktop.org/software/systemd/man/udev.html#string_escape=none%7Creplace), e.g. strings used for setting `NAME` and `SYMLINK`.<br />
Rationale:
  1. The strings *crypto-sdcard* generates and uses in [96-cryptosd.rules](https://github.com/Olf0/crypto-sdcard/blob/master/udev/rules.d/96-cryptosd.rules) for device naming should always be free of escape sequences and other "potentially dangerous" characters.  If some flaw causes this not to be true anymore (e.g., see issue #115), it is fine that *udev* might filter device strings (although this mechanism makes debugging such flaws harder).
  1. It did not alleviate the breakage SFOS 4.0.1. brought (issue #115).  This also might prove that `string_escape=none` is not applicable to evaluating "`%c`" or inserting a string into `ENV{SYSTEMD_WANTS}=""` / `ENV{SYSTEMD_USER_WANTS}=""` statements, because the strings generated by `systemd-escape` for *crypto-sdcard* contain a lot of C-style escape sequences (or they are not considered "dangerous").  Because OTOH manually calling (at the command line) instanciated Systemd units with pre-escaped instance names works (?does it still on SFOS 4.0.1?), it is apparently the call interface proper which filters (or just misinterprets, e.g. in one of its many automatic detections) the unit string (?or just the instance name?) when handing over the content of a `ENV{SYSTEMD_WANTS}=""` / `ENV{SYSTEMD_USER_WANTS}=""` statement.
  • Loading branch information
@Olf0
Olf0 authored Feb 25, 2021
1 parent d95b2a3 commit d435bbc
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions udev/rules.d/96-cryptosd.rules
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# For DM-Crypt LUKS, match sda0 to mmcblk1 to both SUBSYSTEM=="block" and ENV{ID_FS_TYPE}=="crypto_LUKS"
KERNEL=="mmcblk1*|sd[a-z]*", SUBSYSTEM=="block", ENV{ID_FS_TYPE}=="crypto_LUKS", ACTION=="add", OPTIONS+="string_escape=none", SYMLINK+="crypto_luks_%E{ID_FS_UUID}", MODE="0660", TAG+="systemd", ENV{SYSTEMD_USER_WANTS}="", PROGRAM=="/usr/bin/systemd-escape --template=cryptosd-luks@.service crypto_luks_%E{ID_FS_UUID}", ENV{SYSTEMD_WANTS}="%c"
KERNEL=="mmcblk1*|sd[a-z]*", SUBSYSTEM=="block", ENV{ID_FS_TYPE}=="crypto_LUKS", ACTION=="add", SYMLINK+="crypto_luks_%E{ID_FS_UUID}", MODE="0660", TAG+="systemd", ENV{SYSTEMD_USER_WANTS}="", PROGRAM=="/usr/bin/systemd-escape --template=cryptosd-luks@.service crypto_luks_%E{ID_FS_UUID}", ENV{SYSTEMD_WANTS}="%c"

# For DM-Crypt "plain", also match sda0 to mmcblk1 to SUBSYSTEM=="block", but ensure (by ENV{ID_*}!= statements) that it appears to be unused space
# Two rules, one for partitions and a tighter one for whole disks:
KERNEL=="mmcblk1*|sd[a-z]*", SUBSYSTEM=="block", ENV{DEVTYPE}=="disk", ENV{ID_FS_USAGE}!="?*", ENV{ID_FS_TYPE}!="?*", ENV{ID_PART_TABLE_TYPE}!="?*", ACTION=="add", OPTIONS+="string_escape=none", SYMLINK+="crypto_plain_%k", MODE="0660", TAG+="systemd", ENV{SYSTEMD_USER_WANTS}="", PROGRAM=="/usr/bin/systemd-escape --template=cryptosd-plain@.service crypto_plain_%k", ENV{SYSTEMD_WANTS}="%c"
KERNEL=="mmcblk1*|sd[a-z]*", SUBSYSTEM=="block", ENV{DEVTYPE}=="partition", ENV{ID_FS_USAGE}!="?*", ENV{ID_FS_TYPE}!="?*", ACTION=="add", OPTIONS+="string_escape=none", SYMLINK+="crypto_plain_%k", MODE="0660", TAG+="systemd", ENV{SYSTEMD_USER_WANTS}="", PROGRAM=="/usr/bin/systemd-escape --template=cryptosd-plain@.service crypto_plain_%k", ENV{SYSTEMD_WANTS}="%c"
KERNEL=="mmcblk1*|sd[a-z]*", SUBSYSTEM=="block", ENV{DEVTYPE}=="disk", ENV{ID_FS_USAGE}!="?*", ENV{ID_FS_TYPE}!="?*", ENV{ID_PART_TABLE_TYPE}!="?*", ACTION=="add", SYMLINK+="crypto_plain_%k", MODE="0660", TAG+="systemd", ENV{SYSTEMD_USER_WANTS}="", PROGRAM=="/usr/bin/systemd-escape --template=cryptosd-plain@.service crypto_plain_%k", ENV{SYSTEMD_WANTS}="%c"
KERNEL=="mmcblk1*|sd[a-z]*", SUBSYSTEM=="block", ENV{DEVTYPE}=="partition", ENV{ID_FS_USAGE}!="?*", ENV{ID_FS_TYPE}!="?*", ACTION=="add", SYMLINK+="crypto_plain_%k", MODE="0660", TAG+="systemd", ENV{SYSTEMD_USER_WANTS}="", PROGRAM=="/usr/bin/systemd-escape --template=cryptosd-plain@.service crypto_plain_%k", ENV{SYSTEMD_WANTS}="%c"

# Carefully match resulting virtual node dm-* to trigger mounting it; see /lib/udev/rules.d/10-dm.rules for details
KERNEL=="dm-[0-9]*", SUBSYSTEM=="block", SYMLINK=="mapper/crypto_luks_*", ENV{ID_FS_USAGE}=="filesystem", ENV{DM_UDEV_RULES_VSN}=="[1-9]*", ACTION=="change", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="0", OPTIONS+="string_escape=none", GROUP="disk", MODE="0660", TAG+="systemd", PROGRAM=="/usr/bin/systemd-escape --template=mount-cryptosd-luks@.service %E{DM_NAME}", ENV{SYSTEMD_WANTS}="%c"
KERNEL=="dm-[0-9]*", SUBSYSTEM=="block", SYMLINK=="mapper/crypto_luks_*", ENV{ID_FS_USAGE}=="filesystem", ENV{DM_UDEV_RULES_VSN}=="[1-9]*", ACTION=="change", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="0", GROUP="disk", MODE="0660", TAG+="systemd", PROGRAM=="/usr/bin/systemd-escape --template=mount-cryptosd-luks@.service %E{DM_NAME}", ENV{SYSTEMD_WANTS}="%c"

# Ditto for DM-Crypt "plain":
KERNEL=="dm-[0-9]*", SUBSYSTEM=="block", SYMLINK=="mapper/crypto_plain_*", ENV{ID_FS_USAGE}=="filesystem", ENV{DM_UDEV_RULES_VSN}=="[1-9]*", ACTION=="change", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="0", OPTIONS+="string_escape=none", GROUP="disk", MODE="0660", TAG+="systemd", PROGRAM=="/usr/bin/systemd-escape --template=mount-cryptosd-plain@.service %E{DM_NAME}", ENV{SYSTEMD_WANTS}="%c"
KERNEL=="dm-[0-9]*", SUBSYSTEM=="block", SYMLINK=="mapper/crypto_plain_*", ENV{ID_FS_USAGE}=="filesystem", ENV{DM_UDEV_RULES_VSN}=="[1-9]*", ACTION=="change", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="0", GROUP="disk", MODE="0660", TAG+="systemd", PROGRAM=="/usr/bin/systemd-escape --template=mount-cryptosd-plain@.service %E{DM_NAME}", ENV{SYSTEMD_WANTS}="%c"

1 comment on commit d435bbc

@Olf0
Copy link
Owner Author

@Olf0 Olf0 commented on d435bbc Feb 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert commit #2bf46d9

Revert commit #2bf46d9, because OPTIONS+="string_escape=none" should only affect "strings used for device naming" as per udev documentation, e.g. strings used for setting NAME and SYMLINK.

Rationale

  1. The strings crypto-sdcard generates and uses in 96-cryptosd.rules for device naming should always be free of escape sequences and other "potentially dangerous" characters. If some flaw causes this not to be true anymore (e.g., see issue fails on SailfishOS 4.0 #115), it is fine that udev might filter device strings (although this mechanism makes debugging such flaws harder).
  2. It did not alleviate the breakage SFOS 4.0.1. brought (issue fails on SailfishOS 4.0 #115). This also might prove that string_escape=none is not applicable to evaluating "%c" or inserting a string into ENV{SYSTEMD_WANTS}="" / ENV{SYSTEMD_USER_WANTS}="" statements, because the strings generated by systemd-escape for crypto-sdcard contain a lot of C-style escape sequences (or they are not considered "dangerous"). Because OTOH manually calling (at the command line) instanciated Systemd units with pre-escaped instance names works (?does it still on SFOS 4.0.1?), it is apparently the call interface proper which filters (or just misinterprets, e.g. in one of its many automatic detections) the unit string (?or just the instance name?) when handing over the content of a ENV{SYSTEMD_WANTS}="" / ENV{SYSTEMD_USER_WANTS}="" statement.
    Edit / Explanation for point 2:
    Systemd explicitly calls its function extract_first_word with the attribute EXTRACT_UNQUOTE, hence the string handed over per ENV{SYSTEMD_*WANTS}="" from udev to Systemd (and then the separated instance string to the called unit) can be hard-quoted (per "'"), becomes unquoted by Systemd (then still including the unit name) and should ultimately arrive at the called unit (the instance string as "%i") as it was originally inserted into PROGRAM="".
    In somebody else's words.

Please sign in to comment.