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

invalid character on device description passes from snmp_exporter to prometheus - invalid utf-8 label #279

Closed
lethalwp opened this issue Apr 6, 2018 · 14 comments · Fixed by #968

Comments

@lethalwp
Copy link

lethalwp commented Apr 6, 2018

Some polled devices were marked as down even when snmp_exporter?target=... worked.
the target error in prom was "invalid utf-8 label".

The problem was an invalid character on the interface description. (Notélé)

Host operating system: output of uname -a

Linux SBVCP-PROMETHEUS-01 3.10.0-514.21.1.el7.x86_64 #1 SMP Thu May 25 17:04:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

[root@SBVCP-PROMETHEUS-01 ~]# env | grep -i lang
LANG=en_US.UTF-8

snmp_exporter version: output of snmp_exporter -version

]# ./snmp_exporter --version
snmp_exporter, version 0.9.0 (branch: HEAD, revision: abb143a)
build user: root@8246f0b1908c
build date: 20180226-16:10:54
go version: go1.9.2

What device/snmpwalk OID are you using?

on ifAlias

See the attached file as an example of the error.
(some alias have been
snmptarget=sw-xxx&module=xxx.txt
removed)

I changed the description and it worked. SuperQ mentionned this should be catched by snmp_exporter.

@brian-brazil
Copy link
Contributor

This is a device bug, it is only permitted to have ASCII characters in a DisplayString such as ifAlias. You should report this to your vendor.

@SuperQ
Copy link
Member

SuperQ commented Apr 6, 2018

Device bug or not, this output needs to be filtered by the exporter to avoid exposing invalid data to Prometheus.

@RichiH
Copy link
Member

RichiH commented Apr 6, 2018

@SuperQ I agree with your sentiment, but I do wonder what it should be changed to. Drop the label silently? Strip down é to e? If yes, what about ☺?

@brian-brazil
Copy link
Contributor

The only correct action we can take is to fail the scrape, which is what we do. No invalid data enters Prometheus. Returning partial results or munging data which may for example cause non-uniqueness is a very bad idea semantically, it has to be all or nothing.

The SNMP exporter works with SNMP, if the other end doesn't implement SNMP then we can't support it.

@brian-brazil
Copy link
Contributor

Once #186 is in you could get the snmp exporter handle this as the arbitrary binary string that it is. There's only so much we can do with garbage data.

@SuperQ
Copy link
Member

SuperQ commented Apr 6, 2018

We should fail the scrape at the exporter, rather than emit "corrupt" invalid data over the wire.

@brian-brazil
Copy link
Contributor

We're using the Go client, so that's what we already do. Prometheus also has checks for this.

@SuperQ
Copy link
Member

SuperQ commented Apr 6, 2018

Except, as you can see by this report, it's not happening. The snmp_exporter is emitting corrupted non-utf8 data.

@brian-brazil
Copy link
Contributor

Ah, looks like we're using an older version of the go client. I did some work in this area about a year ago, and had presumed it had all propagated by now.

@SuperQ SuperQ reopened this Apr 6, 2018
@SuperQ SuperQ added the bug label Apr 6, 2018
@SuperQ
Copy link
Member

SuperQ commented Apr 6, 2018

Unrelated, I checked the output reported, what's getting passed to the exporter is iso-8859-1 encoding.

@SuperQ
Copy link
Member

SuperQ commented Apr 6, 2018

#281 to update the client vendoring.

@RichiH
Copy link
Member

RichiH commented Feb 25, 2021

@SuperQ just to deliberately think about it once.

@RichiH RichiH reopened this Feb 25, 2021
@luizluca
Copy link

luizluca commented May 7, 2021

This is a device bug, it is only permitted to have ASCII characters in a DisplayString such as ifAlias. You should report this to your vendor.

It's difficult when the "vendor" is Microsoft. Windows 2k12 reports ifAlias as a localized string. Using OctetString does the trick but the label is now mostly useless.

$ echo 436F6E6578E36F204C6F63616C2A2036 | xxd -ps -r | hexdump -C
00000000  43 6f 6e 65 78 e3 6f 20  4c 6f 63 61 6c 2a 20 36  |Conex.o Local* 6|
00000010
$ echo 436F6E6578E36F204C6F63616C2A2036 | xxd -ps -r | iconv -f iso-8859-1 ; echo
Conexão Local* 6
$ echo 436F6E6578E36F204C6F63616C2A2036 | xxd -ps -r | iconv -f iso-8859-1 | hexdump -c
0000000   C   o   n   e   x 303 243   o       L   o   c   a   l   *    
0000010   6                                                            
0000011

