Skip to content

Commit

Permalink
security: addresses insecure randomness finding
Browse files Browse the repository at this point in the history 
See also: greenpau/caddy-security#265
  • Loading branch information
@greenpau
greenpau committed Dec 2, 2023
1 parent e626f47 commit ecd3725
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 85 deletions.
4 changes: 3 additions & 1 deletion Makefile
View file
Expand Up @@ -102,7 +102,9 @@ clean:
qtest: covdir
@echo "Perform quick tests ..."
@#time richgo test -v -coverprofile=.coverage/coverage.out internal/tag/*.go
@#time richgo test -v -coverprofile=.coverage/coverage.out internal/testutils/*.go
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/util/data/...
@time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/util/...
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run TestNewConfig ./*.go
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run TestNewServer ./*.go
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/registry/...
Expand Down Expand Up @@ -147,7 +149,7 @@ qtest: covdir
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/authproxy/...
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/identity/...
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out ./pkg/authn/backends/...
@time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run NewAPIKey ./pkg/identity/...
@#time richgo test $(VERBOSE) $(TEST) -coverprofile=.coverage/coverage.out -run NewAPIKey ./pkg/identity/...
@go tool cover -html=.coverage/coverage.out -o .coverage/coverage.html
@#go tool cover -func=.coverage/coverage.out | grep -v "100.0"
@go tool cover -func=.coverage/coverage.out
Expand Down
24 changes: 19 additions & 5 deletions internal/tests/random.go
View file
Expand Up @@ -15,8 +15,10 @@
package tests

import (
"crypto/rand"
"github.com/google/uuid"
"math/rand"
"io"
mathrand "math/rand"
"strings"
)

Expand All @@ -28,12 +30,24 @@ func NewID() string {
// NewRandomString returns a random string.
func NewRandomString(length int) string {
chars := []rune("abcdefghijklmnopqrstuvwxyz0123456789")
charsLen := byte(36)

if length == 0 {
length = 32
}
var b strings.Builder
for i := 0; i < length; i++ {
b.WriteRune(chars[rand.Intn(len(chars))])

b := make([]byte, length)
if _, err := io.ReadFull(rand.Reader, b); err != nil {
var sb strings.Builder
for i := 0; i < length; i++ {
sb.WriteRune(chars[mathrand.Intn(len(chars))])
}
return sb.String()
}
return b.String()

for i, char := range b {
b[i] = byte(chars[char%charsLen])
}

return string(b)
}
3 changes: 2 additions & 1 deletion pkg/identity/api_key.go
View file
Expand Up @@ -17,6 +17,7 @@ package identity
import (
"github.com/greenpau/go-authcrunch/pkg/errors"
"github.com/greenpau/go-authcrunch/pkg/requests"
"github.com/greenpau/go-authcrunch/pkg/util"
"golang.org/x/crypto/bcrypt"
"time"
)
Expand Down Expand Up @@ -80,7 +81,7 @@ func NewAPIKey(r *requests.Request) (*APIKey, error) {
}
p := &APIKey{
Comment: r.Key.Comment,
ID: GetRandomString(40),
ID: util.GetRandomString(40),
Prefix: r.Key.Prefix,
Payload: r.Key.Payload,
Usage: r.Key.Usage,
Expand Down
11 changes: 6 additions & 5 deletions pkg/identity/api_key_test.go
View file
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/greenpau/go-authcrunch/internal/tests"
"github.com/greenpau/go-authcrunch/pkg/errors"
"github.com/greenpau/go-authcrunch/pkg/requests"
"github.com/greenpau/go-authcrunch/pkg/util"
)

func TestNewAPIKey(t *testing.T) {
Expand All @@ -37,7 +38,7 @@ func TestNewAPIKey(t *testing.T) {
Key: requests.Key{
Usage: "api",
Comment: "jsmith-api-key",
Payload: GetRandomStringFromRange(54, 72),
Payload: util.GetRandomStringFromRange(54, 72),
},
},
want: map[string]interface{}{
Expand All @@ -53,7 +54,7 @@ func TestNewAPIKey(t *testing.T) {
Usage: "api",
Comment: "jsmith-api-key",
Disabled: true,
Payload: GetRandomStringFromRange(54, 72),
Payload: util.GetRandomStringFromRange(54, 72),
},
},
want: map[string]interface{}{
Expand Down Expand Up @@ -91,7 +92,7 @@ func TestNewAPIKey(t *testing.T) {
req: &requests.Request{
Key: requests.Key{
Comment: "jsmith-api-key",
Payload: GetRandomStringFromRange(54, 72),
Payload: util.GetRandomStringFromRange(54, 72),
Disabled: true,
},
},
Expand All @@ -104,7 +105,7 @@ func TestNewAPIKey(t *testing.T) {
Key: requests.Key{
Usage: "foo",
Comment: "jsmith-api-key",
Payload: GetRandomStringFromRange(54, 72),
Payload: util.GetRandomStringFromRange(54, 72),
Disabled: true,
},
},
Expand All @@ -116,7 +117,7 @@ func TestNewAPIKey(t *testing.T) {
req: &requests.Request{
Key: requests.Key{
Usage: "api",
Payload: GetRandomStringFromRange(54, 72),
Payload: util.GetRandomStringFromRange(54, 72),
Disabled: true,
},
},
Expand Down
3 changes: 2 additions & 1 deletion pkg/identity/database.go
View file
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/greenpau/go-authcrunch/internal/utils"
"github.com/greenpau/go-authcrunch/pkg/errors"
"github.com/greenpau/go-authcrunch/pkg/requests"
"github.com/greenpau/go-authcrunch/pkg/util"
"github.com/greenpau/versioned"
"io/ioutil"
"os"
Expand Down Expand Up @@ -566,7 +567,7 @@ func (db *Database) AddAPIKey(r *requests.Request) error {
if err != nil {
return errors.ErrAddAPIKey.WithArgs(r.Key.Usage, err)
}
s := GetRandomStringFromRange(72, 96)
s := util.GetRandomStringFromRange(72, 96)
failCount := 0
for {
hk, err := NewPassword(s)
Expand Down
3 changes: 2 additions & 1 deletion pkg/identity/mfa_token.go
View file
Expand Up @@ -36,6 +36,7 @@ import (

"github.com/greenpau/go-authcrunch/pkg/errors"
"github.com/greenpau/go-authcrunch/pkg/requests"
"github.com/greenpau/go-authcrunch/pkg/util"
)

// MfaTokenBundle is a collection of public keys.
Expand Down Expand Up @@ -99,7 +100,7 @@ func (b *MfaTokenBundle) Size() int {
// NewMfaToken returns an instance of MfaToken.
func NewMfaToken(req *requests.Request) (*MfaToken, error) {
p := &MfaToken{
ID: GetRandomString(40),
ID: util.GetRandomString(40),
CreatedAt: time.Now().UTC(),
Parameters: make(map[string]string),
Flags: make(map[string]bool),
Expand Down
3 changes: 2 additions & 1 deletion pkg/identity/public_key.go
View file
Expand Up @@ -24,6 +24,7 @@ import (
"fmt"
"github.com/greenpau/go-authcrunch/pkg/errors"
"github.com/greenpau/go-authcrunch/pkg/requests"
"github.com/greenpau/go-authcrunch/pkg/util"
"golang.org/x/crypto/openpgp"
"golang.org/x/crypto/ssh"
"strconv"
Expand Down Expand Up @@ -87,7 +88,7 @@ func (b *PublicKeyBundle) Size() int {
func NewPublicKey(r *requests.Request) (*PublicKey, error) {
p := &PublicKey{
Comment: r.Key.Comment,
ID: GetRandomString(40),
ID: util.GetRandomString(40),
Payload: r.Key.Payload,
Usage: r.Key.Usage,
CreatedAt: time.Now().UTC(),
Expand Down
55 changes: 0 additions & 55 deletions pkg/identity/random.go
View file

This file was deleted.

50 changes: 35 additions & 15 deletions pkg/util/random.go
View file
Expand Up @@ -15,9 +15,12 @@
package util

import (
"crypto/rand"
"encoding/base32"
"math/rand"
"time"
"io"
// "math"
"math/big"
mathrand "math/rand"
"unicode"
)

Expand All @@ -33,15 +36,32 @@ var charsetTable = &unicode.RangeTable{
LatinOffset: 1,
}

var seed *rand.Rand = rand.New(
rand.NewSource(time.Now().UnixNano()),
)
func genRandInt(i int) uint32 {
n, err := rand.Int(rand.Reader, big.NewInt(int64(i)))
if err != nil {
return uint32(mathrand.Intn(i))
}
//if n.Uint64() > math.MaxUint32+1 {
// return uint32(n.Uint64() & uint32(0xFFFFFFFF))
//}
return uint32(n.Uint64())
}

func gen(length int, charset string) string {
func gen(length uint32, charset string) string {
charsetLen := byte(len(charset))
b := make([]byte, length)
for i := range b {
b[i] = charset[seed.Intn(len(charset))]
if _, err := io.ReadFull(rand.Reader, b); err != nil {
// for i uint32 := 0; i < length; i++ {
for i := uint32(0); i < length; {
b[i] = charset[mathrand.Intn(len(charset))]
}
return string(b)
}

for i, char := range b {
b[i] = byte(charset[char%charsetLen])
}

return string(b)
}

Expand All @@ -50,17 +70,17 @@ func GetRandomString(i int) string {
if i < 1 {
i = 40
}
return gen(i, charset)
return gen(uint32(i), charset)
}

// GetRandomStringFromRange generates random string of a random length. The
// random lenght is bounded by a and b.
func GetRandomStringFromRange(a, b int) string {
var i int
var i uint32
if a > b {
i = rand.Intn(a-b) + b
i = genRandInt(a-b) + uint32(b)
} else {
i = rand.Intn(b-a) + a
i = genRandInt(b-a) + uint32(a)
}
return gen(i, charset)
}
Expand All @@ -75,11 +95,11 @@ func GetRandomEncodedStringFromRange(a, b int) string {
// GetRandomStringFromRangeWithCharset generates random string of a random length. The
// random lenght is bounded by a and b. The charset is provided.
func GetRandomStringFromRangeWithCharset(a, b int, cs string) string {
var i int
var i uint32
if a > b {
i = rand.Intn(a-b) + b
i = genRandInt(a-b) + uint32(b)
} else {
i = rand.Intn(b-a) + a
i = genRandInt(b-a) + uint32(a)
}
return gen(i, cs)
}
Expand Down

0 comments on commit ecd3725

Please sign in to comment.