Skip to content

enhancement(5039): bump up go version to 1.24, replace pbkdf2 with stdlib pbkdf2 #291

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.

Sign up for GitHub

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

Closed
kaanyalti wants to merge 1 commit into from
Closed

enhancement(5039): bump up go version to 1.24, replace pbkdf2 with stdlib pbkdf2 #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/elastic/elastic-agent-libs

go 1.22.12
go 1.24
Copy link
Copy Markdown
Member

@kruskall kruskall Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go 1.23 is still supported, we can't drop it

Copy link
Copy Markdown
Contributor

@simitt simitt Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we still need to support go.1.23 here then we won't be able to move from golang.org/x/crypto/pbkdf2 to the stdlib crypto implementations.

@kaanyalti @kruskall I wonder why the keystore even shows up as an issue in the downstream dependencies since it was excluded when in fips mode.

Copy link
Copy Markdown
Member

@mauri870 mauri870 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should refrain from updating the minimum Go version in libs until the respective update for Beats/Agent is well underway. Sometimes, we have important fixes in libs that need to be backported to Beats/Agent. If we update this too soon, we risk soft-locking ourselves out of those important changes.

That said, if we first update Beats/Agent to 1.24 and it goes well, I see no reason why we cannot make the move to update libs too.

Copy link
Copy Markdown
Member

@kruskall kruskall Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we still need to support go.1.23 here then we won't be able to move from golang.org/x/crypto/pbkdf2 to the stdlib crypto implementations.

we can use x/crypto for go <1.24 and stdlib for go >=1.24. See #289

I wonder why the keystore even shows up as an issue in the downstream dependencies since it was excluded when in fips mode.

If the package is imported then it is linked in the final binary (especially with DCE disabled). It's one of the reason the kerberos library was excluded in a way that avoided importing the package.

That said, if we first update Beats/Agent to 1.24 and it goes well, I see no reason why we cannot make the move to update libs too.

I'm not sure I agree :(
This is a library and it's not only used by beats/agent. The policy has usually been to support all the supported versions of go in libraries (currently 1.23 and 1.24)

Copy link
Copy Markdown
Member

@mauri870 mauri870 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a library and it's not only used by beats/agent. The policy has usually been to support all the supported versions of go in libraries (currently 1.23 and 1.24)

Fair point, agree with you on this.


require (
github.com/Microsoft/go-winio v0.5.2
Expand All @@ -20,7 +20,6 @@ require (
go.elastic.co/ecszap v1.0.2
go.elastic.co/go-licence-detector v0.6.0
go.uber.org/zap v1.27.0
golang.org/x/crypto v0.32.0
golang.org/x/net v0.34.0
golang.org/x/sys v0.29.0
golang.org/x/text v0.21.0
Expand Down Expand Up @@ -53,6 +52,7 @@ require (
go.opentelemetry.io/otel/metric v1.28.0 // indirect
go.opentelemetry.io/otel/trace v1.28.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/crypto v0.32.0 // indirect
golang.org/x/mod v0.17.0 // indirect
golang.org/x/sync v0.10.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
Expand Down
20 changes: 11 additions & 9 deletions keystore/file_keystore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"bytes"
"crypto/aes"
"crypto/cipher"
"crypto/pbkdf2"
"crypto/rand"
"crypto/sha512"
"encoding/base64"
Expand All @@ -32,8 +33,6 @@ import (
"runtime"
"sync"

"golang.org/x/crypto/pbkdf2"

"github.com/elastic/elastic-agent-libs/config"
"github.com/elastic/elastic-agent-libs/file"
)
Expand Down Expand Up @@ -81,7 +80,6 @@ func Factory(c *config.C, defaultPath string, strictPerms bool) (Keystore, error
c = config.NewConfig()
}
err := c.Unpack(&cfg)

if err != nil {
return nil, fmt.Errorf("could not read keystore configuration, err: %w", err)
}
Expand Down Expand Up @@ -330,7 +328,6 @@ func (k *FileKeystore) encrypt(reader io.Reader) (io.Reader, error) {
// randomly generate the salt and the initialization vector, this information will be saved
// on disk in the file as part of the header
iv, err := randomBytes(iVLength)

if err != nil {
return nil, err
}
Expand All @@ -342,7 +339,10 @@ func (k *FileKeystore) encrypt(reader io.Reader) (io.Reader, error) {

// Stretch the user provided key
password, _ := k.password.Get()
passwordBytes := k.hashPassword(password, salt)
passwordBytes, err := k.hashPassword(password, salt)
if err != nil {
return nil, err
}

// Select AES-256: because len(passwordBytes) == 32 bytes
block, err := aes.NewCipher(passwordBytes)
Expand Down Expand Up @@ -388,7 +388,10 @@ func (k *FileKeystore) decrypt(reader io.Reader) (io.Reader, error) {
encodedBytes := data[saltLength+iVLength:]

password, _ := k.password.Get()
passwordBytes := k.hashPassword(password, salt)
passwordBytes, err := k.hashPassword(password, salt)
if err != nil {
return nil, err
}

block, err := aes.NewCipher(passwordBytes)
if err != nil {
Expand Down Expand Up @@ -456,15 +459,14 @@ func (k *FileKeystore) ConfiguredPath() string {
return k.Path
}

func (k *FileKeystore) hashPassword(password, salt []byte) []byte {
return pbkdf2.Key(password, salt, iterationsCount, keyLength, sha512.New)
func (k *FileKeystore) hashPassword(password, salt []byte) ([]byte, error) {
return pbkdf2.Key(sha512.New, string(password), salt, iterationsCount, keyLength)
}

// randomBytes return a slice of random bytes of the defined length
func randomBytes(length int) ([]byte, error) {
r := make([]byte, length)
_, err := rand.Read(r)

if err != nil {
return nil, err
}
Expand Down