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

[receiver/sqlserver] x509 negativeserial issue #38141

Merged

Conversation

sincejune
Copy link
Contributor

Description

SQL Server receiver now requires a positive serial number for X509 certificates due to the Go 1.23 adoption in #38099

Link to tracking issue

n/a

Testing

n/a

Documentation

n/a

@sincejune
Copy link
Contributor Author

SQL Server docker users may run into an issue that the collector fails to parse certificate from server due to x509: negative serial number. That's because we adopted Go 1.23 starting from contrib v0.121.0:

Before Go 1.23, ParseCertificate accepted certificates with negative serial numbers.
This behavior can be restored by including "x509negativeserial=1" in the GODEBUG environment variable.

references:

  1. https://pkg.go.dev/crypto/x509#ParseCertificate
  2. Default TLS cert uses negative serial number microsoft/mssql-docker#895

@sincejune
Copy link
Contributor Author

This change is actually not in the release notes of Go 1.23.
Since the SQL Server receiver uses the underlying scraperhelper dependency (which raises negative serial number issue) in core repo, should we consider adding a note at the project level?

cc @atoulme @bogdandrutu @mx-psi @songy23

@MovieStoreGuy
Copy link
Contributor

This change is actually not in the release notes of Go 1.23. Since the SQL Server receiver uses the underlying scraperhelper dependency (which raises negative serial number issue) in core repo, should we consider adding a note at the project level?

cc @atoulme @bogdandrutu @mx-psi @songy23

So this raises an interesting question, do we raise a breaking change for each component that would be using the impacted package, or raise it a breaking change for the project.

I'll delegate that to @open-telemetry/collector-approvers to see what they say.

@atoulme atoulme merged commit d2a9385 into open-telemetry:main Mar 4, 2025
156 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants