[PATCH] dhcp: Add domain name option handler

[PATCH] dhcp: Add domain name option handler

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]

Add parser and accessor for the domain name lease option.
---
 ell/dhcp-lease.c   | 24 +++++++++++++++++++++++-
 ell/dhcp-private.h |  1 +
 ell/dhcp.h         |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/ell/dhcp-lease.c b/ell/dhcp-lease.c
index 5f801e7..06ef76a 100644
--- a/ell/dhcp-lease.c
+++ b/ell/dhcp-lease.c
@@ -45,6 +45,8 @@ void _dhcp_lease_free(struct l_dhcp_lease *lease)
 		return;
 
 	l_free(lease->dns);
+	l_free(lease->domain_name);
+
 	l_free(lease);
 }
 
@@ -99,6 +101,18 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
 				}
 			}
 			break;
+		case L_DHCP_OPTION_DOMAIN_NAME:
+			if (l < 1)
+				break;
+
+			lease->domain_name = l_new(char, l + 1);
+
+			memcpy(lease->domain_name, v, l);
+
+			if (lease->domain_name[l - 1] != '\0')
+				lease->domain_name[l] = '\0';
+
+			break;
 		default:
 			break;
 		}
@@ -135,7 +149,7 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
 
 	return lease;
 error:
-	l_free(lease);
+	_dhcp_lease_free(lease);
 	return NULL;
 }
 
@@ -212,6 +226,14 @@ LIB_EXPORT char **l_dhcp_lease_get_dns(const struct l_dhcp_lease *lease)
 	return dns_list;
 }
 
+LIB_EXPORT char *l_dhcp_lease_get_domain_name(const struct l_dhcp_lease *lease)
+{
+	if (unlikely(!lease))
+		return NULL;
+
+	return l_strdup(lease->domain_name);
+}
+
 LIB_EXPORT uint32_t l_dhcp_lease_get_t1(const struct l_dhcp_lease *lease)
 {
 	if (unlikely(!lease))
diff --git a/ell/dhcp-private.h b/ell/dhcp-private.h
index 6554fc6..a75bb8b 100644
--- a/ell/dhcp-private.h
+++ b/ell/dhcp-private.h
@@ -120,6 +120,7 @@ struct l_dhcp_lease {
 	uint32_t t2;
 	uint32_t router;
 	uint32_t *dns;
+	char *domain_name;
 };
 
 struct l_dhcp_lease *_dhcp_lease_new(void);
diff --git a/ell/dhcp.h b/ell/dhcp.h
index c3a4988..b8a5b41 100644
--- a/ell/dhcp.h
+++ b/ell/dhcp.h
@@ -95,6 +95,7 @@ char *l_dhcp_lease_get_netmask(const struct l_dhcp_lease *lease);
 char *l_dhcp_lease_get_broadcast(const struct l_dhcp_lease *lease);
 char *l_dhcp_lease_get_server_id(const struct l_dhcp_lease *lease);
 char **l_dhcp_lease_get_dns(const struct l_dhcp_lease *lease);
+char *l_dhcp_lease_get_domain_name(const struct l_dhcp_lease *lease);
 
 uint32_t l_dhcp_lease_get_t1(const struct l_dhcp_lease *lease);
 uint32_t l_dhcp_lease_get_t2(const struct l_dhcp_lease *lease);
-- 
2.13.6

Re: [PATCH] dhcp: Add domain name option handler

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

Hi Tim,

On 12/6/19 5:21 PM, Tim Kourt wrote:
> Add parser and accessor for the domain name lease option.
> ---
>   ell/dhcp-lease.c   | 24 +++++++++++++++++++++++-
>   ell/dhcp-private.h |  1 +
>   ell/dhcp.h         |  1 +
>   3 files changed, 25 insertions(+), 1 deletion(-)
> 

<snip>

> @@ -99,6 +101,18 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
>   				}
>   			}
>   			break;
> +		case L_DHCP_OPTION_DOMAIN_NAME:
> +			if (l < 1)
> +				break;
> +
> +			lease->domain_name = l_new(char, l + 1);
> +
> +			memcpy(lease->domain_name, v, l);
> +
> +			if (lease->domain_name[l - 1] != '\0')
> +				lease->domain_name[l] = '\0';

