diff --git a/hscontrol/db/api_key.go b/hscontrol/db/api_key.go index b395c38f..41f16915 100644 --- a/hscontrol/db/api_key.go +++ b/hscontrol/db/api_key.go @@ -7,9 +7,9 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" "golang.org/x/crypto/bcrypt" "gorm.io/gorm" + "tailscale.com/util/rands" ) const ( @@ -23,9 +23,8 @@ const ( ) var ( - ErrAPIKeyFailedToParse = errors.New("failed to parse ApiKey") - ErrAPIKeyGenerationFailed = errors.New("failed to generate API key") - ErrAPIKeyInvalidGeneration = errors.New("generated API key failed validation") + ErrAPIKeyFailedToParse = errors.New("failed to parse ApiKey") + ErrAPIKeyGenerationFailed = errors.New("failed to generate API key") ) // CreateAPIKey creates a new [types.APIKey] in a user, and returns it. @@ -33,34 +32,10 @@ func (hsdb *HSDatabase) CreateAPIKey( expiration *time.Time, ) (string, *types.APIKey, error) { // Generate public prefix (12 chars) - prefix, err := util.GenerateRandomStringURLSafe(apiKeyPrefixLength) - if err != nil { - return "", nil, err - } - - // Validate prefix - if len(prefix) != apiKeyPrefixLength { - return "", nil, fmt.Errorf("%w: generated prefix has invalid length: expected %d, got %d", ErrAPIKeyInvalidGeneration, apiKeyPrefixLength, len(prefix)) - } - - if !isValidBase64URLSafe(prefix) { - return "", nil, fmt.Errorf("%w: generated prefix contains invalid characters", ErrAPIKeyInvalidGeneration) - } + prefix := rands.HexString(apiKeyPrefixLength) // Generate secret (64 chars) - secret, err := util.GenerateRandomStringURLSafe(apiKeyHashLength) - if err != nil { - return "", nil, err - } - - // Validate secret - if len(secret) != apiKeyHashLength { - return "", nil, fmt.Errorf("%w: generated secret has invalid length: expected %d, got %d", ErrAPIKeyInvalidGeneration, apiKeyHashLength, len(secret)) - } - - if !isValidBase64URLSafe(secret) { - return "", nil, fmt.Errorf("%w: generated secret contains invalid characters", ErrAPIKeyInvalidGeneration) - } + secret := rands.HexString(apiKeyHashLength) // Full key string (shown ONCE to user) keyStr := apiKeyPrefix + prefix + "-" + secret diff --git a/hscontrol/db/preauth_keys.go b/hscontrol/db/preauth_keys.go index fbdcd8c7..bff75ea7 100644 --- a/hscontrol/db/preauth_keys.go +++ b/hscontrol/db/preauth_keys.go @@ -8,9 +8,9 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" "golang.org/x/crypto/bcrypt" "gorm.io/gorm" + "tailscale.com/util/rands" "tailscale.com/util/set" ) @@ -94,33 +94,9 @@ func CreatePreAuthKey( now := time.Now().UTC() - prefix, err := util.GenerateRandomStringURLSafe(authKeyPrefixLength) - if err != nil { - return nil, err - } + prefix := rands.HexString(authKeyPrefixLength) - // Validate generated prefix (should always be valid, but be defensive) - if len(prefix) != authKeyPrefixLength { - return nil, fmt.Errorf("%w: generated prefix has invalid length: expected %d, got %d", ErrPreAuthKeyFailedToParse, authKeyPrefixLength, len(prefix)) - } - - if !isValidBase64URLSafe(prefix) { - return nil, fmt.Errorf("%w: generated prefix contains invalid characters", ErrPreAuthKeyFailedToParse) - } - - toBeHashed, err := util.GenerateRandomStringURLSafe(authKeyLength) - if err != nil { - return nil, err - } - - // Validate generated hash (should always be valid, but be defensive) - if len(toBeHashed) != authKeyLength { - return nil, fmt.Errorf("%w: generated hash has invalid length: expected %d, got %d", ErrPreAuthKeyFailedToParse, authKeyLength, len(toBeHashed)) - } - - if !isValidBase64URLSafe(toBeHashed) { - return nil, fmt.Errorf("%w: generated hash contains invalid characters", ErrPreAuthKeyFailedToParse) - } + toBeHashed := rands.HexString(authKeyLength) keyStr := authKeyPrefix + prefix + "-" + toBeHashed @@ -299,15 +275,14 @@ func parsePrefixedKey( return prefix, secret, nil } -// isValidBase64URLSafe checks if a string contains only base64 URL-safe characters. +// isValidBase64URLSafe reports whether s contains only base64 URL-safe +// characters (A-Za-z0-9-_). Key material is now generated as hex, a subset of +// this alphabet, so this accepts both current hex keys and any legacy keys +// still stored in the database. func isValidBase64URLSafe(s string) bool { - for _, c := range s { - if (c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9') && c != '-' && c != '_' { - return false - } - } - - return true + return !strings.ContainsFunc(s, func(c rune) bool { + return (c < 'A' || c > 'Z') && (c < 'a' || c > 'z') && (c < '0' || c > '9') && c != '-' && c != '_' + }) } func (hsdb *HSDatabase) GetPreAuthKey(key string) (*types.PreAuthKey, error) { diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 95c7b739..a4fb06f1 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -20,6 +20,7 @@ import ( "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" "golang.org/x/oauth2" + "tailscale.com/util/rands" ) const ( @@ -706,12 +707,7 @@ func (a *AuthProviderOIDC) renderRegistrationConfirmInterstitial( return } - csrf, err := util.GenerateRandomStringURLSafe(32) - if err != nil { - httpUserError(writer, fmt.Errorf("generating csrf token: %w", err)) - - return - } + csrf := rands.HexString(32) authReq.SetPendingConfirmation(&types.PendingRegistrationConfirmation{ UserID: user.ID, @@ -933,10 +929,7 @@ func getCookieName(baseName, value string) string { } func setCSRFCookie(w http.ResponseWriter, r *http.Request, name string) (string, error) { - val, err := util.GenerateRandomStringURLSafe(64) - if err != nil { - return val, err - } + val := rands.HexString(64) //nolint:gosec // G124: Secure set conditionally via r.TLS; HttpOnly + SameSite set below c := &http.Cookie{ diff --git a/hscontrol/state/ping.go b/hscontrol/state/ping.go index 9406e0e1..2f688b94 100644 --- a/hscontrol/state/ping.go +++ b/hscontrol/state/ping.go @@ -5,7 +5,7 @@ import ( "time" "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" + "tailscale.com/util/rands" ) const pingIDLength = 16 @@ -35,7 +35,7 @@ func newPingTracker() *pingTracker { // channel that receives the round-trip latency once the response // arrives. func (pt *pingTracker) register(nodeID types.NodeID) (string, <-chan time.Duration) { - pingID, _ := util.GenerateRandomStringDNSSafe(pingIDLength) + pingID := rands.HexString(pingIDLength) ch := make(chan time.Duration, 1) pt.mu.Lock() diff --git a/hscontrol/types/common.go b/hscontrol/types/common.go index 5e112800..86e42f28 100644 --- a/hscontrol/types/common.go +++ b/hscontrol/types/common.go @@ -11,7 +11,7 @@ import ( "sync/atomic" "time" - "github.com/juanfont/headscale/hscontrol/util" + "tailscale.com/util/rands" ) const ( @@ -37,12 +37,7 @@ const ( type AuthID string func NewAuthID() (AuthID, error) { - rid, err := util.GenerateRandomStringURLSafe(authIDRandomLength) - if err != nil { - return "", err - } - - return AuthID(authIDPrefix + rid), nil + return AuthID(authIDPrefix + rands.HexString(authIDRandomLength)), nil } func MustAuthID() AuthID { diff --git a/hscontrol/util/string.go b/hscontrol/util/string.go deleted file mode 100644 index bed2b868..00000000 --- a/hscontrol/util/string.go +++ /dev/null @@ -1,78 +0,0 @@ -package util - -import ( - "crypto/rand" - "encoding/base64" - "strings" -) - -// GenerateRandomBytes returns securely generated random bytes. -// It will return an error if the system's secure random -// number generator fails to function correctly, in which -// case the caller should not continue. -func GenerateRandomBytes(n int) ([]byte, error) { - bytes := make([]byte, n) - - // Note that err == nil only if we read len(b) bytes. - if _, err := rand.Read(bytes); err != nil { //nolint:noinlineerr - return nil, err - } - - return bytes, nil -} - -// GenerateRandomStringURLSafe returns a URL-safe, base64 encoded -// securely generated random string. -// It will return an error if the system's secure random -// number generator fails to function correctly, in which -// case the caller should not continue. -func GenerateRandomStringURLSafe(n int) (string, error) { - b, err := GenerateRandomBytes(n) - - return encodeRandomURLSafe(b, n, err) -} - -// encodeRandomURLSafe URL-safe base64-encodes b and truncates to n. It checks -// err first: on an RNG failure b is nil, so slicing the empty encoding would -// panic instead of returning the ("", err) the caller is promised. -func encodeRandomURLSafe(b []byte, n int, err error) (string, error) { - if err != nil { - return "", err - } - - return base64.RawURLEncoding.EncodeToString(b)[:n], nil -} - -// GenerateRandomStringDNSSafe returns a DNS-safe -// securely generated random string. -// It will return an error if the system's secure random -// number generator fails to function correctly, in which -// case the caller should not continue. -func GenerateRandomStringDNSSafe(size int) (string, error) { - var ( - str string - err error - ) - - for len(str) < size { - str, err = GenerateRandomStringURLSafe(size) - if err != nil { - return "", err - } - - str = strings.ToLower( - strings.ReplaceAll(strings.ReplaceAll(str, "_", ""), "-", ""), - ) - } - - return str[:size], nil -} - -func MustGenerateRandomStringDNSSafe(size int) string { - hash, err := GenerateRandomStringDNSSafe(size) - if err != nil { - panic(err) - } - - return hash -} diff --git a/hscontrol/util/string_random_test.go b/hscontrol/util/string_random_test.go deleted file mode 100644 index c697841b..00000000 --- a/hscontrol/util/string_random_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package util - -import ( - "bytes" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// TestEncodeRandomURLSafeChecksErrorFirst ensures the URL-safe random string -// encoder returns ("", err) on an RNG failure instead of slicing the empty -// base64 of nil bytes and panicking. -func TestEncodeRandomURLSafeChecksErrorFirst(t *testing.T) { - require.NotPanics(t, func() { - s, err := encodeRandomURLSafe(nil, 32, assert.AnError) - assert.Empty(t, s) - require.Error(t, err) - }) - - s, err := encodeRandomURLSafe(bytes.Repeat([]byte{0x1}, 32), 32, nil) - require.NoError(t, err) - assert.Len(t, s, 32) -} diff --git a/hscontrol/util/string_test.go b/hscontrol/util/string_test.go deleted file mode 100644 index f0b4c558..00000000 --- a/hscontrol/util/string_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package util - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGenerateRandomStringDNSSafe(t *testing.T) { - for range 100000 { - str, err := GenerateRandomStringDNSSafe(8) - require.NoError(t, err) - assert.Len(t, str, 8) - } -} diff --git a/hscontrol/util/util.go b/hscontrol/util/util.go index e61c6b9b..d0120e05 100644 --- a/hscontrol/util/util.go +++ b/hscontrol/util/util.go @@ -12,6 +12,7 @@ import ( "time" "tailscale.com/util/cmpver" + "tailscale.com/util/rands" ) // URL parsing errors. @@ -282,10 +283,5 @@ func GenerateRegistrationKey() (string, error) { registerKeyLength = 64 ) - randomPart, err := GenerateRandomStringURLSafe(registerKeyLength) - if err != nil { - return "", fmt.Errorf("generating registration key: %w", err) - } - - return registerKeyPrefix + randomPart, nil + return registerKeyPrefix + rands.HexString(registerKeyLength), nil } diff --git a/integration/derp_verify_endpoint_test.go b/integration/derp_verify_endpoint_test.go index 85818912..c3cc8837 100644 --- a/integration/derp_verify_endpoint_test.go +++ b/integration/derp_verify_endpoint_test.go @@ -6,7 +6,6 @@ import ( "strconv" "testing" - "github.com/juanfont/headscale/hscontrol/util" "github.com/juanfont/headscale/integration/dsic" "github.com/juanfont/headscale/integration/hsic" "github.com/juanfont/headscale/integration/integrationutil" @@ -17,14 +16,14 @@ import ( "tailscale.com/net/netmon" "tailscale.com/tailcfg" "tailscale.com/types/key" + "tailscale.com/util/rands" ) func TestDERPVerifyEndpoint(t *testing.T) { IntegrationSkip(t) // Generate random hostname for the headscale instance - hash, err := util.GenerateRandomStringDNSSafe(6) - require.NoError(t, err) + hash := rands.HexString(6) testName := "derpverify" hostname := fmt.Sprintf("hs-%s-%s", testName, hash) @@ -45,7 +44,8 @@ func TestDERPVerifyEndpoint(t *testing.T) { require.NoError(t, err) defer scenario.ShutdownAssertNoPanics(t) - derper, err := scenario.CreateDERPServer("head", + derper, err := scenario.CreateDERPServer( + "head", dsic.WithCACert(caHeadscale), dsic.WithVerifyClientURL(fmt.Sprintf("https://%s/verify", net.JoinHostPort(hostname, strconv.Itoa(headscalePort)))), ) diff --git a/integration/dockertestutil/config.go b/integration/dockertestutil/config.go index 1f77fede..692dc9de 100644 --- a/integration/dockertestutil/config.go +++ b/integration/dockertestutil/config.go @@ -6,8 +6,8 @@ import ( "strings" "time" - "github.com/juanfont/headscale/hscontrol/util" "github.com/ory/dockertest/v3" + "tailscale.com/util/rands" ) const ( @@ -46,7 +46,7 @@ func GenerateRunID() string { timestamp := now.Format(TimestampFormatRunID) // Add a short random hash to ensure uniqueness - randomHash := util.MustGenerateRandomStringDNSSafe(6) + randomHash := rands.HexString(6) return fmt.Sprintf("%s-%s", timestamp, randomHash) } diff --git a/integration/dsic/dsic.go b/integration/dsic/dsic.go index 442298a1..c165eee1 100644 --- a/integration/dsic/dsic.go +++ b/integration/dsic/dsic.go @@ -11,11 +11,11 @@ import ( "strings" "time" - "github.com/juanfont/headscale/hscontrol/util" "github.com/juanfont/headscale/integration/dockertestutil" "github.com/juanfont/headscale/integration/integrationutil" "github.com/ory/dockertest/v3" "github.com/ory/dockertest/v3/docker" + "tailscale.com/util/rands" ) const ( @@ -143,10 +143,7 @@ func New( networks []*dockertest.Network, opts ...Option, ) (*DERPServerInContainer, error) { - hash, err := util.GenerateRandomStringDNSSafe(dsicHashLength) - if err != nil { - return nil, err - } + hash := rands.HexString(dsicHashLength) // Include run ID in hostname for easier identification of which test run owns this container runID := dockertestutil.GetIntegrationRunID() diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index 337c4d85..e20e6466 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -36,6 +36,7 @@ import ( "gopkg.in/yaml.v3" "tailscale.com/tailcfg" "tailscale.com/util/mak" + "tailscale.com/util/rands" ) const ( @@ -172,7 +173,7 @@ func WithHostPortBindings(bindings map[string][]string) Option { // in the Docker container name. func WithTestName(testName string) Option { return func(hsic *HeadscaleInContainer) { - hash, _ := util.GenerateRandomStringDNSSafe(hsicHashLength) + hash := rands.HexString(hsicHashLength) hostname := fmt.Sprintf("hs-%s-%s", testName, hash) hsic.hostname = hostname @@ -331,10 +332,7 @@ func New( networks []*dockertest.Network, opts ...Option, ) (*HeadscaleInContainer, error) { - hash, err := util.GenerateRandomStringDNSSafe(hsicHashLength) - if err != nil { - return nil, err - } + hash := rands.HexString(hsicHashLength) // Include run ID in hostname for easier identification of which test run owns this container runID := dockertestutil.GetIntegrationRunID() @@ -499,7 +497,7 @@ func New( // dockertest isn't very good at handling containers that has already // been created, this is an attempt to make sure this container isn't // present. - err = pool.RemoveContainerByName(hsic.hostname) + err := pool.RemoveContainerByName(hsic.hostname) if err != nil { return nil, err } diff --git a/integration/scenario.go b/integration/scenario.go index c6d5c419..be19d78b 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -24,7 +24,6 @@ import ( v1 "github.com/juanfont/headscale/gen/go/headscale/v1" "github.com/juanfont/headscale/hscontrol/capver" "github.com/juanfont/headscale/hscontrol/types" - "github.com/juanfont/headscale/hscontrol/util" "github.com/juanfont/headscale/integration/dockertestutil" "github.com/juanfont/headscale/integration/dsic" "github.com/juanfont/headscale/integration/hsic" @@ -42,6 +41,7 @@ import ( "tailscale.com/envknob" "tailscale.com/util/mak" "tailscale.com/util/multierr" + "tailscale.com/util/rands" ) const ( @@ -197,7 +197,7 @@ func NewScenario(spec ScenarioSpec) (*Scenario, error) { pool.MaxWait = spec.MaxWait } - testHashPrefix := "hs-" + util.MustGenerateRandomStringDNSSafe(scenarioHashLength) + testHashPrefix := "hs-" + rands.HexString(scenarioHashLength) s := &Scenario{ controlServers: xsync.NewMap[string, ControlServer](), users: make(map[string]*User), @@ -1571,7 +1571,7 @@ func (s *Scenario) runMockOIDC(accessTTL time.Duration, users []mockoidc.MockUse portNotation := fmt.Sprintf("%d/tcp", port) - hash, _ := util.GenerateRandomStringDNSSafe(hsicOIDCMockHashLength) + hash := rands.HexString(hsicOIDCMockHashLength) hostname := "hs-oidcmock-" + hash @@ -1679,7 +1679,7 @@ func Webservice(s *Scenario, networkName string) (*dockertest.Resource, error) { // log.Fatalf("finding open port: %s", err) // } // portNotation := fmt.Sprintf("%d/tcp", port) - hash := util.MustGenerateRandomStringDNSSafe(hsicOIDCMockHashLength) + hash := rands.HexString(hsicOIDCMockHashLength) hostname := "hs-webservice-" + hash diff --git a/integration/tsic/tsic.go b/integration/tsic/tsic.go index a27c593f..553cc0a8 100644 --- a/integration/tsic/tsic.go +++ b/integration/tsic/tsic.go @@ -34,6 +34,7 @@ import ( "tailscale.com/types/key" "tailscale.com/types/netmap" "tailscale.com/util/multierr" + "tailscale.com/util/rands" "tailscale.com/wgengine/filter" ) @@ -314,10 +315,9 @@ func New( version string, opts ...Option, ) (*TailscaleInContainer, error) { - hash, err := util.GenerateRandomStringDNSSafe(tsicHashLength) - if err != nil { - return nil, err - } + hash := rands.HexString(tsicHashLength) + + var err error // Include run ID in hostname for easier identification of which test run owns this container runID := dockertestutil.GetIntegrationRunID() diff --git a/integration/tsric/tsric.go b/integration/tsric/tsric.go index 873b04fa..e160d855 100644 --- a/integration/tsric/tsric.go +++ b/integration/tsric/tsric.go @@ -15,11 +15,11 @@ import ( "os" "strings" - "github.com/juanfont/headscale/hscontrol/util" "github.com/juanfont/headscale/integration/dockertestutil" "github.com/juanfont/headscale/integration/integrationutil" "github.com/ory/dockertest/v3" "github.com/ory/dockertest/v3/docker" + "tailscale.com/util/rands" ) const ( @@ -129,7 +129,8 @@ func (t *TailscaleRustInContainer) buildEntrypoint() []string { commands = append(commands, "update-ca-certificates 2>/dev/null || true") - commands = append(commands, + commands = append( + commands, fmt.Sprintf(`export TS_CONTROL_URL=%q`, t.headscaleURL), // The tailscale crate refuses to run without this env gate; // see lib.rs in tailscale-rs. @@ -151,10 +152,9 @@ func New( pool *dockertest.Pool, opts ...Option, ) (*TailscaleRustInContainer, error) { - hash, err := util.GenerateRandomStringDNSSafe(tsricHashLength) - if err != nil { - return nil, err - } + hash := rands.HexString(tsricHashLength) + + var err error runID := dockertestutil.GetIntegrationRunID()