Skip to content

Commit

Permalink
Use consistent naming for dnsName gRPC fields (#7654)
Browse files Browse the repository at this point in the history
Find all gRPC fields which represent DNS Names -- sometimes called
"identifier", "hostname", "domain", "identifierValue", or other things
-- and unify their naming. This naming makes it very clear that these
values are strings which may be included in the SAN extension of a
certificate with type dnsName.

As we move towards issuing IP Address certificates, all of these fields
will need to be replaced by fields which carry both an identifier type
and value, not just a single name. This unified naming makes it very
clear which messages and methods need to be updated to support
non-dnsName identifiers.

Part of #7647
  • Loading branch information
aarongable authored Aug 12, 2024
1 parent fa732df commit 46859a2
Show file tree
Hide file tree
Showing 29 changed files with 1,355 additions and 1,339 deletions.
2 changes: 1 addition & 1 deletion cmd/cert-checker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (c *certChecker) checkValidations(ctx context.Context, cert core.Certificat
// Any authorization for a given name is sufficient.
nameToAuthz := make(map[string]*corepb.Authorization)
for _, m := range authzs {
nameToAuthz[m.Identifier] = m
nameToAuthz[m.DnsName] = m
}

var errors []error
Expand Down
8 changes: 4 additions & 4 deletions core/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ type ValidationRecord struct {
URL string `json:"url,omitempty"`

// Shared
Hostname string `json:"hostname,omitempty"`
DnsName string `json:"hostname,omitempty"`
Port string `json:"port,omitempty"`
AddressesResolved []net.IP `json:"addressesResolved,omitempty"`
AddressUsed net.IP `json:"addressUsed,omitempty"`
Expand Down Expand Up @@ -209,7 +209,7 @@ func (ch Challenge) RecordsSane() bool {
for _, rec := range ch.ValidationRecord {
// TODO(#7140): Add a check for ResolverAddress == "" only after the
// core.proto change has been deployed.
if rec.URL == "" || rec.Hostname == "" || rec.Port == "" || rec.AddressUsed == nil ||
if rec.URL == "" || rec.DnsName == "" || rec.Port == "" || rec.AddressUsed == nil ||
len(rec.AddressesResolved) == 0 {
return false
}
Expand All @@ -223,7 +223,7 @@ func (ch Challenge) RecordsSane() bool {
}
// TODO(#7140): Add a check for ResolverAddress == "" only after the
// core.proto change has been deployed.
if ch.ValidationRecord[0].Hostname == "" || ch.ValidationRecord[0].Port == "" ||
if ch.ValidationRecord[0].DnsName == "" || ch.ValidationRecord[0].Port == "" ||
ch.ValidationRecord[0].AddressUsed == nil || len(ch.ValidationRecord[0].AddressesResolved) == 0 {
return false
}
Expand All @@ -233,7 +233,7 @@ func (ch Challenge) RecordsSane() bool {
}
// TODO(#7140): Add a check for ResolverAddress == "" only after the
// core.proto change has been deployed.
if ch.ValidationRecord[0].Hostname == "" {
if ch.ValidationRecord[0].DnsName == "" {
return false
}
return true
Expand Down
2 changes: 1 addition & 1 deletion core/objects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestRecordSanityCheckOnUnsupportedChallengeType(t *testing.T) {
rec := []ValidationRecord{
{
URL: "http://localhost/test",
Hostname: "localhost",
DnsName: "localhost",
Port: "80",
AddressesResolved: []net.IP{{127, 0, 0, 1}},
AddressUsed: net.IP{127, 0, 0, 1},
Expand Down
430 changes: 218 additions & 212 deletions core/proto/core.pb.go

Large diffs are not rendered by default.

38 changes: 21 additions & 17 deletions core/proto/core.proto
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ import "google/protobuf/timestamp.proto";

message Challenge {
// Next unused field number: 13
reserved 4, 5, 8, 11;
int64 id = 1;
// Fields specified by RFC 8555, Section 8.
string type = 2;
string url = 9;
string status = 6;
string uri = 9;
google.protobuf.Timestamp validated = 12;
ProblemDetails error = 7;
// Fields specified by individual validation methods.
string token = 3;
reserved 4; // Previously accountKey
// Additional fields for our own record keeping.
repeated ValidationRecord validationrecords = 10;
ProblemDetails error = 7;
reserved 8; // Unused and accidentally skipped during initial commit.
reserved 11; // Previously validatedNS
google.protobuf.Timestamp validated = 12;
}

message ValidationRecord {
Expand Down Expand Up @@ -88,33 +89,36 @@ message Registration {

message Authorization {
// Next unused field number: 10
reserved 5, 7, 8;
string id = 1;
string identifier = 2;
int64 registrationID = 3;
// Fields specified by RFC 8555, Section 7.1.4
string dnsName = 2;
string status = 4;
reserved 5; // Previously expiresNS
google.protobuf.Timestamp expires = 9;
repeated core.Challenge challenges = 6;
reserved 7; // previously ACMEv1 combinations
reserved 8; // previously v2
// We do not directly represent the "wildcard" field, instead inferring it
// from the identifier value.
}

message Order {
// Next unused field number: 15
reserved 3, 6, 10;
int64 id = 1;
int64 registrationID = 2;
reserved 3; // Previously expiresNS
// Fields specified by RFC 8555, Section 7.1.3
// Note that we do not respect notBefore and notAfter, and we infer the
// finalize and certificate URLs from the id and certificateSerial fields.
string status = 7;
google.protobuf.Timestamp expires = 12;
repeated string dnsNames = 8;
ProblemDetails error = 4;
repeated int64 v2Authorizations = 11;
string certificateSerial = 5;
reserved 6; // previously authorizations, deprecated in favor of v2Authorizations
string status = 7;
repeated string names = 8;
bool beganProcessing = 9;
reserved 10; // Previously createdNS
// Additional fields for our own record-keeping.
google.protobuf.Timestamp created = 13;
repeated int64 v2Authorizations = 11;
string certificateProfileName = 14;
bool beganProcessing = 9;
}

message CRLEntry {
Expand Down
10 changes: 5 additions & 5 deletions grpc/pb-marshalling.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func ValidationRecordToPB(record core.ValidationRecord) (*corepb.ValidationRecor
return nil, err
}
return &corepb.ValidationRecord{
Hostname: record.Hostname,
Hostname: record.DnsName,
Port: record.Port,
AddressesResolved: addrs,
AddressUsed: addrUsed,
Expand Down Expand Up @@ -169,7 +169,7 @@ func PBToValidationRecord(in *corepb.ValidationRecord) (record core.ValidationRe
return
}
return core.ValidationRecord{
Hostname: in.Hostname,
DnsName: in.Hostname,
Port: in.Port,
AddressesResolved: addrs,
AddressUsed: addrUsed,
Expand Down Expand Up @@ -314,7 +314,7 @@ func AuthzToPB(authz core.Authorization) (*corepb.Authorization, error) {

return &corepb.Authorization{
Id: authz.ID,
Identifier: authz.Identifier.Value,
DnsName: authz.Identifier.Value,
RegistrationID: authz.RegistrationID,
Status: string(authz.Status),
Expires: expires,
Expand All @@ -338,7 +338,7 @@ func PBToAuthz(pb *corepb.Authorization) (core.Authorization, error) {
}
authz := core.Authorization{
ID: pb.Id,
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: pb.Identifier},
Identifier: identifier.ACMEIdentifier{Type: identifier.DNS, Value: pb.DnsName},
RegistrationID: pb.RegistrationID,
Status: core.AcmeStatus(pb.Status),
Expires: expires,
Expand All @@ -362,7 +362,7 @@ func orderValid(order *corepb.Order) bool {
// `order.CertificateSerial` to be nil such that it can be used in places where
// the order has not been finalized yet.
func newOrderValid(order *corepb.Order) bool {
return !(order.RegistrationID == 0 || order.Expires == nil || len(order.Names) == 0)
return !(order.RegistrationID == 0 || order.Expires == nil || len(order.DnsNames) == 0)
}

func CertToPB(cert core.Certificate) *corepb.Certificate {
Expand Down
20 changes: 10 additions & 10 deletions grpc/pb-marshalling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestChallenge(t *testing.T) {
ip := net.ParseIP("1.1.1.1")
chall.ValidationRecord = []core.ValidationRecord{
{
Hostname: "example.com",
DnsName: "example.com",
Port: "2020",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestChallenge(t *testing.T) {
func TestValidationRecord(t *testing.T) {
ip := net.ParseIP("1.1.1.1")
vr := core.ValidationRecord{
Hostname: "exampleA.com",
DnsName: "exampleA.com",
Port: "80",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand All @@ -134,7 +134,7 @@ func TestValidationRecord(t *testing.T) {
func TestValidationResult(t *testing.T) {
ip := net.ParseIP("1.1.1.1")
vrA := core.ValidationRecord{
Hostname: "exampleA.com",
DnsName: "exampleA.com",
Port: "443",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand All @@ -143,7 +143,7 @@ func TestValidationResult(t *testing.T) {
ResolverAddrs: []string{"resolver:5353"},
}
vrB := core.ValidationRecord{
Hostname: "exampleB.com",
DnsName: "exampleB.com",
Port: "443",
AddressesResolved: []net.IP{ip},
AddressUsed: ip,
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
Created: timestamppb.New(created),
},
Expand All @@ -311,7 +311,7 @@ func TestOrderValid(t *testing.T) {
RegistrationID: 1,
Expires: timestamppb.New(expires),
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
Created: timestamppb.New(created),
},
Expand All @@ -329,7 +329,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
},
},
Expand All @@ -341,7 +341,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
},
},
Expand All @@ -353,7 +353,7 @@ func TestOrderValid(t *testing.T) {
Expires: nil,
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
BeganProcessing: false,
},
},
Expand All @@ -365,7 +365,7 @@ func TestOrderValid(t *testing.T) {
Expires: timestamppb.New(expires),
CertificateSerial: "",
V2Authorizations: []int64{},
Names: []string{},
DnsNames: []string{},
BeganProcessing: false,
},
},
Expand Down
6 changes: 3 additions & 3 deletions mocks/sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func (sa *StorageAuthority) NewOrderAndAuthzs(_ context.Context, req *sapb.NewOr
// Fields from the input new order request.
RegistrationID: req.NewOrder.RegistrationID,
Expires: req.NewOrder.Expires,
Names: req.NewOrder.Names,
DnsNames: req.NewOrder.DnsNames,
V2Authorizations: req.NewOrder.V2Authorizations,
// Mock new fields generated by the database transaction.
Id: rand.Int64(),
Expand Down Expand Up @@ -394,7 +394,7 @@ func (sa *StorageAuthorityReadOnly) GetOrder(_ context.Context, req *sapb.OrderR
RegistrationID: 1,
Created: timestamppb.New(created),
Expires: timestamppb.New(exp),
Names: []string{"example.com"},
DnsNames: []string{"example.com"},
Status: string(core.StatusValid),
V2Authorizations: []int64{1},
CertificateSerial: "serial",
Expand Down Expand Up @@ -470,7 +470,7 @@ func (sa *StorageAuthorityReadOnly) GetValidAuthorizations2(ctx context.Context,
}
expiryCutoff := req.ValidUntil.AsTime()
auths := &sapb.Authorizations{}
for _, name := range req.Domains {
for _, name := range req.DnsNames {
exp := expiryCutoff.AddDate(100, 0, 0)
authzPB, err := bgrpc.AuthzToPB(core.Authorization{
Status: core.StatusValid,
Expand Down
Loading

0 comments on commit 46859a2

Please sign in to comment.