So strictly speaking this if is not necessary as you already use l_new 
which memsets returned memory to 0.  However, you might want to be much 
more strict about accepting NULs at the end.  One is okay, multiple is 
likely not ok.  Embedded NULs are probably not okay either.

Also, it might be a good idea to more strictly check the domain name 
here.  Fail if we get an empty string, a dot or localhost.  I believe 
hostnames has a max length as well.  And we should maybe limit these to 
ascii or at least valid utf8 for now.

> +
> +			break;
>   		default:
>   			break;
>   		}

Regards,
-Denis

[PATCH] dhcp: Add domain name option handler

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

Add parser and accessor for domain name lease option.
---
 ell/dhcp-lease.c   | 16 +++++++++++++++-
 ell/dhcp-private.h |  1 +
 ell/dhcp.h         |  1 +
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/ell/dhcp-lease.c b/ell/dhcp-lease.c
index a48cfa4..ba57322 100644
--- a/ell/dhcp-lease.c
+++ b/ell/dhcp-lease.c
@@ -45,6 +45,8 @@ void _dhcp_lease_free(struct l_dhcp_lease *lease)
 		return;
 
 	l_free(lease->dns);
+	l_free(lease->domain_name);
+
 	l_free(lease);
 }
 
@@ -99,6 +101,10 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
 				}
 			}
 			break;
+		case L_DHCP_OPTION_DOMAIN_NAME:
+			if (l >= 1)
+				lease->domain_name = l_strdup(v);
+			break;
 		default:
 			break;
 		}
@@ -124,7 +130,7 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
 
 	return lease;
 error:
-	l_free(lease);
+	_dhcp_lease_free(lease);
 	return NULL;
 }
 
@@ -201,6 +207,14 @@ LIB_EXPORT char **l_dhcp_lease_get_dns(const struct l_dhcp_lease *lease)
 	return dns_list;
 }
 
+LIB_EXPORT char *l_dhcp_lease_get_domain_name(const struct l_dhcp_lease *lease)
+{
+	if (unlikely(!lease))
+		return NULL;
+
+	return l_strdup(lease->domain_name);
+}
+
 LIB_EXPORT uint32_t l_dhcp_lease_get_t1(const struct l_dhcp_lease *lease)
 {
 	if (unlikely(!lease))
diff --git a/ell/dhcp-private.h b/ell/dhcp-private.h
index 6554fc6..a75bb8b 100644
--- a/ell/dhcp-private.h
+++ b/ell/dhcp-private.h
@@ -120,6 +120,7 @@ struct l_dhcp_lease {
 	uint32_t t2;
 	uint32_t router;
 	uint32_t *dns;
+	char *domain_name;
 };
 
 struct l_dhcp_lease *_dhcp_lease_new(void);
diff --git a/ell/dhcp.h b/ell/dhcp.h
index c3a4988..b8a5b41 100644
--- a/ell/dhcp.h
+++ b/ell/dhcp.h
@@ -95,6 +95,7 @@ char *l_dhcp_lease_get_netmask(const struct l_dhcp_lease *lease);
 char *l_dhcp_lease_get_broadcast(const struct l_dhcp_lease *lease);
 char *l_dhcp_lease_get_server_id(const struct l_dhcp_lease *lease);
 char **l_dhcp_lease_get_dns(const struct l_dhcp_lease *lease);
+char *l_dhcp_lease_get_domain_name(const struct l_dhcp_lease *lease);
 
 uint32_t l_dhcp_lease_get_t1(const struct l_dhcp_lease *lease);
 uint32_t l_dhcp_lease_get_t2(const struct l_dhcp_lease *lease);
-- 
2.13.6

Re: [PATCH] dhcp: Add domain name option handler

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2792 bytes --]

