| Message ID | 20230927093631.1595494-6-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add EFI HTTP boot support | expand |
On Wed, 27 Sept 2023 at 15:38, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: > This adds the URI device path option for 'boot add' subcommand. > User can add the URI load option for downloading ISO image file > or EFI application through network. Currently HTTP is only supported. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > cmd/efidebug.c | 50 +++++++++++++++++++++++++++++++++++ > include/net.h | 8 ++++++ > net/wget.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 130 insertions(+) > > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > index 0be3af3e76..f2fd6ba71d 100644 > --- a/cmd/efidebug.c > +++ b/cmd/efidebug.c > @@ -19,6 +19,7 @@ > #include <log.h> > #include <malloc.h> > #include <mapmem.h> > +#include <net.h> > #include <part.h> > #include <search.h> > #include <linux/ctype.h> > @@ -829,6 +830,52 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int > flag, > argc -= 1; > argv += 1; > break; > +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && > IS_ENABLED(CONFIG_CMD_DNS)) > + case 'u': > + { > + char *pos; > + int uridp_len; > + struct efi_device_path_uri *uridp; > + > + if (argc < 3 || lo.label) { > + r = CMD_RET_USAGE; > + goto out; > + } > + id = (int)hextoul(argv[1], &endp); > + if (*endp != '\0' || id > 0xffff) > + return CMD_RET_USAGE; > + > + efi_create_indexed_name(var_name16, > sizeof(var_name16), > + "Boot", id); > + > + label = efi_convert_string(argv[2]); > + if (!label) > + return CMD_RET_FAILURE; > + lo.label = label; > + > + uridp_len = sizeof(struct efi_device_path) + > strlen(argv[3]) + 1; > + fp_free = efi_alloc(uridp_len + sizeof(END)); > + uridp = (struct efi_device_path_uri *)fp_free; > + uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; > + uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI; > + uridp->dp.length = uridp_len; > + if (!wget_validate_uri(argv[3])) { > + printf("ERROR: invalid URI\n"); > + r = CMD_RET_FAILURE; > + goto out; > + } > + > + strcpy(uridp->uri, argv[3]); > + pos = (char *)uridp + uridp_len; > + memcpy(pos, &END, sizeof(END)); > + fp_size += uridp_len + sizeof(END); > + file_path = (struct efi_device_path *)uridp; > + argc -= 3; > + argv += 3; > + break; > + } > +#endif > + > default: > r = CMD_RET_USAGE; > goto out; > @@ -1492,6 +1539,9 @@ static char efidebug_help_text[] = > " -b|-B <bootid> <label> <interface> <devnum>[:<part>] <file > path>\n" > " -i|-I <interface> <devnum>[:<part>] <initrd file path>\n" > " (-b, -i for short form device path)\n" > +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && > IS_ENABLED(CONFIG_CMD_DNS)) > + " -u <bootid> <label> <uri>\n" > +#endif > " -s '<optional data>'\n" > "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" > " - delete UEFI BootXXXX variables\n" > diff --git a/include/net.h b/include/net.h > index 57889d8b7a..c748974573 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -935,4 +935,12 @@ static inline void eth_set_enable_bootdevs(bool > enable) {} > */ > int wget_with_dns(ulong dst_addr, char *uri); > > +/** > + * wget_validate_uri() - varidate the uri > + * > + * @uri: uri string of target file of wget > + * Return: true if uri is valid, false if uri is invalid > + */ > +bool wget_validate_uri(char *uri); > + > #endif /* __NET_H__ */ > diff --git a/net/wget.c b/net/wget.c > index 4801e28eb9..6a4d22be32 100644 > --- a/net/wget.c > +++ b/net/wget.c > @@ -558,3 +558,75 @@ out: > return ret; > } > #endif > + > +/** > + * wget_validate_uri() - validate the uri for wget > + * > + * @uri: uri string > + * Return: true on success, false on failure > + */ > It's better to have a comment description in one place. You already have it in the header file. And even here the text is a little bit differ. > +bool wget_validate_uri(char *uri) > +{ > + char c; > + bool ret = true; > + char *str_copy, *s, *authority; > + > + /* TODO: strict uri conformance check */ > + > + /* > + * Uri is expected to be correctly percent encoded. > + * This is the minimum check, control codes(0x1-0x19, 0x7F, except > '\0') > + * and space character(0x20) are not allowed. > + */ > + for (c = 0x1; c < 0x21; c++) { > + if (strchr(uri, c)) { > + printf("invalid character is used\n"); > + return false; > + } > + } > + if (strchr(uri, 0x7f)) { > + printf("invalid character is used\n"); > + return false; > + } > So you validate that only English characters exist in the url. There are urls with non English characters. > + > + /* > + * This follows the current U-Boot wget implementation. > + * scheme: only "http:" is supported > + * authority: > + * - user information: not supported > + * - host: supported > + * - port: not supported(always use the default port) > + */ > + if (strncmp(uri, "http://", 7)) { > + printf("only http:// is supported\n"); > log_err() > + return false; > + } > + str_copy = strdup(uri); > + if (!str_copy) > + return false; > + > + s = str_copy + strlen("http://"); > + authority = strsep(&s, "/"); > + if (!s) { > + printf("invalid uri, no file path\n"); > + ret = false; > + goto out; > + } > + s = strchr(authority, '@'); > + if (s) { > + printf("user information is not supported\n"); > + ret = false; > + goto out; > + } > + s = strchr(authority, ':'); > + if (s) { > + printf("user defined port is not supported\n"); > + ret = false; > + goto out; > + } > + > +out: > + free(str_copy); > + > + return ret; > +} > -- > 2.34.1 > >
On Thu, 28 Sept 2023 at 23:32, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > > > > On Wed, 27 Sept 2023 at 15:38, Masahisa Kojima <masahisa.kojima@linaro.org> wrote: >> >> This adds the URI device path option for 'boot add' subcommand. >> User can add the URI load option for downloading ISO image file >> or EFI application through network. Currently HTTP is only supported. >> >> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> >> --- >> cmd/efidebug.c | 50 +++++++++++++++++++++++++++++++++++ >> include/net.h | 8 ++++++ >> net/wget.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 130 insertions(+) >> >> diff --git a/cmd/efidebug.c b/cmd/efidebug.c >> index 0be3af3e76..f2fd6ba71d 100644 >> --- a/cmd/efidebug.c >> +++ b/cmd/efidebug.c >> @@ -19,6 +19,7 @@ >> #include <log.h> >> #include <malloc.h> >> #include <mapmem.h> >> +#include <net.h> >> #include <part.h> >> #include <search.h> >> #include <linux/ctype.h> >> @@ -829,6 +830,52 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, >> argc -= 1; >> argv += 1; >> break; >> +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS)) >> + case 'u': >> + { >> + char *pos; >> + int uridp_len; >> + struct efi_device_path_uri *uridp; >> + >> + if (argc < 3 || lo.label) { >> + r = CMD_RET_USAGE; >> + goto out; >> + } >> + id = (int)hextoul(argv[1], &endp); >> + if (*endp != '\0' || id > 0xffff) >> + return CMD_RET_USAGE; >> + >> + efi_create_indexed_name(var_name16, sizeof(var_name16), >> + "Boot", id); >> + >> + label = efi_convert_string(argv[2]); >> + if (!label) >> + return CMD_RET_FAILURE; >> + lo.label = label; >> + >> + uridp_len = sizeof(struct efi_device_path) + strlen(argv[3]) + 1; >> + fp_free = efi_alloc(uridp_len + sizeof(END)); >> + uridp = (struct efi_device_path_uri *)fp_free; >> + uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; >> + uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI; >> + uridp->dp.length = uridp_len; >> + if (!wget_validate_uri(argv[3])) { >> + printf("ERROR: invalid URI\n"); >> + r = CMD_RET_FAILURE; >> + goto out; >> + } >> + >> + strcpy(uridp->uri, argv[3]); >> + pos = (char *)uridp + uridp_len; >> + memcpy(pos, &END, sizeof(END)); >> + fp_size += uridp_len + sizeof(END); >> + file_path = (struct efi_device_path *)uridp; >> + argc -= 3; >> + argv += 3; >> + break; >> + } >> +#endif >> + >> default: >> r = CMD_RET_USAGE; >> goto out; >> @@ -1492,6 +1539,9 @@ static char efidebug_help_text[] = >> " -b|-B <bootid> <label> <interface> <devnum>[:<part>] <file path>\n" >> " -i|-I <interface> <devnum>[:<part>] <initrd file path>\n" >> " (-b, -i for short form device path)\n" >> +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS)) >> + " -u <bootid> <label> <uri>\n" >> +#endif >> " -s '<optional data>'\n" >> "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" >> " - delete UEFI BootXXXX variables\n" >> diff --git a/include/net.h b/include/net.h >> index 57889d8b7a..c748974573 100644 >> --- a/include/net.h >> +++ b/include/net.h >> @@ -935,4 +935,12 @@ static inline void eth_set_enable_bootdevs(bool enable) {} >> */ >> int wget_with_dns(ulong dst_addr, char *uri); >> >> +/** >> + * wget_validate_uri() - varidate the uri >> + * >> + * @uri: uri string of target file of wget >> + * Return: true if uri is valid, false if uri is invalid >> + */ >> +bool wget_validate_uri(char *uri); >> + >> #endif /* __NET_H__ */ >> diff --git a/net/wget.c b/net/wget.c >> index 4801e28eb9..6a4d22be32 100644 >> --- a/net/wget.c >> +++ b/net/wget.c >> @@ -558,3 +558,75 @@ out: >> return ret; >> } >> #endif >> + >> +/** >> + * wget_validate_uri() - validate the uri for wget >> + * >> + * @uri: uri string >> + * Return: true on success, false on failure >> + */ > > > It's better to have a comment description in one place. You already have > it in the header file. And even here the text is a little bit differ. OK, I will add a comment in the function header. > >> >> +bool wget_validate_uri(char *uri) >> +{ >> + char c; >> + bool ret = true; >> + char *str_copy, *s, *authority; >> + >> + /* TODO: strict uri conformance check */ >> + >> + /* >> + * Uri is expected to be correctly percent encoded. >> + * This is the minimum check, control codes(0x1-0x19, 0x7F, except '\0') >> + * and space character(0x20) are not allowed. >> + */ >> + for (c = 0x1; c < 0x21; c++) { >> + if (strchr(uri, c)) { >> + printf("invalid character is used\n"); >> + return false; >> + } >> + } >> + if (strchr(uri, 0x7f)) { >> + printf("invalid character is used\n"); >> + return false; >> + } > > > So you validate that only English characters exist in the url. There are urls with non English characters. I only excluded the control codes and space(0x20), so non-English characters and UTF-8 encoded URIs can be available. > > >> >> + >> + /* >> + * This follows the current U-Boot wget implementation. >> + * scheme: only "http:" is supported >> + * authority: >> + * - user information: not supported >> + * - host: supported >> + * - port: not supported(always use the default port) >> + */ >> + if (strncmp(uri, "http://", 7)) { >> + printf("only http:// is supported\n"); > > > log_err() OK, I will replace other printf() calls. Thanks, Masahisa Kojima > >> >> + return false; >> + } >> + str_copy = strdup(uri); >> + if (!str_copy) >> + return false; >> + >> + s = str_copy + strlen("http://"); >> + authority = strsep(&s, "/"); >> + if (!s) { >> + printf("invalid uri, no file path\n"); >> + ret = false; >> + goto out; >> + } >> + s = strchr(authority, '@'); >> + if (s) { >> + printf("user information is not supported\n"); >> + ret = false; >> + goto out; >> + } >> + s = strchr(authority, ':'); >> + if (s) { >> + printf("user defined port is not supported\n"); >> + ret = false; >> + goto out; >> + } >> + >> +out: >> + free(str_copy); >> + >> + return ret; >> +} >> -- >> 2.34.1 >>
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 0be3af3e76..f2fd6ba71d 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -19,6 +19,7 @@ #include <log.h> #include <malloc.h> #include <mapmem.h> +#include <net.h> #include <part.h> #include <search.h> #include <linux/ctype.h> @@ -829,6 +830,52 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, argc -= 1; argv += 1; break; +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS)) + case 'u': + { + char *pos; + int uridp_len; + struct efi_device_path_uri *uridp; + + if (argc < 3 || lo.label) { + r = CMD_RET_USAGE; + goto out; + } + id = (int)hextoul(argv[1], &endp); + if (*endp != '\0' || id > 0xffff) + return CMD_RET_USAGE; + + efi_create_indexed_name(var_name16, sizeof(var_name16), + "Boot", id); + + label = efi_convert_string(argv[2]); + if (!label) + return CMD_RET_FAILURE; + lo.label = label; + + uridp_len = sizeof(struct efi_device_path) + strlen(argv[3]) + 1; + fp_free = efi_alloc(uridp_len + sizeof(END)); + uridp = (struct efi_device_path_uri *)fp_free; + uridp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE; + uridp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_URI; + uridp->dp.length = uridp_len; + if (!wget_validate_uri(argv[3])) { + printf("ERROR: invalid URI\n"); + r = CMD_RET_FAILURE; + goto out; + } + + strcpy(uridp->uri, argv[3]); + pos = (char *)uridp + uridp_len; + memcpy(pos, &END, sizeof(END)); + fp_size += uridp_len + sizeof(END); + file_path = (struct efi_device_path *)uridp; + argc -= 3; + argv += 3; + break; + } +#endif + default: r = CMD_RET_USAGE; goto out; @@ -1492,6 +1539,9 @@ static char efidebug_help_text[] = " -b|-B <bootid> <label> <interface> <devnum>[:<part>] <file path>\n" " -i|-I <interface> <devnum>[:<part>] <initrd file path>\n" " (-b, -i for short form device path)\n" +#if (IS_ENABLED(CONFIG_BLKMAP) && IS_ENABLED(CONFIG_CMD_WGET) && IS_ENABLED(CONFIG_CMD_DNS)) + " -u <bootid> <label> <uri>\n" +#endif " -s '<optional data>'\n" "efidebug boot rm <bootid#1> [<bootid#2> [<bootid#3> [...]]]\n" " - delete UEFI BootXXXX variables\n" diff --git a/include/net.h b/include/net.h index 57889d8b7a..c748974573 100644 --- a/include/net.h +++ b/include/net.h @@ -935,4 +935,12 @@ static inline void eth_set_enable_bootdevs(bool enable) {} */ int wget_with_dns(ulong dst_addr, char *uri); +/** + * wget_validate_uri() - varidate the uri + * + * @uri: uri string of target file of wget + * Return: true if uri is valid, false if uri is invalid + */ +bool wget_validate_uri(char *uri); + #endif /* __NET_H__ */ diff --git a/net/wget.c b/net/wget.c index 4801e28eb9..6a4d22be32 100644 --- a/net/wget.c +++ b/net/wget.c @@ -558,3 +558,75 @@ out: return ret; } #endif + +/** + * wget_validate_uri() - validate the uri for wget + * + * @uri: uri string + * Return: true on success, false on failure + */ +bool wget_validate_uri(char *uri) +{ + char c; + bool ret = true; + char *str_copy, *s, *authority; + + /* TODO: strict uri conformance check */ + + /* + * Uri is expected to be correctly percent encoded. + * This is the minimum check, control codes(0x1-0x19, 0x7F, except '\0') + * and space character(0x20) are not allowed. + */ + for (c = 0x1; c < 0x21; c++) { + if (strchr(uri, c)) { + printf("invalid character is used\n"); + return false; + } + } + if (strchr(uri, 0x7f)) { + printf("invalid character is used\n"); + return false; + } + + /* + * This follows the current U-Boot wget implementation. + * scheme: only "http:" is supported + * authority: + * - user information: not supported + * - host: supported + * - port: not supported(always use the default port) + */ + if (strncmp(uri, "http://", 7)) { + printf("only http:// is supported\n"); + return false; + } + str_copy = strdup(uri); + if (!str_copy) + return false; + + s = str_copy + strlen("http://"); + authority = strsep(&s, "/"); + if (!s) { + printf("invalid uri, no file path\n"); + ret = false; + goto out; + } + s = strchr(authority, '@'); + if (s) { + printf("user information is not supported\n"); + ret = false; + goto out; + } + s = strchr(authority, ':'); + if (s) { + printf("user defined port is not supported\n"); + ret = false; + goto out; + } + +out: + free(str_copy); + + return ret; +}
This adds the URI device path option for 'boot add' subcommand. User can add the URI load option for downloading ISO image file or EFI application through network. Currently HTTP is only supported. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- cmd/efidebug.c | 50 +++++++++++++++++++++++++++++++++++ include/net.h | 8 ++++++ net/wget.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+)