Skip to content

Commit

Permalink
Update crl-checker to use math/rand/v2, generate RSA keys sometimes (#60
Browse files Browse the repository at this point in the history
)

This switches to math/rand/v2, and adds a bit more randomization while
we're here.

We randomly generate either an ECDSA or RSA key to ensure we split
issuance across all intermediates.

The random domain name setup is simplified by using a random Uint32
instead of a byte buffer.
  • Loading branch information
mcpherrinm authored Jun 24, 2024
1 parent ec51e0c commit b7910d3
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 17 deletions.
5 changes: 3 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ linters:
- unused
- wastedassign
linters-settings:
errcheck:
ignore: fmt:[FS]?[Pp]rint*,io:Write,os:Remove,net/http:Write,net:Write,encoding/binary:Write
gosec:
excludes:
- G404
2 changes: 1 addition & 1 deletion checker/earlyremoval/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"crypto/x509"
"log"
"math/big"
"math/rand"
"math/rand/v2"
"time"

"github.com/letsencrypt/boulder/crl/checker"
Expand Down
7 changes: 3 additions & 4 deletions checker/earlyremoval/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"crypto/x509"
"math/big"
"math/rand"
"math/rand/v2"
"testing"
"time"

Expand Down Expand Up @@ -68,9 +68,8 @@ func TestSample(t *testing.T) {
require.Empty(t, sample([]int{}, 999))

var data []int
// Generate a random array for tests. Insecure RNG is fine.
// #nosec G404
length := 100 + rand.Intn(300)
// Generate a random array for tests.
length := 100 + rand.IntN(300)
for i := 0; i < length; i++ {
data = append(data, i)
}
Expand Down
22 changes: 13 additions & 9 deletions churner/churner.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"encoding/pem"
"fmt"
"log"
mathrand "math/rand/v2"
"time"

"github.com/caddyserver/certmagic"
Expand Down Expand Up @@ -133,8 +135,7 @@ func (c *Churner) retryObtain(ctx context.Context, certPrivateKey crypto.Signer,

// Churn issues a certificate, revokes it, and stores the result in DynamoDB
func (c *Churner) Churn(ctx context.Context) error {
// Generate either an ecdsa or rsa private key
certPrivateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
certPrivateKey, err := randomKey()
if err != nil {
return err
}
Expand All @@ -161,16 +162,19 @@ func (c *Churner) Churn(ctx context.Context) error {
return c.db.AddCert(ctx, cert, time.Now())
}

// randomKey generates either an ecdsa or rsa private key
func randomKey() (crypto.Signer, error) {
if mathrand.IntN(2) == 0 {
return ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
} else {
return rsa.GenerateKey(rand.Reader, 2048)
}
}

// randDomains picks the domains to include on the certificate.
// We put a single domain which includes the current time and a random value.
func randDomains(baseDomain string) []string {
randomSuffix := make([]byte, 2)
_, err := rand.Read(randomSuffix)
if err != nil {
// Something has to go terribly wrong for this
panic(fmt.Sprintf("random read failed: %v", err))
}
domain := fmt.Sprintf("r%dz%x.%s", time.Now().Unix(), randomSuffix, baseDomain)
domain := fmt.Sprintf("r%dZ%x.%s", time.Now().Unix(), mathrand.Uint32(), baseDomain)
return []string{domain}
}

Expand Down
2 changes: 1 addition & 1 deletion churner/churner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestRandDomains(t *testing.T) {
base := "revoked.invalid"
domains := randDomains(base)
require.Len(t, domains, 1)
require.Regexp(t, regexp.MustCompile(`r[0-9]{10}z[0-9a-f]{4}\.`+regexp.QuoteMeta(base)), domains[0])
require.Regexp(t, regexp.MustCompile(`r[0-9]{10}Z[0-9a-f]+\.`+regexp.QuoteMeta(base)), domains[0])

second := randDomains(base)
require.NotEqual(t, domains, second, "Domains should be different each invocation")
Expand Down

0 comments on commit b7910d3

Please sign in to comment.