Hi Tim,

On 10/18/19 5:29 PM, Tim Kourt wrote:
> Add parser and accessor for domain name lease option.
> ---
>   ell/dhcp-lease.c   | 16 +++++++++++++++-
>   ell/dhcp-private.h |  1 +
>   ell/dhcp.h         |  1 +
>   3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/ell/dhcp-lease.c b/ell/dhcp-lease.c
> index a48cfa4..ba57322 100644
> --- a/ell/dhcp-lease.c
> +++ b/ell/dhcp-lease.c
> @@ -45,6 +45,8 @@ void _dhcp_lease_free(struct l_dhcp_lease *lease)
>   		return;
>   
>   	l_free(lease->dns);
> +	l_free(lease->domain_name);
> +
>   	l_free(lease);
>   }
>   
> @@ -99,6 +101,10 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
>   				}
>   			}
>   			break;
> +		case L_DHCP_OPTION_DOMAIN_NAME:
> +			if (l >= 1)
> +				lease->domain_name = l_strdup(v);
> +			break;

No. You cannot assume that v is null terminated.  Nobody will guarantee 
that the packets coming from the 'dhcp server' aren't fuzzed or are not 
from dhcp server at all.

>   		default:
>   			break;
>   		}
> @@ -124,7 +130,7 @@ struct l_dhcp_lease *_dhcp_lease_parse_options(struct dhcp_message_iter *iter)
>   
>   	return lease;
>   error:
> -	l_free(lease);
> +	_dhcp_lease_free(lease);
>   	return NULL;
>   }
>   
> @@ -201,6 +207,14 @@ LIB_EXPORT char **l_dhcp_lease_get_dns(const struct l_dhcp_lease *lease)
>   	return dns_list;
>   }
>   
> +LIB_EXPORT char *l_dhcp_lease_get_domain_name(const struct l_dhcp_lease *lease)
> +{
> +	if (unlikely(!lease))
> +		return NULL;
> +
> +	return l_strdup(lease->domain_name);

Strictly speaking this can be const char *, but okay.

> +}
> +
>   LIB_EXPORT uint32_t l_dhcp_lease_get_t1(const struct l_dhcp_lease *lease)
>   {
>   	if (unlikely(!lease))
> diff --git a/ell/dhcp-private.h b/ell/dhcp-private.h
> index 6554fc6..a75bb8b 100644
> --- a/ell/dhcp-private.h
> +++ b/ell/dhcp-private.h
> @@ -120,6 +120,7 @@ struct l_dhcp_lease {
>   	uint32_t t2;
>   	uint32_t router;
>   	uint32_t *dns;
> +	char *domain_name;
>   };
>   
>   struct l_dhcp_lease *_dhcp_lease_new(void);
> diff --git a/ell/dhcp.h b/ell/dhcp.h
> index c3a4988..b8a5b41 100644
> --- a/ell/dhcp.h
> +++ b/ell/dhcp.h
> @@ -95,6 +95,7 @@ char *l_dhcp_lease_get_netmask(const struct l_dhcp_lease *lease);
>   char *l_dhcp_lease_get_broadcast(const struct l_dhcp_lease *lease);
>   char *l_dhcp_lease_get_server_id(const struct l_dhcp_lease *lease);
>   char **l_dhcp_lease_get_dns(const struct l_dhcp_lease *lease);
> +char *l_dhcp_lease_get_domain_name(const struct l_dhcp_lease *lease);
>   
>   uint32_t l_dhcp_lease_get_t1(const struct l_dhcp_lease *lease);
>   uint32_t l_dhcp_lease_get_t2(const struct l_dhcp_lease *lease);
> 

Regards,
-Denis