Project

General

Profile

Feature #3684

Windows version of strongswan does not allow wildcards in configuration

Added by Michał Skalski 8 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Category:
windows
Target version:
Start date:
Due date:
Estimated time:
Resolution:
Fixed

Description

When compiling on Windows using mingw compiler (mingw 5.03, gcc 7.3-win32 20180312) and compilation instructions on Windows resulting charon and swanctl binaries do not support include directives containing wildcards, i.e.

include strongswan.d/*.conf

This way configuration generated during build does not work at all, all files must be included directly without special characters.
It is caused by lack of support for glob() function and <glob.h> header.

Is there any workaround or any configure/compilation options which allow using wildcards?
Or implementation of globing enumerator is required?

depends-shlwapi.dll.png.jpg (47.6 KB) depends-shlwapi.dll.png.jpg Shlwapi.dll direct dependencies Michał Skalski, 02.02.2021 16:56
depends-charon-svc.exe.png (33.3 KB) depends-charon-svc.exe.png strongswan DLL dependencies Michał Skalski, 03.02.2021 18:14

Associated revisions

Revision 3de65f8d (diff)
Added by Tobias Brunner 8 months ago

enumerator: Implement globbing enumerator on Windows

We don't have glob() available there. This replacement should work
similarly for simple cases like `include conf.d/*.conf`.

Fixes #3684.

Revision e6a6fc33 (diff)
Added by Tobias Brunner 8 months ago

path: Also accept / as directory separator on Windows

This adds helper functions to determine the first or last directory separator
in a string and to check if a given character is a separator.

Paths starting with a separator are now also considered absolute on
Windows as these are rooted at the current drive.

Note that it's fine to use DIRECTORY_SEPARATOR when combining strings as
Windows API calls accept both forward and backward slashes as separators.

Co-authored-by: Michał Skalski <>

References #3684.

History

#1 Updated by Tobias Brunner 8 months ago

  • Tracker changed from Issue to Feature
  • Category changed from configuration to windows
  • Status changed from New to Feedback

Is there any workaround or any configure/compilation options which allow using wildcards?

Not really. I guess you could write a script that stuffs a bunch of files into a single one.

Or implementation of globing enumerator is required?

That would be the best solution. I've tried to implement that in the 3684-windows-glob branch. Let me know if that works for you.

#2 Updated by Michał Skalski 8 months ago

Unfortunately this does not work for files not in current directory. Probably needs some tweaking.

On the other hand I created independently implementation which is not using Windows API at all, only enumerator_create_directory() and simple filtering of results. This works well on Windows. Could you take a look at it? https://github.com/mskalski/strongswan/, branch 3684-generic-glob. I will create MR in free time.

My implementation has few shortcomings:
- it supports only wildcards in last part of path (for glob() function they can be any part of path), but for parsing strongswan.conf is usually enough.
- it supports only * and ? wildcards and does not support escaping (as \ is valid directory separator on Windows).
- does not support character ranges [a-z], but I think Windows version does not support them either.

This also allows using / characters as directory separators, as they are valid on Windows (and used on generated *.conf files).

#3 Updated by Tobias Brunner 8 months ago

Unfortunately this does not work for files not in current directory.

Works fine here if the right path separator is used. For Windows builds this is currently hard-coded to \ (see source:src/libstrongswan/utils/utils/path.h#L29), which our path manipulating utility functions then use internally (i.e. you have to change the include pattern in the default config templates). I guess this could be improved so that / is also supported, but that will need some work in all of these and maybe elsewhere (e.g. where DIRECTORY_SEPARATOR is used as mixing them is probably not a good idea). (Looks like you did some of this in your branch.)

On the other hand I created independently implementation which is not using Windows API at all, only enumerator_create_directory() and simple filtering of results.

Seems like a bit much code for something a system call can handle for us.

#4 Updated by Michał Skalski 8 months ago

Yes, you're right with \ as directory separators, I just used auto generated config files. But I think this generated configuration should work, as / separators are perfectly valid on Windows. In fact second commit in my branch 3684-generic-glob solves issue with slashes as separators.

I don't think mixed separators in path could be any problem for Windows API - from my observations it just works. And StrongSwan uses hardcoded \ on Windows just to build full paths, so it should not be a problem too.

And of course you're right with amount of code in my branch. Most of it is due to handling of / as directory separators (could use path_dirname() and path_basename() for it), and maybe also could use create_enumerator_filter().

On the other hand this approach has an advantage being generic, working on all non-glob() platforms, not only on Windows.

#5 Updated by Tobias Brunner 8 months ago

I don't think mixed separators in path could be any problem for Windows API - from my observations it just works.

Interesting, didn't know that's generally the case. But it's e.g. documented for CreateFile() (not explicitly for FindFirstFile(), which I used, but it still works there).

In fact second commit in my branch 3684-generic-glob solves issue with slashes as separators.

Thanks, I've pushed a modified version of that commit to my branch. I hope that's OK. Note that I don't think paths starting with / should be considered absolute on Windows. At least in my tests such paths didn't work, however, absolute paths that started with a drive letter worked (note that I used MSYS2 and not the toolchain packaged in Ubuntu).

#6 Updated by Michał Skalski 8 months ago

Note that I don't think paths starting with / should be considered absolute on Windows.

Yes, it's a little bit confusing, because paths starting from / are absolute in the meaning they cannot be appended to current directory to form full absolute path.
But they are absolute only on Current drive :-) (see https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcurrentdirectory and https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamea). And sure, path in form c:\... is absolute.

But on the other hand, path in form d:temp is NOT absolute, as it means temp path in current directory on drive d:. It's much more complicated.

So it all depends on what to do with information if path is absolute or not. I assumed it is usually used to decide if concatenate current directory with some other path or not, and it is the reason of change I made in path.c.

#7 Updated by Michał Skalski 8 months ago

I think we can assume when strongswan usually starts its current drive is the one it is installed on. So all paths in form /... mean absolute path on drive where strongswan is installed and referencing paths on other drives require adding theirs drive designator.

NB I pushed one more commit with simplified generic glob enumerator, could you look at it? Specifically do you think simplified function pattern_match() could go to <utils/string.h>?
That way create_enumerator_glob() would be quite simple.

#8 Updated by Tobias Brunner 8 months ago

So all paths in form /... mean absolute path on drive where strongswan is installed and referencing paths on other drives require adding theirs drive designator.

Interesting, I wasn't aware these kind of paths relative to the current drive are possible on Windows. So yeah, I suppose that makes paths starting with a separator absolute on Windows too (your assessment was absolutely correct that we use this to decide whether to combine a path with other paths). I've updated the commit.

NB I pushed one more commit with simplified generic glob enumerator, could you look at it? Specifically do you think simplified function pattern_match() could go to <utils/string.h>?

I really appreciate your efforts, but I don't think we currently have a need for it. It's basically a custom version of fnmatch(3), which we only use indirectly via glob(). And like glob(3) it's defined by newer POSIX standards (starting with POSIX.1-2001), so other than Windows there are no major platforms/libcs that don't support it (and MinGW maybe should really provide implementations of these too). By the way, PathMatchSpec() is apparently a (limited) Windows API version of fnmatch().

#9 Updated by Michał Skalski 8 months ago

OK, so if the only problematic system is Windows then there is no need for generic implementation. The only benefit of using generic approach is more control over accepted patterns, more intuitive error messages when pattern is not supported and maybe easier future extending support for more sophisticated patterns. I will leave my branch on github for reference though.

Worth of noting is (as I observed) that patterns on Windows also do not support wildcard characters in directory part nor character ranges (i.e. [A-z]) as posix glob() function does.
When trying use wildcards in directory part GetLastError() returns 123 (ERROR_INVALID_NAME). Character ranges are matched directly.

(and MinGW maybe should really provide implementations of these too).

I don't think mingw would provide own implementation of glob() available for windows applications, as the whole point of compiling with mingw is to use Microsoft's implementation of libc - msvcrt. It probably supports glob() only when compiling programs to be a part of msys -- when linked with msys-2.0.dll C library.

I tested 3864-windows-glob branch and it works correctly with autogenerated configuration for charon / swanctl, so I think it can be merged to master.

#10 Updated by Michał Skalski 8 months ago

PathMatchSpec() is apparently a (limited) Windows API version of fnmatch().

One more note: using this function would cause inclusion of many dependent libraries and enlarge memory footprint. So I think using shlwapi.dll only for PathMatchSpec() is undesirable :-) -- see below:
Shlwapi.dll direct dependencies.

And thank you very much for your patience and help.

#11 Updated by Tobias Brunner 8 months ago

  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Target version set to 5.9.2
  • Resolution set to Fixed

Worth of noting is (as I observed) that patterns on Windows also do not support wildcard characters in directory part nor character ranges (i.e. [A-z]) as posix glob() function does.
When trying use wildcards in directory part GetLastError() returns 123 (ERROR_INVALID_NAME). Character ranges are matched directly.

Correct, but I guess that's consistent with the platform (in cmd.exe you can't use such wildcards with e.g. dir).

(and MinGW maybe should really provide implementations of these too).

I don't think mingw would provide own implementation of glob() available for windows applications, as the whole point of compiling with mingw is to use Microsoft's implementation of libc - msvcrt. It probably supports glob() only when compiling programs to be a part of msys -- when linked with msys-2.0.dll C library.

You're right of course, I confused this with Cygwin's approach of providing a POSIX compatibility layer.

I tested 3864-windows-glob branch and it works correctly with autogenerated configuration for charon / swanctl, so I think it can be merged to master.

Thanks for testing, I've pushed the commits to master.

One more note: using this function would cause inclusion of many dependent libraries and enlarge memory footprint. So I think using shlwapi.dll only for PathMatchSpec() is undesirable :-)

Wouldn't the physical memory pages occupied by these DLLs be shared between multiple processes (I imagine many of these are used also by other processes)?

#12 Updated by Michał Skalski 8 months ago

Correct, but I guess that's consistent with the platform (in cmd.exe you can't use such wildcards with e.g. dir).

It's not so consistent. For example you cannot use directly / separators (you must quote them), on the other hand \ separators don't have to be quoted, but this behavior is due to backwards compatibility.

So for cmd.exe first command works and finds all files and directories of c:\windows directory, the second and third one not:

c:\>dir \windows\*.*
 Wolumin w stacji C nie ma etykiety.
 Numer seryjny woluminu: A0ED-DDE0

 Katalog: c:\windows

2021-01-10  01:42    <DIR>          .
2021-01-10  01:42    <DIR>          ..
2009-07-14  06:32    <DIR>          addins
2017-02-05  04:31    <DIR>          AppCompat
2020-01-25  09:49    <DIR>          AppPatch
2010-11-21  04:24            71 168 bfsvc.exe
2009-07-14  06:32    <DIR>          Boot
...

c:\>dir /windows/*.*
Niepoprawny format parametru - "windows".

c:\>dir "/windows/*.*" 
 Wolumin w stacji C nie ma etykiety.
 Numer seryjny woluminu: A0ED-DDE0

 Katalog: c:\windows

Nie można odnaleźć pliku.

Note the second command treats / as option, third just doesn't find any file (translated messages are "Incorrect parameter - windows" and "File not found." respectively). Substituting in third the last separator / to \ makes this command working.
Note also that pattern *.* finds also all files without dot in name. On the other hand pattern *. finds only files without extension, but it should be documented elsewhere. I'm not sure, but I think PathMatchSpec() matches the same way.

It's also worth noting that cmd.exe uses FindFirstFile() and other searching functions directly from kernel32.dll and does not use shlwapi.dll at all (the same as your enumerator_create_glob() implementation does).

Wouldn't the physical memory pages occupied by these DLLs be shared between multiple processes (I imagine many of these are used also by other processes)?

Sure, but only text (code) can be shared, but most of used by DLL memory contains data structures, so rather unique for every process, not shared. And loading these extra DLLs takes extra unnecessary time.
I made as an exercise dependency list of charon-svc (and libcharon.dll and libstrongswan.dll), they are rather small, so adding PathMatchSpec() to them would be just a waste:
strongswan DLL dependencies

Thanks again for your time.

Also available in: Atom PDF