[PATCH ustream-ssl v2] ustream-mbedtls: Use getrandom() instead of /dev/urandom
Instead of keeping a file descriptor open just use the getrandom syscall
to get random data. This is supported by the musl, glibc and Linux for
some time now.
This also improves the error handling in case this function returns not
as many bytes as expected.
Signed-off-by: Hauke Mehrtens
---
ustream-mbedtls.c | 25 ++---
1 file changed, 6 insertions(+), 19 deletions(-)
changes since v1:
* rename _urandom to _random
diff --git a/ustream-mbedtls.c b/ustream-mbedtls.c
index e79e37b..7fc7874 100644
--- a/ustream-mbedtls.c
+++ b/ustream-mbedtls.c
@@ -17,6 +17,7 @@
*/
#include
+#include
#include
#include
#include
@@ -25,8 +26,6 @@
#include "ustream-ssl.h"
#include "ustream-internal.h"
-static int urandom_fd = -1;
-
static int s_ustream_read(void *ctx, unsigned char *buf, size_t len)
{
struct ustream *s = ctx;
@@ -66,21 +65,12 @@ __hidden void ustream_set_io(struct ustream_ssl_ctx *ctx,
void *ssl, struct ustr
mbedtls_ssl_set_bio(ssl, conn, s_ustream_write, s_ustream_read, NULL);
}
-static bool urandom_init(void)
+static int _random(void *ctx, unsigned char *out, size_t len)
{
- if (urandom_fd > -1)
- return true;
+ ssize_t ret;
- urandom_fd = open("/dev/urandom", O_RDONLY);
- if (urandom_fd < 0)
- return false;
-
- return true;
-}
-
-static int _urandom(void *ctx, unsigned char *out, size_t len)
-{
- if (read(urandom_fd, out, len) < 0)
+ ret = getrandom(out, len, 0);
+ if (ret < 0 || (size_t)ret != len)
return MBEDTLS_ERR_ENTROPY_SOURCE_FAILED;
return 0;
@@ -134,9 +124,6 @@ __ustream_ssl_context_new(bool server)
mbedtls_ssl_config *conf;
int ep;
- if (!urandom_init())
- return NULL;
-
ctx = calloc(1, sizeof(*ctx));
if (!ctx)
return NULL;
@@ -159,7 +146,7 @@ __ustream_ssl_context_new(bool server)
mbedtls_ssl_config_defaults(conf, ep, MBEDTLS_SSL_TRANSPORT_STREAM,
MBEDTLS_SSL_PRESET_DEFAULT);
- mbedtls_ssl_conf_rng(conf, _urandom, NULL);
+ mbedtls_ssl_conf_rng(conf, _random, NULL);
if (server) {
mbedtls_ssl_conf_authmode(conf, MBEDTLS_SSL_VERIFY_NONE);
--
2.39.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH ustream-ssl] ustream-mbedtls: Use getrandom() instead of /dev/urandom
Hi Torsten, Sorry for the late answer, I forgot about this mail thread. On 1/30/23 10:57, Torsten Duwe wrote: Hi Hauke! On Sun, 29 Jan 2023 17:08:38 +0100 Hauke Mehrtens wrote: drivers/char/random.c lines 1240- ... * Reading from /dev/urandom has the same functionality as calling * getrandom(2) with flags=GRND_INSECURE. Because it does not block * waiting for the RNG to be ready, it should not be used. Haven't audited mbedtls, but I assume it reads urandom for "lesser" entropy when needed. In any case, getrandom(out, len, GRND_INSECURE) would be the proper replacement. Torsten Hi Torsten, The mapage says this: > By default, getrandom() draws entropy from the urandom source > (i.e., the same source as the /dev/urandom device). This > behavior can be changed via the flags argument. https://man7.org/linux/man-pages/man2/getrandom.2.html GRND_INSECURE is also not documented in the man page. Well, it exists in the kernel source and headers... The option was added to the Linux kernel 5.6 here: https://git.kernel.org/linus/75551dbf112c992bc6c99a972990b3f272247e23 The documentation says > GRND_INSECUREReturn non-cryptographic random bytes We want to use the random bytes in mbedtls for cryptographic operations. I think giving no flags is the correct option here. I think the behavior of /dev/random changed some years ago. This article described it a bit: https://lwn.net/Articles/808575/ That's only the internal workings. BTW, the mentioned quote of Andy Lutomirski applies here in some sense. You read away the true entropy and might even block on it when pseudo- randomness might suffice, see below. As far as I understood the code, giving no flags will guarantee that the random pool is initialized (crng_ready() returns true) and otherwise it is the same as using GRND_INSECURE. As we use it for cryptographic operations I think we should give no flags. "cryptographic operations" is a wide area. Some randomness is required, to varying degrees, for long-term keys, session keys, IVs and padding. ustreamss uses the randomness to generate session keys (including ephemeral keys), IVs and padding. The long term keys are generated in a different application. Especially for long living keys, each end every bit should be totally unpredictable, which should correspond to read an appropriate amount from /dev/random. IVs and padding can be generated from a pseudo-RNG whose initial state is "uncertain enough", usually /dev/urandom. GIT is cool. c6e9d6f388947986 2014-Jul-17 tytso: introduce getrandom(2) system call 75551dbf112c992b 2019-Dec-23 luto: add GRND_INSECURE ... 48446f198f9adcb4 2019-Dec-23 luto: ignore GRND_RANDOM The first commit also has a man page in the comment, which is probably what was recorded. The second one changes the no-flags behaviour, away from the man page text you quoted. Someone once mentioned on LKML that drivers/char/random.c needs better maintenance... ;) I had a quick look at mbedtls and its usage of the provided rng function. It generates not only padding and IVs, but also session keys. Especially on OpenWRT these are a trade-off IMHO. OpenWRT supports a lot of hardware that is low on entropy at boot (MIPS anyone?) Do you want to block early ssh sessions, maybe even for minutes, or would you rather risk eavesdropping on those early connections? Depending on your choice for ustream, you can keep the proposed code, but please rename the functions to "random", not "urandom". Or you want to stick with the current urandom behaviour but then please add Luto's GRND_INSECURE flag to achieve that. crng_ready() should only return false at bootup before the system got enough random bits, afterwards it never returns false. Even without GRND_INSECURE it will never run out of entropy. I think we should wait with creating TLS sessions till we have enough random data to do it securely. I tested this on a lantiq xrx200 (MIPS) device and it was initialized much before the LAN interface was up. The code in ustream-mbedtls.c was probably initially written when /dev/random was still blocking when too much entropy was read out of the pool. I will rename the function. Hauke ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[sdwalker/sdwalker.github.io] 5b17c4: This week's update
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software.--- Begin Message --- Branch: refs/heads/master Home: https://github.com/sdwalker/sdwalker.github.io Commit: 5b17c4e601ca553743ef10fccd6efc1345a5a4bb https://github.com/sdwalker/sdwalker.github.io/commit/5b17c4e601ca553743ef10fccd6efc1345a5a4bb Author: Stephen Walker Date: 2023-02-19 (Sun, 19 Feb 2023) Changed paths: M uscan/index-21.02.html M uscan/index-22.03.html M uscan/index.html Log Message: --- This week's update --- End Message --- ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[PATCH netifd 2/5] netifd: Fix multiple -Wsign-compare warnings
This fixes warnings like this:
warning: comparison of integer expressions of different signedness: 'int' and
'long unsigned int' [-Wsign-compare]
Mostly this was an int compared to a size_t returned by ARRAY_SIZE().
The easiest fix is to count on the size_t type.
The ifindex is sometimes an unsigned int and sometimes a signed int in
the kernel interfaces. I think it normally fits into an unsigned 16 bit
value, so this should be fine. Do the one comparison where the
compiler complains as a long.
Casting the result of sizeof() to int should be safe. These values are
never out of range of int.
Signed-off-by: Hauke Mehrtens
---
bonding.c | 2 +-
handler.c | 5 +++--
interface-ip.c | 2 +-
main.c | 4 ++--
system-linux.c | 21 -
ubus.c | 4 ++--
vlan.c | 4 ++--
wireless.c | 2 +-
8 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/bonding.c b/bonding.c
index 457fe51..f4005de 100644
--- a/bonding.c
+++ b/bonding.c
@@ -396,7 +396,7 @@ bonding_apply_settings(struct bonding_device *bdev, struct
blob_attr **tb)
if ((cur = tb[BOND_ATTR_POLICY]) != NULL) {
const char *policy = blobmsg_get_string(cur);
- int i;
+ size_t i;
for (i = 0; i < ARRAY_SIZE(bonding_policy_str); i++) {
if (strcmp(policy, bonding_policy_str[i]) != 0)
diff --git a/handler.c b/handler.c
index 04bdbee..78fc9a0 100644
--- a/handler.c
+++ b/handler.c
@@ -229,7 +229,8 @@ netifd_parse_extdev_handler(const char *path_to_file,
create_extdev_handler_cb c
void netifd_init_script_handlers(int dir_fd, script_dump_cb cb)
{
glob_t g;
- int i, prev_fd;
+ int prev_fd;
+ size_t i;
prev_fd = netifd_dir_push(dir_fd);
if (glob("./*.sh", 0, NULL, )) {
@@ -252,7 +253,7 @@ netifd_init_extdev_handlers(int dir_fd,
create_extdev_handler_cb cb)
prev_fd = netifd_dir_push(dir_fd);
glob("*.json", 0, NULL, );
- for (int i = 0; i < g.gl_pathc; i++)
+ for (size_t i = 0; i < g.gl_pathc; i++)
netifd_parse_extdev_handler(g.gl_pathv[i], cb);
netifd_dir_pop(prev_fd);
}
diff --git a/interface-ip.c b/interface-ip.c
index ab4a5cf..7359db2 100644
--- a/interface-ip.c
+++ b/interface-ip.c
@@ -99,7 +99,7 @@ static struct uloop_timeout valid_until_timeout;
static void
clear_if_addr(union if_addr *a, int mask)
{
- int m_bytes = (mask + 7) / 8;
+ size_t m_bytes = (mask + 7) / 8;
uint8_t m_clear = (1 << (m_bytes * 8 - mask)) - 1;
uint8_t *p = (uint8_t *) a;
diff --git a/main.c b/main.c
index 4c1c855..874dc8b 100644
--- a/main.c
+++ b/main.c
@@ -303,8 +303,8 @@ int main(int argc, char **argv)
break;
case 'l':
log_level = atoi(optarg);
- if (log_level >= ARRAY_SIZE(log_class))
- log_level = ARRAY_SIZE(log_class) - 1;
+ if (log_level >= (int)ARRAY_SIZE(log_class))
+ log_level = (int)ARRAY_SIZE(log_class) - 1;
break;
#ifndef DUMMY_MODE
case 'S':
diff --git a/system-linux.c b/system-linux.c
index 45a9efb..d13a561 100644
--- a/system-linux.c
+++ b/system-linux.c
@@ -1154,7 +1154,7 @@ static bool check_ifaddr(struct nlmsghdr *hdr, int
ifindex)
{
struct ifaddrmsg *ifa = NLMSG_DATA(hdr);
- return ifa->ifa_index == ifindex;
+ return (long)ifa->ifa_index == ifindex;
}
static bool check_route(struct nlmsghdr *hdr, int ifindex)
@@ -1438,7 +1438,8 @@ int system_macvlan_add(struct device *macvlan, struct
device *dev, struct macvla
{
struct nl_msg *msg;
struct nlattr *linkinfo, *data;
- int i, rv;
+ size_t i;
+ int rv;
static const struct {
const char *name;
enum macvlan_mode val;
@@ -1700,7 +1701,7 @@ system_set_ethtool_settings(struct device *dev, struct
device_settings *s)
.ifr_data = (caddr_t),
};
static const struct {
- int speed;
+ unsigned int speed;
uint8_t bit_half;
uint8_t bit_full;
} speed_mask[] = {
@@ -1709,7 +1710,7 @@ system_set_ethtool_settings(struct device *dev, struct
device_settings *s)
{ 1000, ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
ETHTOOL_LINK_MODE_1000baseT_Full_BIT },
};
uint32_t adv;
- int i;
+ size_t i;
strncpy(ifr.ifr_name, dev->ifname, sizeof(ifr.ifr_name) - 1);
@@ -2355,7 +2356,7 @@ static const struct {
static void system_add_link_modes(struct blob_buf *b, __u32 mask)
{
- int i;
+ size_t i;
for (i = 0; i < ARRAY_SIZE(ethtool_link_modes); i++) {
if (mask & ethtool_link_modes[i].mask)
blobmsg_add_string(b, NULL, ethtool_link_modes[i].name);
@@
[PATCH netifd 4/5] netifd: Explicitly zero initialize variables
The -pedantic option was complaining about the old initialization and
prefers if it is explicitly initialized to zero.
Signed-off-by: Hauke Mehrtens
---
proto.c| 2 +-
system-linux.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/proto.c b/proto.c
index 01473f2..48dd213 100644
--- a/proto.c
+++ b/proto.c
@@ -416,7 +416,7 @@ proto_apply_static_ip_settings(struct interface *iface,
struct blob_attr *attr)
unsigned int netmask = 32;
bool ip6deprecated;
int n_v4 = 0, n_v6 = 0;
- struct in_addr bcast = {}, ptp = {};
+ struct in_addr bcast = {0,}, ptp = {0,};
blobmsg_parse(proto_ip_attributes, __OPT_MAX, tb, blob_data(attr),
blob_len(attr));
diff --git a/system-linux.c b/system-linux.c
index d13a561..e4041fb 100644
--- a/system-linux.c
+++ b/system-linux.c
@@ -1529,7 +1529,7 @@ int system_netns_set(int netns_fd)
int system_veth_add(struct device *veth, struct veth_config *cfg)
{
struct nl_msg *msg;
- struct ifinfomsg empty_iim = {};
+ struct ifinfomsg empty_iim = {0,};
struct nlattr *linkinfo, *data, *veth_info;
int rv;
--
2.39.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[PATCH netifd 5/5] netifd: Activate -Wextra compile warnings
This activates some more compile warnings.
-pedantic is not yet activated, then we see too many errors which I do
not know how to mitigate.
Signed-off-by: Hauke Mehrtens
---
CMakeLists.txt | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b3bf411..b87c300 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -7,7 +7,11 @@ IF(NOT ${CMAKE_VERSION} LESS 3.0)
check_c_compiler_flag(-Wimplicit-fallthrough HAS_IMPLICIT_FALLTHROUGH)
ENDIF()
-ADD_DEFINITIONS(-Os -Wall -Werror --std=gnu99 -Wmissing-declarations
-Wno-unknown-warning-option -Wno-format-truncation)
+ADD_DEFINITIONS(-Os -Wall -Werror --std=gnu99 -Wmissing-declarations
-Wno-unused-parameter -Wno-unused-but-set-parameter)
+IF(CMAKE_C_COMPILER_VERSION VERSION_GREATER 6)
+ add_definitions(-Wextra -Werror=implicit-function-declaration)
+ add_definitions(-Wformat -Werror=format-security
-Werror=format-nonliteral)
+ENDIF()
IF(HAS_IMPLICIT_FALLTHROUGH)
ADD_DEFINITIONS(-Wimplicit-fallthrough)
--
2.39.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[PATCH netifd 3/5] netifd: Do not return values in void function
These two functions return void, do not try to return a parameter.
Signed-off-by: Hauke Mehrtens
---
interface-event.c | 6 --
main.c| 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/interface-event.c b/interface-event.c
index a40f6dc..b03bfbc 100644
--- a/interface-event.c
+++ b/interface-event.c
@@ -49,8 +49,10 @@ run_cmd(const char *ifname, const char *device, enum
interface_event event,
int pid;
pid = fork();
- if (pid < 0)
- return task_complete(NULL, -1);
+ if (pid < 0) {
+ task_complete(NULL, -1);
+ return;
+ }
if (pid > 0) {
task.pid = pid;
diff --git a/main.c b/main.c
index 874dc8b..e5260b5 100644
--- a/main.c
+++ b/main.c
@@ -129,7 +129,8 @@ netifd_process_cb(struct uloop_process *proc, int ret)
np = container_of(proc, struct netifd_process, uloop);
netifd_delete_process(np);
- return np->cb(np, ret);
+ np->cb(np, ret);
+ return;
}
int
--
2.39.1
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[PATCH netifd 1/5] netifd: bridge: Fix format string position
This fixes the following compile error: error: format not a string literal, argument types not checked [-Werror=format-nonliteral] blobmsg_printf() has the following signature: int blobmsg_printf(struct blob_buf *buf, const char *name, const char *format, ...) Signed-off-by: Hauke Mehrtens --- bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge.c b/bridge.c index 7e61b9d..ae305e8 100644 --- a/bridge.c +++ b/bridge.c @@ -934,7 +934,7 @@ bridge_dump_port(struct blob_buf *b, struct bridge_vlan_port *port) bool tagged = !(port->flags & BRVLAN_F_UNTAGGED); bool pvid = (port->flags & BRVLAN_F_PVID); - blobmsg_printf(b, "%s%s%s%s\n", port->ifname, + blobmsg_printf(b, NULL, "%s%s%s%s\n", port->ifname, tagged || pvid ? ":" : "", tagged ? "t" : "", pvid ? "*" : ""); -- 2.39.1 ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[PATCH netifd 0/5] Fix some compiler warnings
This fixes some compiler warnings and activates -Wextra by default now. Hauke Mehrtens (5): netifd: bridge: Fix format string position netifd: Fix multiple -Wsign-compare warnings netifd: Do not return values in void function netifd: Explicitly zero initialize variables netifd: Activate -Wextra compile warnings CMakeLists.txt| 6 +- bonding.c | 2 +- bridge.c | 2 +- handler.c | 5 +++-- interface-event.c | 6 -- interface-ip.c| 2 +- main.c| 7 --- proto.c | 2 +- system-linux.c| 23 +-- ubus.c| 4 ++-- vlan.c| 4 ++-- wireless.c| 2 +- 12 files changed, 38 insertions(+), 27 deletions(-) -- 2.39.1 ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH 2/2] hack-5.15: add Aquantia PHY hwmon temperature clamp patch
On Sat, 18 Feb 2023 at 21:47, Enrico Mioso wrote:
>
>
>
>
> On Sat, 18 Feb 2023, Robert Marko wrote:
>
> > Date: Sat, 18 Feb 2023 12:45:34
> > From: Robert Marko
> > To: Enrico Mioso
> > Cc: openwrt-devel@lists.openwrt.org, Andre Valentin ,
> > Karol Przybylski
> > Subject: Re: [PATCH 2/2] hack-5.15: add Aquantia PHY hwmon temperature clamp
> > patch
> >
> > On Sat, 18 Feb 2023 at 00:58, Enrico Mioso wrote:
> >>
> >> This is needed to avoid failures in the thermal subsystem while using this
> >> driver via hwmon subsystem.
> >
> > This should be submitted upstream, we have enough hacks already and
> > you will get proper feedback from Guenter rather fast whether this is a bug
> > in hwmon or the driver needs fixups.
>
> Thanks for your review and feedback!
>
> I am in the process of discussing this change upstream; the problem seems not
> to be related to hwmon core, but my approach of clamping the value is not
> going to be accepted either.
> I'm being asked to simply change the -ERANGE returned value to -EINVAL, so I
> will do it independently of this openwrt patch.
Perfect, I see that Andrew chimed in as well.
Realistically it should probably be clamped to -40 to 108 as that is
what the industrial models are rated at.
>
> But I am supposed to set max a min limits directly to reasonable values.
> I have no clear idea yet on where to set them in DTS, any help or hint would
> be very apreciated.
I am not sure if it's possible via DTS or only via sysfs.
Regards,
Robert
>
> Enrico
> >
> > Regards,
> > Robert
> >>
> >> CC: Andre Valentin
> >> CC: Karol Przybylski
> >> Signed-off-by: Enrico Mioso
> >> ---
> >> ...-clamp-temperature-value-in-aqr_hwmo.patch | 30 +++
> >> 1 file changed, 30 insertions(+)
> >> create mode 100644
> >> target/linux/generic/hack-5.15/726-net-phy-aquantia-clamp-temperature-value-in-aqr_hwmo.patch
> >>
> >> diff --git
> >> a/target/linux/generic/hack-5.15/726-net-phy-aquantia-clamp-temperature-value-in-aqr_hwmo.patch
> >>
> >> b/target/linux/generic/hack-5.15/726-net-phy-aquantia-clamp-temperature-value-in-aqr_hwmo.patch
> >> new file mode 100644
> >> index 00..36f0b37130
> >> --- /dev/null
> >> +++
> >> b/target/linux/generic/hack-5.15/726-net-phy-aquantia-clamp-temperature-value-in-aqr_hwmo.patch
> >> @@ -0,0 +1,30 @@
> >> +From 7bfceb1036d2ccda7b8e1e177e834c1cea9f0858 Mon Sep 17 00:00:00 2001
> >> +From: Enrico Mioso
> >> +Date: Sat, 18 Feb 2023 00:27:55 +0100
> >> +Subject: [PATCH] net: phy: aquantia: clamp temperature value in
> >> aqr_hwmon_set
> >> +
> >> +This patch is still under evaluation and is not guaranteed to be correct,
> >> +therefore it is submitted here in hack form. :)
> >> +
> >> +Signed-off-by: Enrico Mioso
> >> +---
> >> + drivers/net/phy/aquantia_hwmon.c | 3 +--
> >> + 1 file changed, 1 insertion(+), 2 deletions(-)
> >> +
> >> +diff --git a/drivers/net/phy/aquantia_hwmon.c
> >> b/drivers/net/phy/aquantia_hwmon.c
> >> +index 19c4c280a6cd..6444055e720c 100644
> >> +--- a/drivers/net/phy/aquantia_hwmon.c
> >> b/drivers/net/phy/aquantia_hwmon.c
> >> +@@ -70,8 +70,7 @@ static int aqr_hwmon_set(struct phy_device *phydev, int
> >> reg, long value)
> >> + {
> >> + int temp;
> >> +
> >> +- if (value >= 128000 || value < -128000)
> >> +- return -ERANGE;
> >> ++ clamp_val(value, -128000, 128000);
> >> +
> >> + temp = value * 256 / 1000;
> >> +
> >> +--
> >> +2.39.2
> >> +
> >> --
> >> 2.39.2
> >>
> >>
> >> ___
> >> openwrt-devel mailing list
> >> openwrt-devel@lists.openwrt.org
> >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> >
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
