| Message ID | 20221129183756.657793-1-selva.nair@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,v2] pull-filter: ignore leading "spaces" in option names | expand |
Hi, On Tue, Nov 29, 2022 at 01:37:56PM -0500, selva.nair@gmail.com wrote: > From: Selva Nair <selva.nair@gmail.com> > > It seems sometimes comma-separated pulled options have > an offending leading space. Not sure whether that is an error, > but the change here matches the behaviour of option parsing. > > v2: fix typo in commit message. I have seen this, but I'd really prefer to fix the pushing side. Which options have you seen this with? I only observed with with "auth-token", which grows an extra leading space when being sent unsolicited. gert
Hi, On Tue, Nov 29, 2022 at 4:04 PM Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Tue, Nov 29, 2022 at 01:37:56PM -0500, selva.nair@gmail.com wrote: > > From: Selva Nair <selva.nair@gmail.com> > > > > It seems sometimes comma-separated pulled options have > > an offending leading space. Not sure whether that is an error, > > but the change here matches the behaviour of option parsing. > > > > v2: fix typo in commit message. > > I have seen this, but I'd really prefer to fix the pushing side. > > Which options have you seen this with? I only observed with with > "auth-token", which grows an extra leading space when being sent > unsolicited. > That's the only one I have seen too -- I made this on noticing that option parser is happy with the leading space. While fixing this at the server side would be great, it's arguable that this one too is needed to bring it line with option parsing. I'm fine either way. Selva
Am 29.11.2022 um 22:04 schrieb Gert Doering: > Hi, > > On Tue, Nov 29, 2022 at 01:37:56PM -0500, selva.nair@gmail.com wrote: >> From: Selva Nair <selva.nair@gmail.com> >> >> It seems sometimes comma-separated pulled options have >> an offending leading space. Not sure whether that is an error, >> but the change here matches the behaviour of option parsing. >> >> v2: fix typo in commit message. > I have seen this, but I'd really prefer to fix the pushing side. > > Which options have you seen this with? I only observed with with > "auth-token", which grows an extra leading space when being sent > unsolicited. I think since the client wants to ignore these option, we should add this to the client as well as you can other abuse this to work around a client that uses pull filter. Arne
Hi, On Wed, Nov 30, 2022 at 07:15:50AM +0100, Arne Schwabe wrote: > > Which options have you seen this with? I only observed with with > > "auth-token", which grows an extra leading space when being sent > > unsolicited. > > I think since the client wants to ignore these option, we should add > this to the client as well as you can other abuse this to work around a > client that uses pull filter. Yeah, right. I was more thinking about this being another tool in my toolchest (if I push this with an extra space, I can use pull-filter to just ignore this special version). But this is going to get more and more obscure, so I'll just merge Selva's patch and see if I can get the auth-token pushing fixed as well. gert
Hi, On Tue, Nov 29, 2022 at 01:37:56PM -0500, selva.nair@gmail.com wrote: > + /* skip leading spaces matching the behaviour of parse_line */ > + while(*line && space(*line)) I would actually prefer to use isspace(*line) here. space() is one of our special magic functions which returns true on 0-byte *or* isspace(), which won't have an effect (as *line checks for the 0 already), but will just confuse readers wrt "how is space() different from isspace()?". Can you send a v3? ACK on that :-) gert
diff --git a/src/openvpn/options.c b/src/openvpn/options.c index b7b34c9c..94cbd659 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -5385,6 +5385,12 @@ apply_pull_filter(const struct options *o, char *line) return true; } + /* skip leading spaces matching the behaviour of parse_line */ + while(*line && space(*line)) + { + line++; + } + for (f = o->pull_filter_list->head; f; f = f->next) { if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0)