telegraf icon indicating copy to clipboard operation
telegraf copied to clipboard

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

Open davidaugustin1 opened this issue 3 years ago • 4 comments

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/[email protected]/doublestar.go:326 +0x37b
github.com/bmatcuk/doublestar/v3.Glob(...)
        /go/pkg/mod/github.com/bmatcuk/doublestar/[email protected]/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 avatar Feb 03 '22 10:02 davidaugustin1

same bug for me

aarnaud avatar Feb 03 '22 16:02 aarnaud

This could be PR9289(https://github.com/influxdata/telegraf/pull/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 ?

jjh74 avatar Feb 03 '22 18:02 jjh74

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.

davidaugustin1 avatar Feb 04 '22 11:02 davidaugustin1

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

  • If source starts with file:// sourcesToURLs() strips file:// and uses globpath. Globpath fails if source is for example file:///c:/Temp/some*.pem (globpath fails to match /c:/Temp/some*.pem). (Globpath works if we strip first /).
  • globpath compile converts slashes in path to backslashes (https://github.com/influxdata/telegraf/blob/master/internal/globpath/globpath.go#L38) but if path is matched backslashes are not converted back to slashes before converting path back to file:// url (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L412-L413)
  • getCert tries to use uri path in os.ReadFile (https://github.com/influxdata/telegraf/blob/master/plugins/inputs/x509_cert/x509_cert.go#L268) and this fails if path is for example /c:/Temp/some.pem. AFAIK for os.ReadFile you need to remove slash before drive letter (c:/Temp/some.pem).
  • I don't think globpath works with network share names (for example: file://localhost/Temp/some.pem).
  • globpath probably doesn't work with UNC paths (\\?\)

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 ?

jjh74 avatar Dec 27 '22 12:12 jjh74

@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.

powersj avatar Jan 09 '23 23:01 powersj

@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!

srebhan avatar Feb 06 '23 21:02 srebhan

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

aarnaud avatar Feb 08 '23 01:02 aarnaud

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"

jjh74 avatar Feb 08 '23 07:02 jjh74

@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 avatar Feb 08 '23 11:02 srebhan

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)... ;-)

srebhan avatar Feb 09 '23 10:02 srebhan

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

aarnaud avatar Feb 11 '23 19:02 aarnaud