Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[[inputs.x509_cert]] not working with filepath on windows #10580

Closed
davidaugustin1 opened this issue Feb 3, 2022 · 11 comments · Fixed by #12629
Closed

[[inputs.x509_cert]] not working with filepath on windows #10580

davidaugustin1 opened this issue Feb 3, 2022 · 11 comments · Fixed by #12629
Labels
bug unexpected problem or unintended behavior platform/windows

Comments

@davidaugustin1
Copy link

Relevent telegraf.conf

[agent]
  debug = true

[[inputs.x509_cert]]
  sources = ["file:///temp/mycertfile.crt"]

Logs from Telegraf

2022-02-03T10:01:12Z I! Starting Telegraf 1.21.3
2022-02-03T10:01:12Z I! Loaded inputs: x509_cert
2022-02-03T10:01:12Z I! Loaded aggregators:
2022-02-03T10:01:12Z I! Loaded processors:
2022-02-03T10:01:12Z W! ←[31mOutputs are not used in testing mode!←[0m
2022-02-03T10:01:12Z I! Tags enabled: host=NB336037
2022-02-03T10:01:12Z D! [agent] Initializing plugins
2022-02-03T10:01:12Z D! [agent] Starting service inputs
2022-02-03T10:01:12Z E! [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\temp\\mycertfile.crt": invalid character "\\" in host name
2022-02-03T10:01:12Z D! [agent] Stopping service inputs
2022-02-03T10:01:12Z D! [agent] Input channel closed
2022-02-03T10:01:12Z D! [agent] Stopped Successfully
2022-02-03T10:01:12Z E! [telegraf] Error running agent: input plugins recorded 1 errors

System info

Telegraf 1.21.3, Telegraf 1.18.3, Windows 10

Docker

No response

Steps to reproduce

  1. create config with working certifikate file on windows
  2. run telegraf with --test and --debug

...

Expected behavior

Check the certificate file and give an output

Actual behavior

Error: Couldn't parse/find the certificate file

Additional info

I've tested multiple ways to enter the filepath on windows.
See below for some of them I used in the config (didn't log everyone). I have grouped them by the error they show.
Some of the entries worked under 1.18.3, others are completely rubish but maybe they can help.

I know that Windows drive letters aren't possible with this Plugin at the moment. I've tested them anyway to see if i can find a workaround. The first tree entries shouldn't work, but telegraf finds the certificate and checks it, but missreads the drive letter as host. This however doesn't work with wildcards

sources = ['''C:\temp\mycertfile.crt'''] # works, validates chain, verification_error="x509: certificate is valid for myhost.domain.biz, myhost, not C"
sources = ["c:\\\\localhost/c$/temp/mycertfile.crt"] # works, validates chain, Output source=file://c://localhost/c$/temp/mycertfile.crt,verification=invalid verification_error="x509: certificate is valid for myhost.domain.biz, myhost, not c"
sources = ["d:\\\\localhost/c$/temp/mycertfile.crt"] # works, validates chain, Output source=file://d://localhost/c$/temp/mycertfile.crt,verification=invalid verification_error="x509: certificate is valid for myhost.domain.biz, myhost, not d"

sources = ['''C:\temp\certtest\crt\collector\*'''] # [inputs.x509_cert] Error in plugin: cannot get SSL cert 'file://C:/temp/certtest/crt/collector/*': open /temp/certtest/crt/collector/*: The filename, directory name, or volume label syntax is incorrect.

sources = ["//my.network/path/mycertfile.crt"] # works with 1.18.3 on 1.21.3: [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\\\my.network\\path\\mycertfile.crt": invalid character "\\" in host name
sources = ["/temp/mycertfile.crt"] # works with 1.18.3 on 1.21.3: [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\temp\\mycertfile.crt": invalid character "\\" in host name
sources = ["file:////my.network/path/mycertfile.crt"] # works with 1.18.3 on 1.21.3: [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\\\my.network\\path\\mycertfile.crt": invalid character "\\" in host name

sources = ["c:\\\\temp//mycertfile.crt"] # [inputs.x509_cert] Error in plugin: cannot get SSL cert 'file://c://temp//mycertfile.crt': open //temp//mycertfile.crt: The network path was not found.

sources = ['''file:\temp\mycertfile.crt'''] # [inputs.x509_cert] Error in plugin: cannot get SSL cert 'file:\temp\mycertfile.crt': open : The system cannot find the file specified.

sources = ['''\temp\mycertfile.crt'''] # [inputs.x509_cert] Error in plugin: cannot get SSL cert '%5Ctemp%5Cmycertfile.crt': unsupported scheme '' in location %5Ctemp%5Cmycertfile.crt

sources = [''':\temp\mycertfile.crt'''] # [telegraf] Error running agent: could not initialize input inputs.x509_cert: failed to parse cert location - parse ":\\temp\\mycertfile.crt": missing protocol scheme
sources = ["file:///temp/certtest/crt/collector/*.crt"] # [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\temp\\certtest\\crt\\collector\\mycertfile.crt": invalid character "\\" in host name
sources = ["file:///temp/mycertfile.crt"] # [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\temp\\mycertfile.crt": invalid character "\\" in host name
sources = ["file://temp/mycertfile.crt"] # [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://temp\\mycertfile.crt": invalid character "\\" in host name

sources = ["//?\\c:\\temp\\*.crt"] # finds the correct file but: [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\\\?\\c:\\temp\\mycertfile.crt": invalid character "\\" in host name
sources = ["//localhost/c$/temp/certtest/crt/collector/*.crt"] # finds the correct file but: [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\\\localhost\\c$\\temp\\certtest\\crt\\collector\\mycertfile.crt": invalid character "\\" in host name

sources = ["file:///my.network/path/mycertfile.crt"] # [inputs.x509_cert] could not find file: &{\my.network\path\mycertfile.crt false false  <nil>}

sources = ["file:////temp/mycertfile.crt"] # panic panic: runtime error: slice bounds out of range [25:24]

goroutine 87 [running]:
github.com/bmatcuk/doublestar/v3.GlobOS({0x55167a8, 0x7fc1020}, {0xc00012d428, 0x18})
        /go/pkg/mod/github.com/bmatcuk/doublestar/v3@v3.0.0/doublestar.go:326 +0x37b
github.com/bmatcuk/doublestar/v3.Glob(...)
        /go/pkg/mod/github.com/bmatcuk/doublestar/v3@v3.0.0/doublestar.go:291
github.com/influxdata/telegraf/internal/globpath.(*GlobPath).Match(0xc00012b1c0)
        /go/src/github.com/influxdata/telegraf/internal/globpath/globpath.go:53 +0xc9
github.com/influxdata/telegraf/plugins/inputs/x509_cert.(*X509Cert).collectCertURLs(0xc00019c500)
        /go/src/github.com/influxdata/telegraf/plugins/inputs/x509_cert/x509_cert.go:269 +0xae
github.com/influxdata/telegraf/plugins/inputs/x509_cert.(*X509Cert).Gather(0xc00019c500, {0x557db08, 0xc000591ce0})
        /go/src/github.com/influxdata/telegraf/plugins/inputs/x509_cert/x509_cert.go:290 +0x6b
github.com/influxdata/telegraf/agent.(*Agent).testRunInputs.func2(0xc000c9e960)
        /go/src/github.com/influxdata/telegraf/agent/agent.go:403 +0x2f8
created by github.com/influxdata/telegraf/agent.(*Agent).testRunInputs
        /go/src/github.com/influxdata/telegraf/agent/agent.go:372 +0xcd
@davidaugustin1 davidaugustin1 added the bug unexpected problem or unintended behavior label Feb 3, 2022
@aarnaud
Copy link
Contributor

aarnaud commented Feb 3, 2022

same bug for me

@jjh74
Copy link
Contributor

jjh74 commented Feb 3, 2022

This could be PR9289(#9289) related.
Does "file:///c:/Temp/some.pem" work ?
When telegraf reads the file but validation fails, does validation work if you set tls_server_name ?

@davidaugustin1
Copy link
Author

This could be PR9289(#9289) related. Does "file:///c:/Temp/some.pem" work ? When telegraf reads the file but validation fails, does validation work if you set tls_server_name ?

"file:///c:/Temp/some.pem" gives following errors:
On 1.13.0 and 1.18.3
E! [inputs.x509_cert] Error in plugin: cannot get SSL cert 'file:///c:/temp/mycertfile01.crt': open /c:/temp/mycertfile01.crt: The filename, directory name, or volume label syntax is incorrect.

On 1.19.0 and 1.21.3
[inputs.x509_cert] could not find file: &{\c:\temp\mycertfile01.crt false false <nil>}

If I try '''C:\temp\mycertfile02.crt''' it loads the certificate on 1.18.3/1.1910/1.21.3. Validation works when I set tls_server_name or server_name, otherwise it uses the drive letter as host

"/temp/mycertfile03.crt" is loaded by telegraf 1.13.0 and 1.18.3 and gets validated no mater I set tls_server_name or not (tls_server_name and server_name won't work with 1.13)

on 1.19.0 and later i get E! [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\temp\\mycertfile03.crt": invalid character "\\" in host name with and without tls_server_name/server_name

None of the paths above works with wildcards. If i replace the mycertfile0 with * I get the following errors:
E! [inputs.x509_cert] could not find file: &{\c:\temp\*3.crt true false <nil>}
E! [inputs.x509_cert] Error in plugin: cannot get file: failed to parse cert location - parse "file://\\temp\\mycertfile01.crt": invalid character "\\" in host name
E! [inputs.x509_cert] Error in plugin: cannot get SSL cert 'file://C:/temp/*2.crt': open /temp/*2.crt: The filename, directory name, or volume label syntax is incorrect.

@jjh74
Copy link
Contributor

jjh74 commented Dec 27, 2022

I took a look at this and looks like there are few issues with windows paths(file:// uris).

Using file:// urls (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L171 and https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L413 ) can also lead to certificate validation failing. If url.parse parses file:// url and finds some hostname component then this hostname is used in validation (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L186 and https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L99).

With file:// urls (u.Scheme == "file") should we just ignore uri hostname for validation ?

I'm not sure if network shares are supported (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L268) uses just url Path for os.ReadFile so if url is parsed to Host / Path then only Path is used. (Does golang os.ReadFile work with network shares if passed correct filename ?)

I can think of few changes:

  1. Don't use uri hostname for validation when u.Scheme == "file"
  2. Use globpath only when file:// source contains metachars (strings.ContainsAny(source, "*?[")). (If source has no metachars append source to c.locations (in sourcesToURLs())
  3. On windows(runtime.GOOS == "windows") strip slash (/c:/Temp/some1.pem -> c:/Temp/some1.pem) before reading file (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L268) if source starts with slash and driveletter: slash (/c:/)
  4. On windows: strip first slash (/c:/Temp/some*.pem -> c:/Temp/some*.pem before globpath.Compile (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L162) if source starts with slash and driveletter: slash (/c:/).

@srebhan do you have time to take a look if this looks reasonable ?

@powersj
Copy link
Contributor

powersj commented Jan 9, 2023

@jjh74,

Would you be willing to put up a PR? I think we would be happy to review something that improves this situation on Windows.

@srebhan
Copy link
Contributor

srebhan commented Feb 6, 2023

@davidaugustin1 and @aarnaud can you please test the binary built by CI in #12629!?! Windows paths can now be

  sources = [
     'file://C:\Windows\Temp\test.pem',
     "file://C:\\Windows\\Temp\\test.pem",
     "/Windows/Temp/test.pem",
     "file://C:/Windows/Temp/test.pem"
  ]

Please not the single-quotes for the first entry to avoid the need to escape the backslashes... Let me know if this fixes your issue!

@aarnaud
Copy link
Contributor

aarnaud commented Feb 8, 2023

I pretty sure it's fixed, but let me come back to confirm.
I will test the 4 syntax

@jjh74
Copy link
Contributor

jjh74 commented Feb 8, 2023

I'm not sure if "file://C:/Windows/Temp/test.pem" is exactly valid uri format (https://learn.microsoft.com/en-us/archive/blogs/ie/file-uris-in-windows) (probably needs three slashes after file: ).

@aarnaud can you also test if some of these work:

"file:///c:/Temp/some*.pem"
"file://c:/Temp/some*.pem"
"file:///Temp/some*.pem"
"file:///C:/Windows/Temp/test.pem"

@srebhan
Copy link
Contributor

srebhan commented Feb 8, 2023

@jjh74 so do you think we should not support "file://C:/Windows/Temp/test.pem" but "file:///C:/Windows/Temp/test.pem"? If so, please comment on the PR!

@srebhan
Copy link
Contributor

srebhan commented Feb 9, 2023

The PR is updated to handle both cases. @aarnaud if you can test and report back quickly we might be able to include this in v1.25.2 (due on Monday IIRC)... ;-)

@aarnaud
Copy link
Contributor

aarnaud commented Feb 11, 2023

Sorry for delay, I needed time to test on windows...
So everything working, I tested these

sources = ["file:///Folder with Space/certs/github.crt"]
sources = ['file://C:\Folder with Space\certs\github.crt']
sources = ["file://C:\\Folder with Space\\certs\\github.crt"]
sources = ["/Folder with Space/certs/github.crt"]
sources = ["file://c:/Folder with Space/certs/github.crt"]
sources = ["file:///c:/Folder with Space/certs/git*.crt"]
sources = ["file://c:/Folder with Space/certs/git*.crt"]
sources = ["file:///Folder with Space/certs/git*.crt"]
sources = ["file:///C:/Folder with Space/certs/github.crt"]

Ready for 1.25.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unexpected problem or unintended behavior platform/windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants