[PATCH 1/4] Allow non-default ports for HTTP requests

[PATCH 1/4] Allow non-default ports for HTTP requests

[Matthew Garrett]

Add support for passing ports in HTTP requests. This takes the form of:
(http,serverip:portnum)/file
---
 grub-core/net/http.c |  8 ++++++--
 grub-core/net/net.c  | 10 +++++++++-
 include/grub/net.h   |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/grub-core/net/http.c b/grub-core/net/http.c
index 5aa4ad3..389a78e 100644
--- a/grub-core/net/http.c
+++ b/grub-core/net/http.c
@@ -309,7 +309,7 @@ http_establish (struct grub_file *file, grub_off_t offset, int initial)
 {
   http_data_t data = file->data;
   grub_uint8_t *ptr;
-  int i;
+  int i, port;
   struct grub_net_buff *nb;
   grub_err_t err;
 
@@ -390,8 +390,12 @@ http_establish (struct grub_file *file, grub_off_t offset, int initial)
   grub_netbuff_put (nb, 2);
   grub_memcpy (ptr, "\r\n", 2);
 
+  if (file->device->net->port)
+    port = file->device->net->port;
+  else
+    port = HTTP_PORT;
   data->sock = grub_net_tcp_open (file->device->net->server,
-				  HTTP_PORT, http_receive,
+				  port, http_receive,
 				  http_err, http_err,
 				  file);
   if (!data->sock)
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 10773fc..585f4f7 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1261,7 +1261,7 @@ grub_net_open_real (const char *name)
   grub_net_app_level_t proto;
   const char *protname, *server;
   grub_size_t protnamelen;
-  int try;
+  int try, port = 0;
 
   if (grub_strncmp (name, "pxe:", sizeof ("pxe:") - 1) == 0)
     {
@@ -1278,7 +1278,14 @@ grub_net_open_real (const char *name)
   else
     {
       const char *comma;
+      char *colon;
       comma = grub_strchr (name, ',');
+      colon = grub_strchr (name, ':');
+      if (colon)
+	{
+	  port = (int) grub_strtol(colon+1, NULL, 10);
+	  *colon = '\0';
+	}
       if (comma)
 	{
 	  protnamelen = comma - name;
@@ -1310,6 +1317,7 @@ grub_net_open_real (const char *name)
 	    if (!ret)
 	      return NULL;
 	    ret->protocol = proto;
+	    ret->port = port;
 	    ret->server = grub_strdup (server);
 	    if (!ret->server)
 	      {
diff --git a/include/grub/net.h b/include/grub/net.h
index 2192fa1..c517509 100644
--- a/include/grub/net.h
+++ b/include/grub/net.h
@@ -276,6 +276,7 @@ typedef struct grub_net
   grub_fs_t fs;
   int eof;
   int stall;
+  int port;
 } *grub_net_t;
 
 extern grub_net_t (*EXPORT_VAR (grub_net_open)) (const char *name);
-- 
2.9.3

[PATCH 2/4] Send a user class identifier in bootp requests and tag it as DHCP discover

[Matthew Garrett]

It's helpful to determine that a request was sent by grub in order to permit
the server to provide different information at different stages of the boot
process. Send GRUB2 as a type 77 DHCP option when sending bootp packets in
order to make this possible and tag the request as a DHCP discover to
convince servers to pay attention to it. Add the client architecture for
good measure.
---
 grub-core/net/bootp.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index 9e2fdb7..0da8e24 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -25,6 +25,24 @@
 #include <grub/net/udp.h>
 #include <grub/datetime.h>
 
+#if !defined(GRUB_MACHINE_EFI) && (defined(__i386__) || defined(__x86_64__))
+#define GRUB_NET_BOOTP_ARCH 0x0000
+#elif defined(GRUB_MACHINE_EFI) && defined(__x86_64__)
+#define GRUB_NET_BOOTP_ARCH 0x0007
+#elif defined(GRUB_MACHINE_EFI) && defined(__aarch64__)
+#define GRUB_NET_BOOTP_ARCH 0x000B
+#else
+#error "unknown bootp architecture"
+#endif
+
+static grub_uint8_t dhcp_option_header[] = {GRUB_NET_BOOTP_RFC1048_MAGIC_0,
+					    GRUB_NET_BOOTP_RFC1048_MAGIC_1,
+					    GRUB_NET_BOOTP_RFC1048_MAGIC_2,
+					    GRUB_NET_BOOTP_RFC1048_MAGIC_3};
+static grub_uint8_t grub_userclass[] = {0x4D, 0x06, 0x05, 'G', 'R', 'U', 'B', '2'};
+static grub_uint8_t grub_dhcpdiscover[] = {0x35, 0x01, 0x01};
+static grub_uint8_t grub_dhcptime[] = {0x33, 0x04, 0x00, 0x00, 0x0e, 0x10};
+
 static void
 parse_dhcp_vendor (const char *name, const void *vend, int limit, int *mask)
 {
@@ -499,10 +517,14 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
 	  struct udphdr *udph;
 	  grub_net_network_level_address_t target;
 	  grub_net_link_level_address_t ll_target;
+	  grub_uint8_t *offset;
 
 	  if (!ifaces[j].prev)
 	    continue;
-	  nb = grub_netbuff_alloc (sizeof (*pack) + 64 + 128);
+	  nb = grub_netbuff_alloc (sizeof (*pack) + sizeof(dhcp_option_header)
+				   + sizeof(grub_userclass)
+				   + sizeof(grub_dhcpdiscover)
+				   + sizeof(grub_dhcptime) + 64 + 128);
 	  if (!nb)
 	    {
 	      grub_netbuff_free (nb);
@@ -536,6 +558,24 @@ grub_cmd_bootp (struct grub_command *cmd __attribute__ ((unused)),
 	  pack->seconds = grub_cpu_to_be16 (t);
 
 	  grub_memcpy (&pack->mac_addr, &ifaces[j].hwaddress.mac, 6); 
+	  offset = (grub_uint8_t *)&pack->vendor;
+	  grub_memcpy (offset, dhcp_option_header, sizeof(dhcp_option_header));
+	  offset += sizeof(dhcp_option_header);
+	  grub_memcpy (offset, grub_dhcpdiscover, sizeof(grub_dhcpdiscover));
+	  offset += sizeof(grub_dhcpdiscover);
+	  grub_memcpy (offset, grub_userclass, sizeof(grub_userclass));
+	  offset += sizeof(grub_userclass);
+	  grub_memcpy (offset, grub_dhcptime, sizeof(grub_dhcptime));
+
+	  /* insert Client System Architecture (option 93) */
+	  offset += sizeof(grub_dhcptime);
+	  offset[0] = 93;
+	  offset[1] = 2;
+	  offset[2] = (GRUB_NET_BOOTP_ARCH >> 8);
+	  offset[3] = (GRUB_NET_BOOTP_ARCH & 0xFF);
+
+	  /* option terminator */
+	  offset[4] = 255;
 
 	  grub_netbuff_push (nb, sizeof (*udph));
 
-- 
2.9.3

[PATCH 3/4] Don't allocate a new address buffer if we receive multiple DNS responses

[Matthew Garrett]

The current logic in the DNS resolution code allocates an address buffer
based on the number of addresses in the response packet. If we receive
multiple response packets in response to a single query packet, this means
that we will reallocate a new buffer large enough for only the addresses in
that specific packet, discarding any previous results in the process. Worse,
we still keep track of the *total* number of addresses resolved in response
to this query, not merely the number in the packet being currently processed.
Use realloc() rather than malloc() to avoid overwriting the existing data,
and allocate a buffer large enough for the total set of addresses rather
than merely the number in this specific response.
---
 grub-core/net/dns.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
index 5d9afe0..5deb1ef 100644
--- a/grub-core/net/dns.c
+++ b/grub-core/net/dns.c
@@ -285,8 +285,8 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ ((unused)),
       ptr++;
       ptr += 4;
     }
-  *data->addresses = grub_malloc (sizeof ((*data->addresses)[0])
-				 * grub_be_to_cpu16 (head->ancount));
+  *data->addresses = grub_realloc (*data->addresses, sizeof ((*data->addresses)[0])
+		     * (grub_be_to_cpu16 (head->ancount) + *data->naddresses));
   if (!*data->addresses)
     {
       grub_errno = GRUB_ERR_NONE;
-- 
2.9.3

[PATCH 4/4] Allow protocol to be separated from host with a semicolon

[Matthew Garrett]

Some DHCP servers (such as dnsmasq) tokenise parameters with commas, making
it impossible to pass boot files with commas in them. Allow using a
semicolon to separate the protocol from host if a comma wasn't found.
---
 grub-core/net/net.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 585f4f7..efacb30 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1280,6 +1280,10 @@ grub_net_open_real (const char *name)
       const char *comma;
       char *colon;
       comma = grub_strchr (name, ',');
+      if (!comma)
+	{
+	  comma = grub_strchr (name, ';');
+	}
       colon = grub_strchr (name, ':');
       if (colon)
 	{
-- 
2.9.3

Re: [PATCH 3/4] Don't allocate a new address buffer if we receive multiple DNS responses

[Matthew Garrett]

On Mon, Jan 23, 2017 at 7:55 PM, Andrei Borzenkov <arvidjaar@gmail.com> wrote:
> This was noted previously by Josef, we discussed it and my position is
> that resolver code requires redesign to correctly merge multiple answers
> and prioritize A vs AAAA requests.
>
> Do you get actual errors with current master? If yes, could you provide
> more information what this patch fixes?

Ah, yeah, it looks like this is unnecessary with the current state of
the code. I'll drop it, thanks!

Re: [PATCH 3/4] Don't allocate a new address buffer if we receive multiple DNS responses

[Andrei Borzenkov]

24.01.2017 03:36, Matthew Garrett пишет:
> The current logic in the DNS resolution code allocates an address buffer
> based on the number of addresses in the response packet. If we receive
> multiple response packets in response to a single query packet, this means
> that we will reallocate a new buffer large enough for only the addresses in
> that specific packet, discarding any previous results in the process. Worse,
> we still keep track of the *total* number of addresses resolved in response
> to this query, not merely the number in the packet being currently processed.
> Use realloc() rather than malloc() to avoid overwriting the existing data,
> and allocate a buffer large enough for the total set of addresses rather
> than merely the number in this specific response.
> ---
>  grub-core/net/dns.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
> index 5d9afe0..5deb1ef 100644
> --- a/grub-core/net/dns.c
> +++ b/grub-core/net/dns.c
> @@ -285,8 +285,8 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ ((unused)),
>        ptr++;
>        ptr += 4;
>      }
> -  *data->addresses = grub_malloc (sizeof ((*data->addresses)[0])
> -				 * grub_be_to_cpu16 (head->ancount));
> +  *data->addresses = grub_realloc (*data->addresses, sizeof ((*data->addresses)[0])
> +		     * (grub_be_to_cpu16 (head->ancount) + *data->naddresses));

If *data->addresses was not NULL, we should not reach this code.

  /* Code apparently assumed that only one packet is received as response.
     We may get multiple responses due to network condition, so check here
     and quit early. */
  if (*data->addresses)
    {
      grub_netbuff_free (nb);
      return GRUB_ERR_NONE;
    }

This was noted previously by Josef, we discussed it and my position is
that resolver code requires redesign to correctly merge multiple answers
and prioritize A vs AAAA requests.

Do you get actual errors with current master? If yes, could you provide
more information what this patch fixes?

>    if (!*data->addresses)
>      {
>        grub_errno = GRUB_ERR_NONE;
> 

[PATCH 3/4] Don't allocate a new address buffer if we receive multiple DNS responses

[Matthew Garrett]

The current logic in the DNS resolution code allocates an address buffer
based on the number of addresses in the response packet. If we receive
multiple response packets in response to a single query packet, this means
that we will reallocate a new buffer large enough for only the addresses in
that specific packet, discarding any previous results in the process. Worse,
we still keep track of the *total* number of addresses resolved in response
to this query, not merely the number in the packet being currently processed.
Use realloc() rather than malloc() to avoid overwriting the existing data,
and allocate a buffer large enough for the total set of addresses rather
than merely the number in this specific response.
---
 grub-core/net/dns.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
index 5d9afe0..5deb1ef 100644
--- a/grub-core/net/dns.c
+++ b/grub-core/net/dns.c
@@ -285,8 +285,8 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ ((unused)),
       ptr++;
       ptr += 4;
     }
-  *data->addresses = grub_malloc (sizeof ((*data->addresses)[0])
-				 * grub_be_to_cpu16 (head->ancount));
+  *data->addresses = grub_realloc (*data->addresses, sizeof ((*data->addresses)[0])
+		     * (grub_be_to_cpu16 (head->ancount) + *data->naddresses));
   if (!*data->addresses)
     {
       grub_errno = GRUB_ERR_NONE;
-- 
2.9.3