Skip to content

Commit

Permalink
ratelimit: Remove legacy registrations per IP implementation (#7760)
Browse files Browse the repository at this point in the history
Part of #7671
  • Loading branch information
beautifulentropy authored Nov 19, 2024
1 parent 65de9fb commit a8cdaf8
Show file tree
Hide file tree
Showing 22 changed files with 1,089 additions and 1,780 deletions.
33 changes: 13 additions & 20 deletions cmd/contact-auditor/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package notmain
import (
"context"
"fmt"
"net"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -133,37 +132,31 @@ func (tc testCtx) addRegistrations(t *testing.T) {
"e":"AQAB"
}`)

initialIP, err := net.ParseIP("127.0.0.1").MarshalText()
test.AssertNotError(t, err, "Couldn't create initialIP")

regA = &corepb.Registration{
Id: 1,
Contact: []string{emailA},
Key: jsonKeyA,
InitialIP: initialIP,
Id: 1,
Contact: []string{emailA},
Key: jsonKeyA,
}
regB = &corepb.Registration{
Id: 2,
Contact: []string{emailB},
Key: jsonKeyB,
InitialIP: initialIP,
Id: 2,
Contact: []string{emailB},
Key: jsonKeyB,
}
regC = &corepb.Registration{
Id: 3,
Contact: []string{emailC},
Key: jsonKeyC,
InitialIP: initialIP,
Id: 3,
Contact: []string{emailC},
Key: jsonKeyC,
}
// Reg D has a `tel:` contact ACME URL
regD = &corepb.Registration{
Id: 4,
Contact: []string{tel},
Key: jsonKeyD,
InitialIP: initialIP,
Id: 4,
Contact: []string{tel},
Key: jsonKeyD,
}

// Add the four test registrations
ctx := context.Background()
var err error
regA, err = tc.ssa.NewRegistration(ctx, regA)
test.AssertNotError(t, err, "Couldn't store regA")
regB, err = tc.ssa.NewRegistration(ctx, regB)
Expand Down
17 changes: 3 additions & 14 deletions cmd/expiration-mailer/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"errors"
"fmt"
"math/big"
"net"
"strings"
"testing"
"text/template"
Expand Down Expand Up @@ -462,20 +461,10 @@ func TestFindExpiringCertificates(t *testing.T) {
}

func makeRegistration(sac sapb.StorageAuthorityClient, id int64, jsonKey []byte, contacts []string) (*corepb.Registration, error) {
var ip [4]byte
_, err := rand.Reader.Read(ip[:])
if err != nil {
return nil, err
}
ipText, err := net.IP(ip[:]).MarshalText()
if err != nil {
return nil, fmt.Errorf("formatting IP address: %s", err)
}
reg, err := sac.NewRegistration(context.Background(), &corepb.Registration{
Id: id,
Contact: contacts,
Key: jsonKey,
InitialIP: ipText,
Id: id,
Contact: contacts,
Key: jsonKey,
})
if err != nil {
return nil, fmt.Errorf("storing registration: %s", err)
Expand Down
33 changes: 13 additions & 20 deletions cmd/id-exporter/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"encoding/base64"
"fmt"
"math/big"
"net"
"os"
"testing"
"time"
Expand Down Expand Up @@ -276,38 +275,32 @@ func (tc testCtx) addRegistrations(t *testing.T) {
"e":"AQAB"
}`)

initialIP, err := net.ParseIP("127.0.0.1").MarshalText()
test.AssertNotError(t, err, "Couldn't create initialIP")

// Regs A through C have `mailto:` contact ACME URL's
regA = &corepb.Registration{
Id: 1,
Contact: []string{emailA},
Key: jsonKeyA,
InitialIP: initialIP,
Id: 1,
Contact: []string{emailA},
Key: jsonKeyA,
}
regB = &corepb.Registration{
Id: 2,
Contact: []string{emailB},
Key: jsonKeyB,
InitialIP: initialIP,
Id: 2,
Contact: []string{emailB},
Key: jsonKeyB,
}
regC = &corepb.Registration{
Id: 3,
Contact: []string{emailC},
Key: jsonKeyC,
InitialIP: initialIP,
Id: 3,
Contact: []string{emailC},
Key: jsonKeyC,
}
// Reg D has a `tel:` contact ACME URL
regD = &corepb.Registration{
Id: 4,
Contact: []string{tel},
Key: jsonKeyD,
InitialIP: initialIP,
Id: 4,
Contact: []string{tel},
Key: jsonKeyD,
}

// Add the four test registrations
ctx := context.Background()
var err error
regA, err = tc.ssa.NewRegistration(ctx, regA)
test.AssertNotError(t, err, "Couldn't store regA")
regB, err = tc.ssa.NewRegistration(ctx, regB)
Expand Down
3 changes: 0 additions & 3 deletions core/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ type Registration struct {
// Agreement with terms of service
Agreement string `json:"agreement,omitempty"`

// InitialIP is the IP address from which the registration was created
InitialIP net.IP `json:"initialIp"`

// CreatedAt is the time the registration was created.
CreatedAt *time.Time `json:"createdAt,omitempty"`

Expand Down
8 changes: 4 additions & 4 deletions db/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func TestTableFromQuery(t *testing.T) {
expectedTable string
}{
{
query: "SELECT id, jwk, jwk_sha256, contact, agreement, initialIP, createdAt, LockCol, status FROM registrations WHERE jwk_sha256 = ?",
query: "SELECT id, jwk, jwk_sha256, contact, agreement, createdAt, LockCol, status FROM registrations WHERE jwk_sha256 = ?",
expectedTable: "registrations",
},
{
Expand All @@ -134,15 +134,15 @@ func TestTableFromQuery(t *testing.T) {
expectedTable: "authz2",
},
{
query: "insert into `registrations` (`id`,`jwk`,`jw k_sha256`,`contact`,`agreement`,`initialIp`,`createdAt`,`LockCol`,`status`) values (null,?,?,?,?,?,?,?,?);",
query: "insert into `registrations` (`id`,`jwk`,`jw k_sha256`,`contact`,`agreement`,`createdAt`,`LockCol`,`status`) values (null,?,?,?,?,?,?,?,?);",
expectedTable: "`registrations`",
},
{
query: "update `registrations` set `jwk`=?, `jwk_sh a256`=?, `contact`=?, `agreement`=?, `initialIp`=?, `createdAt`=?, `LockCol` =?, `status`=? where `id`=? and `LockCol`=?;",
query: "update `registrations` set `jwk`=?, `jwk_sh a256`=?, `contact`=?, `agreement`=?, `createdAt`=?, `LockCol` =?, `status`=? where `id`=? and `LockCol`=?;",
expectedTable: "`registrations`",
},
{
query: "SELECT COUNT(*) FROM registrations WHERE initialIP = ? AND ? < createdAt AND createdAt <= ?",
query: "SELECT COUNT(*) FROM registrations WHERE ? < createdAt AND createdAt <= ?",
expectedTable: "registrations",
},
{
Expand Down
11 changes: 0 additions & 11 deletions grpc/pb-marshalling.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,6 @@ func RegistrationToPB(reg core.Registration) (*corepb.Registration, error) {
if err != nil {
return nil, err
}
ipBytes, err := reg.InitialIP.MarshalText()
if err != nil {
return nil, err
}
var contacts []string
// Since the default value of corepb.Registration.Contact is a slice
// we need a indicator as to if the value is actually important on
Expand All @@ -248,7 +244,6 @@ func RegistrationToPB(reg core.Registration) (*corepb.Registration, error) {
Contact: contacts,
ContactsPresent: contactsPresent,
Agreement: reg.Agreement,
InitialIP: ipBytes,
CreatedAt: createdAt,
Status: string(reg.Status),
}, nil
Expand All @@ -260,11 +255,6 @@ func PbToRegistration(pb *corepb.Registration) (core.Registration, error) {
if err != nil {
return core.Registration{}, err
}
var initialIP net.IP
err = initialIP.UnmarshalText(pb.InitialIP)
if err != nil {
return core.Registration{}, err
}
var createdAt *time.Time
if !core.IsAnyNilOrZero(pb.CreatedAt) {
c := pb.CreatedAt.AsTime()
Expand All @@ -289,7 +279,6 @@ func PbToRegistration(pb *corepb.Registration) (core.Registration, error) {
Key: &key,
Contact: contacts,
Agreement: pb.Agreement,
InitialIP: initialIP,
CreatedAt: createdAt,
Status: core.AcmeStatus(pb.Status),
}, nil
Expand Down
2 changes: 0 additions & 2 deletions grpc/pb-marshalling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ func TestRegistration(t *testing.T) {
Key: &key,
Contact: &contacts,
Agreement: "yup",
InitialIP: net.ParseIP("1.1.1.1"),
CreatedAt: &createdAt,
Status: core.StatusValid,
}
Expand Down Expand Up @@ -212,7 +211,6 @@ func TestRegistration(t *testing.T) {
Key: &key,
Contact: &contacts,
Agreement: "yup",
InitialIP: net.ParseIP("1.1.1.1"),
CreatedAt: nil,
Status: core.StatusValid,
}
Expand Down
12 changes: 0 additions & 12 deletions mocks/sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"crypto/x509"
"errors"
"math/rand/v2"
"net"
"os"
"time"

Expand Down Expand Up @@ -113,7 +112,6 @@ func (sa *StorageAuthorityReadOnly) GetRegistration(_ context.Context, req *sapb
return goodReg, nil
}

goodReg.InitialIP, _ = net.ParseIP("5.6.7.8").MarshalText()
goodReg.CreatedAt = timestamppb.New(time.Date(2003, 9, 27, 0, 0, 0, 0, time.UTC))
return goodReg, nil
}
Expand Down Expand Up @@ -323,16 +321,6 @@ func (sa *StorageAuthorityReadOnly) CountCertificatesByNames(_ context.Context,
return &sapb.CountByNames{}, nil
}

// CountRegistrationsByIP is a mock
func (sa *StorageAuthorityReadOnly) CountRegistrationsByIP(_ context.Context, _ *sapb.CountRegistrationsByIPRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
return &sapb.Count{}, nil
}

// CountRegistrationsByIPRange is a mock
func (sa *StorageAuthorityReadOnly) CountRegistrationsByIPRange(_ context.Context, _ *sapb.CountRegistrationsByIPRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
return &sapb.Count{}, nil
}

// CountOrders is a mock
func (sa *StorageAuthorityReadOnly) CountOrders(_ context.Context, _ *sapb.CountOrdersRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
return &sapb.Count{}, nil
Expand Down
43 changes: 2 additions & 41 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,47 +386,10 @@ type finalizationCAACheckEvent struct {
Rechecked int `json:",omitempty"`
}

// noRegistrationID is used for the regID parameter to GetThreshold when no
// registration-based overrides are necessary.
const noRegistrationID = -1

// registrationCounter is a type to abstract the use of `CountRegistrationsByIP`
// or `CountRegistrationsByIPRange` SA methods.
type registrationCounter func(context.Context, *sapb.CountRegistrationsByIPRequest, ...grpc.CallOption) (*sapb.Count, error)

// checkRegistrationIPLimit checks a specific registraton limit by using the
// provided registrationCounter function to determine if the limit has been
// exceeded for a given IP or IP range
func (ra *RegistrationAuthorityImpl) checkRegistrationIPLimit(ctx context.Context, limit ratelimit.RateLimitPolicy, ip net.IP, counter registrationCounter) error {
now := ra.clk.Now()
count, err := counter(ctx, &sapb.CountRegistrationsByIPRequest{
Ip: ip,
Range: &sapb.Range{
Earliest: timestamppb.New(limit.WindowBegin(now)),
Latest: timestamppb.New(now),
},
})
if err != nil {
return err
}

threshold, overrideKey := limit.GetThreshold(ip.String(), noRegistrationID)
if count.Count >= threshold {
return berrors.RegistrationsPerIPAddressError(0, "too many registrations for this IP")
}
if overrideKey != "" {
// We do not support overrides for the NewRegistrationsPerIPRange limit.
utilization := float64(count.Count+1) / float64(threshold)
ra.rlOverrideUsageGauge.WithLabelValues(ratelimit.RegistrationsPerIP, overrideKey).Set(utilization)
}

return nil
}

// NewRegistration constructs a new Registration from a request.
func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, request *corepb.Registration) (*corepb.Registration, error) {
// Error if the request is nil, there is no account key or IP address
if request == nil || len(request.Key) == 0 || len(request.InitialIP) == 0 {
if request == nil || len(request.Key) == 0 {
return nil, errIncompleteGRPCRequest
}

Expand Down Expand Up @@ -457,7 +420,6 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, reques
Contact: request.Contact,
ContactsPresent: request.ContactsPresent,
Agreement: request.Agreement,
InitialIP: request.InitialIP,
Status: string(core.StatusValid),
}

Expand Down Expand Up @@ -1627,7 +1589,7 @@ func (ra *RegistrationAuthorityImpl) checkNewOrderLimits(ctx context.Context, na
// Deprecated: Use UpdateRegistrationContact or UpdateRegistrationKey instead.
func (ra *RegistrationAuthorityImpl) UpdateRegistration(ctx context.Context, req *rapb.UpdateRegistrationRequest) (*corepb.Registration, error) {
// Error if the request is nil, there is no account key or IP address
if req.Base == nil || len(req.Base.Key) == 0 || len(req.Base.InitialIP) == 0 || req.Base.Id == 0 {
if req.Base == nil || len(req.Base.Key) == 0 || req.Base.Id == 0 {
return nil, errIncompleteGRPCRequest
}

Expand Down Expand Up @@ -1741,7 +1703,6 @@ func mergeUpdate(base *corepb.Registration, update *corepb.Registration) (*corep
Contact: base.Contact,
ContactsPresent: base.ContactsPresent,
Agreement: base.Agreement,
InitialIP: base.InitialIP,
CreatedAt: base.CreatedAt,
Status: base.Status,
}
Expand Down
Loading

0 comments on commit a8cdaf8

Please sign in to comment.