I tried to use regex replace to manually convert the hex bytestream back to a readable form. However, it looks like, understandably, it does not work with OctedString.

It also sends null terminated strings as ifDescr. However, in this case, nobody seems to bother (I only noticed directly getting metrics from a browser). Anyway, it would be nice to chomp it off. Check byte 0x58:

00000000  69 66 41 64 6d 69 6e 53  74 61 74 75 73 7b 69 66  |ifAdminStatus{if|
00000010  41 6c 69 61 73 3d 22 30  78 34 33 36 46 36 45 36  |Alias="0x436F6E6|
00000020  35 37 38 45 33 36 46 32  30 34 43 36 46 36 33 36  |578E36F204C6F636|
00000030  31 36 43 32 41 32 30 33  31 22 2c 69 66 44 65 73  |16C2A2031",ifDes|
00000040  63 72 3d 22 57 41 4e 20  4d 69 6e 69 70 6f 72 74  |cr="WAN Miniport|
00000050  20 28 4c 32 54 50 29 00  22 2c 69 66 49 6e 64 65  | (L2TP).",ifInde|
00000060  78 3d 22 32 22 2c 69 66  4e 61 6d 65 3d 22 74 75  |x="2",ifName="tu|
00000070  6e 6e 65 6c 5f 30 22 7d  20 31 0a                 |nnel_0"} 1.|
0000007b

@dswarbrick
Copy link
Contributor

dswarbrick commented Apr 9, 2023

DisplayString is effectively limited to 7-bit ASCII:

SNMPv2-TC:DisplayString ::= TEXTUAL-CONVENTION
    DISPLAY-HINT "255a"
    DESCRIPTION
            "Represents textual information taken from the NVT ASCII
            character set, as defined in pages 4, 10-11 of RFC 854...."

What we probably need to do is implement support for SnmpAdminString (defined in RFC 3411), which allows UTF-8 characters (and fix the generator README so that it doesn't say that UTF-8 is allowed for DisplayString).

There are almost certainly cases where vendors may be using even more funky encodings like Shift JIS, KOI-8 etc. to stuff illegal values into non-conformant SNMP engines, so that might mean that we have to provide some kludges to work around that - since it's probably easier than getting vendors to fix their broken MIBs.

The following minimal example shows that it would not take a lot of additional code to support input encoding translation to UTF-8:

import (
	"encoding/hex"
	"fmt"

	"golang.org/x/text/encoding/charmap"
	"golang.org/x/text/transform"
)

func main() {
	const s = "436F6E6578E36F204C6F63616C2A2036"
	octetstr, _ := hex.DecodeString(s)
	fmt.Printf("% x\n", octetstr)
	fmt.Printf("%s\n", octetstr)

	dec := charmap.ISO8859_1.NewDecoder()
	res, _, _ := transform.Bytes(dec, octetstr)
	fmt.Printf("% x\n", res)
	fmt.Printf("%s\n", res)
}
43 6f 6e 65 78 e3 6f 20 4c 6f 63 61 6c 2a 20 36
Conex�o Local* 6
43 6f 6e 65 78 c3 a3 6f 20 4c 6f 63 61 6c 2a 20 36
Conexão Local* 6

SuperQ added a commit that referenced this issue Aug 27, 2023
Avoid sending raw invalid UTF-8 to metrics by replacing invalid chars
with U+FFFD.

Fixes: #279

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this issue Aug 27, 2023
Avoid sending raw invalid UTF-8 to metrics by replacing invalid chars
with U+FFFD.

Fixes: #279

Signed-off-by: SuperQ <superq@gmail.com>
stephan-windischmann-sky pushed a commit to stephan-windischmann-sky/snmp_exporter that referenced this issue Oct 27, 2023
Avoid sending raw invalid UTF-8 to metrics by replacing invalid chars
with U+FFFD.

Fixes: prometheus#279

Signed-off-by: SuperQ <superq@gmail.com>
Signed-off-by: Stephan Windischmann <windi@Stephans-MacBook-Pro.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants