Skip to content

Commit 0f5d206

Browse files
aarongableaarongable
authored
Remove logic from VA PerformValidation wrapper (letsencrypt#5003)
Updates the type of the ValidationAuthority's PerformValidation method to be identical to that of the corresponding auto-generated grpc method, i.e. directly taking and returning proto message types, rather than exploded arguments. This allows all logic to be removed from the VA wrappers, which will allow them to be fully removed after the migration to proto3. Also updates all tests and VA clients to adopt the new interface. Depends on letsencrypt#4983 (do not review first four commits) Part of letsencrypt#4956
1 parent 634d57c commit 0f5d206

File tree

15 files changed

+385
-532
lines changed

15 files changed

+385
-532
lines changed

‎cmd/boulder-va/main.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ func main() {
141141
remotes = append(
142142
remotes,
143143
va.RemoteVA{
144-
ValidationAuthority: bgrpc.NewValidationAuthorityGRPCClient(vaConn),
145-
Address: rva.ServerAddress,
144+
VAClient: bgrpc.NewValidationAuthorityGRPCClient(vaConn),
145+
Address: rva.ServerAddress,
146146
},
147147
)
148148
}

‎core/interfaces.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
rapb "github.com/letsencrypt/boulder/ra/proto"
1717
"github.com/letsencrypt/boulder/revocation"
1818
sapb "github.com/letsencrypt/boulder/sa/proto"
19+
vapb "github.com/letsencrypt/boulder/va/proto"
1920
)
2021

2122
// A WebFrontEnd object supplies methods that can be hooked into
@@ -91,6 +92,10 @@ type RegistrationAuthority interface {
9192
AdministrativelyRevokeCertificate(ctx context.Context, cert x509.Certificate, code revocation.Reason, adminName string) error
9293
}
9394

95+
// ValidationAuthority defines the public interface for the Boulder VA
96+
// TODO(#4956): Remove this unnecessary type alias.
97+
type ValidationAuthority vapb.VAServer
98+
9499
// CertificateAuthority defines the public interface for the Boulder CA
95100
type CertificateAuthority interface {
96101
// [RegistrationAuthority]

‎core/objects.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const (
5858
)
5959

6060
// These types are the available challenges
61+
// TODO(#5009): Make this a custom type as well.
6162
const (
6263
ChallengeTypeHTTP01 = "http-01"
6364
ChallengeTypeDNS01 = "dns-01"

‎core/va.go‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎grpc/pb-marshalling.go‎

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,6 @@ var ErrMissingParameters = CodedError(codes.FailedPrecondition, "required RPC pa
2626
// This file defines functions to translate between the protobuf types and the
2727
// code types.
2828

29-
func authzMetaToPB(authz core.Authorization) (*vapb.AuthzMeta, error) {
30-
return &vapb.AuthzMeta{
31-
Id: &authz.ID,
32-
RegID: &authz.RegistrationID,
33-
}, nil
34-
}
35-
36-
func pbToAuthzMeta(in *vapb.AuthzMeta) (core.Authorization, error) {
37-
if in == nil || in.Id == nil || in.RegID == nil {
38-
return core.Authorization{}, ErrMissingParameters
39-
}
40-
return core.Authorization{
41-
ID: *in.Id,
42-
RegistrationID: *in.RegID,
43-
}, nil
44-
}
45-
4629
func ProblemDetailsToPB(prob *probs.ProblemDetails) (*corepb.ProblemDetails, error) {
4730
if prob == nil {
4831
// nil problemDetails is valid
@@ -98,7 +81,7 @@ func ChallengeToPB(challenge core.Challenge) (*corepb.Challenge, error) {
9881
}, nil
9982
}
10083

101-
func pbToChallenge(in *corepb.Challenge) (challenge core.Challenge, err error) {
84+
func PBToChallenge(in *corepb.Challenge) (challenge core.Challenge, err error) {
10285
if in == nil {
10386
return core.Challenge{}, ErrMissingParameters
10487
}
@@ -224,45 +207,6 @@ func pbToValidationResult(in *vapb.ValidationResult) ([]core.ValidationRecord, *
224207
return recordAry, prob, nil
225208
}
226209

227-
func performValidationReqToArgs(in *vapb.PerformValidationRequest) (domain string, challenge core.Challenge, authz core.Authorization, err error) {
228-
if in == nil {
229-
err = ErrMissingParameters
230-
return
231-
}
232-
if in.Domain == nil {
233-
err = ErrMissingParameters
234-
return
235-
}
236-
domain = *in.Domain
237-
challenge, err = pbToChallenge(in.Challenge)
238-
if err != nil {
239-
return
240-
}
241-
authz, err = pbToAuthzMeta(in.Authz)
242-
if err != nil {
243-
return
244-
}
245-
246-
return domain, challenge, authz, nil
247-
}
248-
249-
func argsToPerformValidationRequest(domain string, challenge core.Challenge, authz core.Authorization) (*vapb.PerformValidationRequest, error) {
250-
pbChall, err := ChallengeToPB(challenge)
251-
if err != nil {
252-
return nil, err
253-
}
254-
authzMeta, err := authzMetaToPB(authz)
255-
if err != nil {
256-
return nil, err
257-
}
258-
return &vapb.PerformValidationRequest{
259-
Domain: &domain,
260-
Challenge: pbChall,
261-
Authz: authzMeta,
262-
}, nil
263-
264-
}
265-
266210
func registrationToPB(reg core.Registration) (*corepb.Registration, error) {
267211
keyBytes, err := reg.Key.MarshalJSON()
268212
if err != nil {
@@ -357,7 +301,7 @@ func AuthzToPB(authz core.Authorization) (*corepb.Authorization, error) {
357301
func PBToAuthz(pb *corepb.Authorization) (core.Authorization, error) {
358302
challs := make([]core.Challenge, len(pb.Challenges))
359303
for i, c := range pb.Challenges {
360-
chall, err := pbToChallenge(c)
304+
chall, err := PBToChallenge(c)
361305
if err != nil {
362306
return core.Authorization{}, err
363307
}

‎grpc/pb-marshalling_test.go‎

Lines changed: 8 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -13,40 +13,8 @@ import (
1313
"github.com/letsencrypt/boulder/identifier"
1414
"github.com/letsencrypt/boulder/probs"
1515
"github.com/letsencrypt/boulder/test"
16-
vapb "github.com/letsencrypt/boulder/va/proto"
1716
)
1817

19-
func TestAuthzMeta(t *testing.T) {
20-
authz := core.Authorization{ID: "asd", RegistrationID: 10}
21-
pb, err := authzMetaToPB(authz)
22-
test.AssertNotError(t, err, "authzMetaToPB failed")
23-
test.Assert(t, pb != nil, "return vapb.AuthzMeta is nill")
24-
test.Assert(t, pb.Id != nil, "Id field is nil")
25-
test.AssertEquals(t, *pb.Id, authz.ID)
26-
test.Assert(t, pb.RegID != nil, "RegistrationID field is nil")
27-
test.AssertEquals(t, *pb.RegID, authz.RegistrationID)
28-
29-
recon, err := pbToAuthzMeta(pb)
30-
test.AssertNotError(t, err, "pbToAuthzMeta failed")
31-
test.AssertEquals(t, recon.ID, authz.ID)
32-
test.AssertEquals(t, recon.RegistrationID, authz.RegistrationID)
33-
34-
_, err = pbToAuthzMeta(nil)
35-
test.AssertError(t, err, "pbToAuthzMeta did not fail")
36-
test.AssertEquals(t, err, ErrMissingParameters)
37-
_, err = pbToAuthzMeta(&vapb.AuthzMeta{})
38-
test.AssertError(t, err, "pbToAuthzMeta did not fail")
39-
test.AssertEquals(t, err, ErrMissingParameters)
40-
empty := ""
41-
one := int64(1)
42-
_, err = pbToAuthzMeta(&vapb.AuthzMeta{Id: &empty})
43-
test.AssertError(t, err, "pbToAuthzMeta did not fail")
44-
test.AssertEquals(t, err, ErrMissingParameters)
45-
_, err = pbToAuthzMeta(&vapb.AuthzMeta{RegID: &one})
46-
test.AssertError(t, err, "pbToAuthzMeta did not fail")
47-
test.AssertEquals(t, err, ErrMissingParameters)
48-
}
49-
5018
const JWK1JSON = `{"kty":"RSA","n":"vuc785P8lBj3fUxyZchF_uZw6WtbxcorqgTyq-qapF5lrO1U82Tp93rpXlmctj6fyFHBVVB5aXnUHJ7LZeVPod7Wnfl8p5OyhlHQHC8BnzdzCqCMKmWZNX5DtETDId0qzU7dPzh0LP0idt5buU7L9QNaabChw3nnaL47iu_1Di5Wp264p2TwACeedv2hfRDjDlJmaQXuS8Rtv9GnRWyC9JBu7XmGvGDziumnJH7Hyzh3VNu-kSPQD3vuAFgMZS6uUzOztCkT0fpOalZI6hqxtWLvXUMj-crXrn-Maavz8qRhpAyp5kcYk3jiHGgQIi7QSK2JIdRJ8APyX9HlmTN5AQ","e":"AQAB"}`
5119

5220
func TestProblemDetails(t *testing.T) {
@@ -96,8 +64,8 @@ func TestChallenge(t *testing.T) {
9664
test.AssertNotError(t, err, "ChallengeToPB failed")
9765
test.Assert(t, pb != nil, "Returned corepb.Challenge is nil")
9866

99-
recon, err := pbToChallenge(pb)
100-
test.AssertNotError(t, err, "pbToChallenge failed")
67+
recon, err := PBToChallenge(pb)
68+
test.AssertNotError(t, err, "PBToChallenge failed")
10169
test.AssertDeepEquals(t, recon, chall)
10270

10371
ip := net.ParseIP("1.1.1.1")
@@ -116,15 +84,15 @@ func TestChallenge(t *testing.T) {
11684
test.AssertNotError(t, err, "ChallengeToPB failed")
11785
test.Assert(t, pb != nil, "Returned corepb.Challenge is nil")
11886

119-
recon, err = pbToChallenge(pb)
120-
test.AssertNotError(t, err, "pbToChallenge failed")
87+
recon, err = PBToChallenge(pb)
88+
test.AssertNotError(t, err, "PBToChallenge failed")
12189
test.AssertDeepEquals(t, recon, chall)
12290

123-
_, err = pbToChallenge(nil)
124-
test.AssertError(t, err, "pbToChallenge did not fail")
91+
_, err = PBToChallenge(nil)
92+
test.AssertError(t, err, "PBToChallenge did not fail")
12593
test.AssertEquals(t, err, ErrMissingParameters)
126-
_, err = pbToChallenge(&corepb.Challenge{})
127-
test.AssertError(t, err, "pbToChallenge did not fail")
94+
_, err = PBToChallenge(&corepb.Challenge{})
95+
test.AssertError(t, err, "PBToChallenge did not fail")
12896
test.AssertEquals(t, err, ErrMissingParameters)
12997
}
13098

@@ -179,30 +147,6 @@ func TestValidationResult(t *testing.T) {
179147
test.AssertDeepEquals(t, reconProb, prob)
180148
}
181149

182-
func TestPerformValidationReq(t *testing.T) {
183-
var jwk jose.JSONWebKey
184-
err := json.Unmarshal([]byte(JWK1JSON), &jwk)
185-
test.AssertNotError(t, err, "Failed to unmarshal test key")
186-
domain := "example.com"
187-
chall := core.Challenge{
188-
Type: core.ChallengeTypeDNS01,
189-
Status: core.StatusPending,
190-
Token: "asd",
191-
ProvidedKeyAuthorization: "keyauth",
192-
}
193-
authz := core.Authorization{ID: "asd", RegistrationID: 10}
194-
195-
pb, err := argsToPerformValidationRequest(domain, chall, authz)
196-
test.AssertNotError(t, err, "argsToPerformValidationRequest failed")
197-
test.Assert(t, pb != nil, "Return vapb.PerformValidationRequest is nil")
198-
199-
reconDomain, reconChall, reconAuthz, err := performValidationReqToArgs(pb)
200-
test.AssertNotError(t, err, "performValidationReqToArgs failed")
201-
test.AssertEquals(t, reconDomain, domain)
202-
test.AssertDeepEquals(t, reconChall, chall)
203-
test.AssertDeepEquals(t, reconAuthz, authz)
204-
}
205-
206150
func TestRegistration(t *testing.T) {
207151
contacts := []string{"email"}
208152
var key jose.JSONWebKey

‎grpc/va-wrappers.go‎

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,68 +11,33 @@ import (
1111

1212
ggrpc "google.golang.org/grpc"
1313

14-
"github.com/letsencrypt/boulder/core"
15-
"github.com/letsencrypt/boulder/probs"
1614
vapb "github.com/letsencrypt/boulder/va/proto"
1715
)
1816

1917
type ValidationAuthorityGRPCServer struct {
20-
impl core.ValidationAuthority
18+
inner vapb.VAServer
2119
}
2220

23-
func (s *ValidationAuthorityGRPCServer) PerformValidation(ctx context.Context, in *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) {
24-
domain, challenge, authz, err := performValidationReqToArgs(in)
25-
if err != nil {
26-
return nil, err
27-
}
28-
records, err := s.impl.PerformValidation(ctx, domain, challenge, authz)
29-
// If the type of error was a ProblemDetails, we need to return
30-
// both that and the records to the caller (so it can update
31-
// the challenge / authz in the SA with the failing records).
32-
// The least error-prone way of doing this is to send a struct
33-
// as the RPC response and return a nil error on the RPC layer,
34-
// then unpack that into (records, error) to the caller.
35-
prob, ok := err.(*probs.ProblemDetails)
36-
if !ok && err != nil {
37-
return nil, err
38-
}
39-
return ValidationResultToPB(records, prob)
21+
func (s *ValidationAuthorityGRPCServer) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) {
22+
return s.inner.PerformValidation(ctx, req)
4023
}
4124

42-
func RegisterValidationAuthorityGRPCServer(s *ggrpc.Server, impl core.ValidationAuthority) error {
43-
rpcSrv := &ValidationAuthorityGRPCServer{impl}
25+
func RegisterValidationAuthorityGRPCServer(s *ggrpc.Server, inner vapb.VAServer) error {
26+
rpcSrv := &ValidationAuthorityGRPCServer{inner}
4427
vapb.RegisterVAServer(s, rpcSrv)
4528
return nil
4629
}
4730

4831
type ValidationAuthorityGRPCClient struct {
49-
gc vapb.VAClient
32+
inner vapb.VAClient
5033
}
5134

52-
func NewValidationAuthorityGRPCClient(cc *ggrpc.ClientConn) core.ValidationAuthority {
35+
func NewValidationAuthorityGRPCClient(cc *ggrpc.ClientConn) vapb.VAClient {
5336
return &ValidationAuthorityGRPCClient{vapb.NewVAClient(cc)}
5437
}
5538

5639
// PerformValidation has the VA revalidate the specified challenge and returns
5740
// the updated Challenge object.
58-
func (vac ValidationAuthorityGRPCClient) PerformValidation(ctx context.Context, domain string, challenge core.Challenge, authz core.Authorization) ([]core.ValidationRecord, error) {
59-
req, err := argsToPerformValidationRequest(domain, challenge, authz)
60-
if err != nil {
61-
return nil, err
62-
}
63-
gRecords, err := vac.gc.PerformValidation(ctx, req)
64-
if err != nil {
65-
return nil, err
66-
}
67-
records, prob, err := pbToValidationResult(gRecords)
68-
if err != nil {
69-
return nil, err
70-
}
71-
if prob != nil {
72-
return records, prob
73-
}
74-
75-
// We return nil explicitly to avoid "typed nil" problems.
76-
// https://golang.org/doc/faq#nil_error
77-
return records, nil
41+
func (vac ValidationAuthorityGRPCClient) PerformValidation(ctx context.Context, req *vapb.PerformValidationRequest, opts ...ggrpc.CallOption) (*vapb.ValidationResult, error) {
42+
return vac.inner.PerformValidation(ctx, req)
7843
}

‎ra/ra.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type caaChecker interface {
5757
// populated, or there is a risk of panic.
5858
type RegistrationAuthorityImpl struct {
5959
CA core.CertificateAuthority
60-
VA core.ValidationAuthority
60+
VA vapb.VAClient
6161
SA core.StorageAuthority
6262
PA core.PolicyAuthority
6363
publisher core.Publisher
@@ -1609,18 +1609,39 @@ func (ra *RegistrationAuthorityImpl) PerformValidation(
16091609
challenges := make([]core.Challenge, len(authz.Challenges))
16101610
copy(challenges, authz.Challenges)
16111611
authz.Challenges = challenges
1612+
chall, _ := bgrpc.ChallengeToPB(authz.Challenges[challIndex])
1613+
1614+
req := vapb.PerformValidationRequest{
1615+
Domain: &authz.Identifier.Value,
1616+
Challenge: chall,
1617+
Authz: &vapb.AuthzMeta{
1618+
Id: &authz.ID,
1619+
RegID: &authz.RegistrationID,
1620+
},
1621+
}
1622+
res, err := ra.VA.PerformValidation(vaCtx, &req)
16121623

1613-
records, err := ra.VA.PerformValidation(vaCtx, authz.Identifier.Value, authz.Challenges[challIndex], authz)
16141624
var prob *probs.ProblemDetails
1615-
if p, ok := err.(*probs.ProblemDetails); ok {
1616-
prob = p
1617-
} else if err != nil {
1625+
if err != nil {
16181626
prob = probs.ServerInternal("Could not communicate with VA")
16191627
ra.log.AuditErrf("Could not communicate with VA: %s", err)
1628+
} else if res.Problems != nil {
1629+
prob, err = bgrpc.PBToProblemDetails(res.Problems)
1630+
if err != nil {
1631+
prob = probs.ServerInternal("Could not communicate with VA")
1632+
ra.log.AuditErrf("Could not communicate with VA: %s", err)
1633+
}
16201634
}
16211635

16221636
// Save the updated records
16231637
challenge := &authz.Challenges[challIndex]
1638+
records := make([]core.ValidationRecord, len(res.Records))
1639+
for i, r := range res.Records {
1640+
records[i], err = bgrpc.PBToValidationRecord(r)
1641+
if err != nil {
1642+
prob = probs.ServerInternal("Records for validation corrupt")
1643+
}
1644+
}
16241645
challenge.ValidationRecord = records
16251646

16261647
if !challenge.RecordsSane() && prob == nil {

0 commit comments

Comments
 (0)