all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer] fetch-answer: encode unsafe characters in partition label
@ 2024-11-19 14:59 Filip Schauer
  2024-11-19 15:40 ` Christoph Heiss
  0 siblings, 1 reply; 3+ messages in thread
From: Filip Schauer @ 2024-11-19 14:59 UTC (permalink / raw)
  To: pve-devel

Ensure potentially unsafe characters in the partition label are encoded,
preventing the installer from failing to find the answer partition when
the label contains whitespaces or !"$%&'()*,/;<>?[\]^`{|}~

The encoding is done according to `blkid_encode_string` [0] in the blkid
util, which is used by `/lib/udev/rules.d/60-persistent-storage.rules`
to create a symlink under `/dev/disk/by-label/`.

For example: "ANSWER PART" is encoded to "ANSWER\x20PART"

[0] https://github.com/util-linux/util-linux/blob/master/libblkid/src/encode.c

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 .../src/fetch_plugins/partition.rs            | 30 ++++++++++++++-----
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index 131f422..0563b2d 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -41,6 +41,19 @@ fn path_exists_logged(file_name: &str, search_path: &str) -> Option<PathBuf> {
     }
 }
 
+fn encode_partlabel(input: &str) -> String {
+    input
+        .chars()
+        .map(|c| {
+            if (' '..='~').contains(&c) && !(c.is_ascii_alphanumeric() || "#+-.:=@_".contains(c)) {
+                format!("\\x{:02x}", c as u32)
+            } else {
+                c.to_string()
+            }
+        })
+        .collect()
+}
+
 /// Searches for the exact case, upper and finally lower case existence of the partlabel in the
 /// search_path, in that order.
 ///
@@ -52,20 +65,21 @@ fn path_exists_logged(file_name: &str, search_path: &str) -> Option<PathBuf> {
 /// * `partlabel_source` - Partition Label, used for matching, in the exact, upper and lower case
 /// * `search_path` - Path where to search for the partition label
 fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
-    if let Some(path) = path_exists_logged(partlabel, search_path) {
-        info!("Found partition with label '{partlabel}'");
+    let partlabel_enc = encode_partlabel(partlabel);
+    if let Some(path) = path_exists_logged(&partlabel_enc, search_path) {
+        info!("Found partition with label '{partlabel_enc}'");
         return Ok(path);
     }
 
-    let partlabel_upper_case = partlabel.to_uppercase();
-    if let Some(path) = path_exists_logged(&partlabel_upper_case, search_path) {
-        info!("Found partition with label '{partlabel_upper_case}'");
+    let partlabel_upper_case_enc = encode_partlabel(&partlabel.to_uppercase());
+    if let Some(path) = path_exists_logged(&partlabel_upper_case_enc, search_path) {
+        info!("Found partition with label '{partlabel_upper_case_enc}'");
         return Ok(path);
     }
 
-    let partlabel_lower_case = partlabel.to_lowercase();
-    if let Some(path) = path_exists_logged(&partlabel_lower_case, search_path) {
-        info!("Found partition with label '{partlabel_lower_case}'");
+    let partlabel_lower_case_enc = encode_partlabel(&partlabel.to_lowercase());
+    if let Some(path) = path_exists_logged(&partlabel_lower_case_enc, search_path) {
+        info!("Found partition with label '{partlabel_lower_case_enc}'");
         return Ok(path);
     }
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH installer] fetch-answer: encode unsafe characters in partition label
  2024-11-19 14:59 [pve-devel] [PATCH installer] fetch-answer: encode unsafe characters in partition label Filip Schauer
@ 2024-11-19 15:40 ` Christoph Heiss
  2024-11-19 15:58   ` Filip Schauer
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Heiss @ 2024-11-19 15:40 UTC (permalink / raw)
  To: Filip Schauer; +Cc: Proxmox VE development discussion

Good catch & thanks for tackeling this!

Also tested it with a simple whitespace character.

Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>

On Tue, Nov 19, 2024 at 03:59:34PM +0100, Filip Schauer wrote:
> Ensure potentially unsafe characters in the partition label are encoded,
> preventing the installer from failing to find the answer partition when
> the label contains whitespaces or !"$%&'()*,/;<>?[\]^`{|}~
> [..]
> ---
>  .../src/fetch_plugins/partition.rs            | 30 ++++++++++++++-----
>  1 file changed, 22 insertions(+), 8 deletions(-)
> [..]
> @@ -52,20 +65,21 @@ fn path_exists_logged(file_name: &str, search_path: &str) -> Option<PathBuf> {
>  /// * `partlabel_source` - Partition Label, used for matching, in the exact, upper and lower case
>  /// * `search_path` - Path where to search for the partition label
>  fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
> -    if let Some(path) = path_exists_logged(partlabel, search_path) {
> -        info!("Found partition with label '{partlabel}'");
> +    let partlabel_enc = encode_partlabel(partlabel);
> +    if let Some(path) = path_exists_logged(&partlabel_enc, search_path) {
> +        info!("Found partition with label '{partlabel_enc}'");

I think the original, non-encoded partition label should be printed
here.

As it would be somewhat surprising to users/administrators to suddenly
see some partition label being searched that what the specified -- thus
considering this as an internal implementation detail, not public
interface.

>          return Ok(path);
>      }
>
> -    let partlabel_upper_case = partlabel.to_uppercase();
> -    if let Some(path) = path_exists_logged(&partlabel_upper_case, search_path) {
> -        info!("Found partition with label '{partlabel_upper_case}'");
> +    let partlabel_upper_case_enc = encode_partlabel(&partlabel.to_uppercase());
> +    if let Some(path) = path_exists_logged(&partlabel_upper_case_enc, search_path) {
> +        info!("Found partition with label '{partlabel_upper_case_enc}'");

.. same here ..

>          return Ok(path);
>      }
>
> -    let partlabel_lower_case = partlabel.to_lowercase();
> -    if let Some(path) = path_exists_logged(&partlabel_lower_case, search_path) {
> -        info!("Found partition with label '{partlabel_lower_case}'");
> +    let partlabel_lower_case_enc = encode_partlabel(&partlabel.to_lowercase());
> +    if let Some(path) = path_exists_logged(&partlabel_lower_case_enc, search_path) {
> +        info!("Found partition with label '{partlabel_lower_case_enc}'");

.. and here.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH installer] fetch-answer: encode unsafe characters in partition label
  2024-11-19 15:40 ` Christoph Heiss
@ 2024-11-19 15:58   ` Filip Schauer
  0 siblings, 0 replies; 3+ messages in thread
From: Filip Schauer @ 2024-11-19 15:58 UTC (permalink / raw)
  To: Christoph Heiss; +Cc: Proxmox VE development discussion

On 19/11/2024 16:40, Christoph Heiss wrote:
> I think the original, non-encoded partition label should be printed
> here.
>
> As it would be somewhat surprising to users/administrators to suddenly
> see some partition label being searched that what the specified -- thus
> considering this as an internal implementation detail, not public
> interface.

done

Superseded by:
https://lists.proxmox.com/pipermail/pve-devel/2024-November/066845.html



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-11-19 15:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-19 14:59 [pve-devel] [PATCH installer] fetch-answer: encode unsafe characters in partition label Filip Schauer
2024-11-19 15:40 ` Christoph Heiss
2024-11-19 15:58   ` Filip Schauer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal