wfe: remove Payload from logs (#6639)

Also remove CSRDNSNames, CSRIPAddresses and CSREmailAddresses.

And add a new log field "DNSNames", for use in new-order, finalize, and
revoke requests.

Add a "RevocationReason" field in the "Extra" section for revoke
requests.
This commit is contained in:
Jacob Hoffman-Andrews 2023-02-09 13:45:14 -08:00 committed by GitHub
parent 134321040b
commit 67927390e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 26 additions and 34 deletions

View File

@ -39,9 +39,8 @@ type RequestEvent struct {
Contacts []string `json:",omitempty"`
UserAgent string `json:"ua,omitempty"`
// Origin is sent by the browser from XHR-based clients.
Origin string `json:",omitempty"`
Payload string `json:",omitempty"`
Extra map[string]interface{} `json:",omitempty"`
Origin string `json:",omitempty"`
Extra map[string]interface{} `json:",omitempty"`
// For endpoints that create objects, the ID of the newly created object.
Created string `json:",omitempty"`
@ -49,8 +48,12 @@ type RequestEvent struct {
// For challenge and authorization GETs and POSTs:
// the status of the authorization at the time the request began.
Status string `json:",omitempty"`
// The DNS name, if applicable
// The DNS name, if there is a single relevant name, for instance
// in an authorization or challenge request.
DNSName string `json:",omitempty"`
// The set of DNS names, if there are potentially multiple relevant
// names, for instance in a new-order, finalize, or revoke request.
DNSNames []string `json:",omitempty"`
// For challenge POSTs, the challenge type.
ChallengeType string `json:",omitempty"`

View File

@ -517,8 +517,7 @@ func (wfe *WebFrontEndImpl) validJWSForKey(
ctx context.Context,
jws *jose.JSONWebSignature,
jwk *jose.JSONWebKey,
request *http.Request,
logEvent *web.RequestEvent) ([]byte, *probs.ProblemDetails) {
request *http.Request) ([]byte, *probs.ProblemDetails) {
// Check that the public key and JWS algorithms match expected
err := checkAlgorithm(jwk, jws)
@ -538,9 +537,6 @@ func (wfe *WebFrontEndImpl) validJWSForKey(
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSVerifyFailed"}).Inc()
return nil, probs.Malformed("JWS verification error")
}
// Store the verified payload in the logEvent
logEvent.Payload = string(payload)
beeline.AddFieldToTrace(ctx, "payload", string(payload))
// Check that the JWS contains a correct Nonce header
if prob := wfe.validNonce(ctx, jws); prob != nil {
@ -586,7 +582,7 @@ func (wfe *WebFrontEndImpl) validJWSForAccount(
}
// Verify the JWS with the JWK from the SA
payload, prob := wfe.validJWSForKey(ctx, jws, pubKey, request, logEvent)
payload, prob := wfe.validJWSForKey(ctx, jws, pubKey, request)
if prob != nil {
return nil, nil, nil, prob
}
@ -658,8 +654,7 @@ func (wfe *WebFrontEndImpl) validPOSTAsGETForAccount(
func (wfe *WebFrontEndImpl) validSelfAuthenticatedJWS(
ctx context.Context,
jws *jose.JSONWebSignature,
request *http.Request,
logEvent *web.RequestEvent) ([]byte, *jose.JSONWebKey, *probs.ProblemDetails) {
request *http.Request) ([]byte, *jose.JSONWebKey, *probs.ProblemDetails) {
// Extract the embedded JWK from the parsed JWS
pubKey, prob := wfe.extractJWK(jws)
if prob != nil {
@ -667,7 +662,7 @@ func (wfe *WebFrontEndImpl) validSelfAuthenticatedJWS(
}
// Verify the JWS with the embedded JWK
payload, prob := wfe.validJWSForKey(ctx, jws, pubKey, request, logEvent)
payload, prob := wfe.validJWSForKey(ctx, jws, pubKey, request)
if prob != nil {
return nil, nil, prob
}
@ -680,8 +675,7 @@ func (wfe *WebFrontEndImpl) validSelfAuthenticatedJWS(
// goodkey policies (key algorithm, length, blocklist, etc).
func (wfe *WebFrontEndImpl) validSelfAuthenticatedPOST(
ctx context.Context,
request *http.Request,
logEvent *web.RequestEvent) ([]byte, *jose.JSONWebKey, *probs.ProblemDetails) {
request *http.Request) ([]byte, *jose.JSONWebKey, *probs.ProblemDetails) {
// Parse the JWS from the POST request
jws, prob := wfe.parseJWSRequest(request)
if prob != nil {
@ -689,7 +683,7 @@ func (wfe *WebFrontEndImpl) validSelfAuthenticatedPOST(
}
// Extract and validate the embedded JWK from the parsed JWS
payload, pubKey, prob := wfe.validSelfAuthenticatedJWS(ctx, jws, request, logEvent)
payload, pubKey, prob := wfe.validSelfAuthenticatedJWS(ctx, jws, request)
if prob != nil {
return nil, nil, prob
}

View File

@ -1295,14 +1295,12 @@ func TestValidJWSForKey(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
wfe.stats.joseErrorCount.Reset()
inputLogEvent := newRequestEvent()
request := makePostRequestWithPath("test", tc.Body)
outPayload, prob := wfe.validJWSForKey(context.Background(), tc.JWS, tc.JWK, request, inputLogEvent)
outPayload, prob := wfe.validJWSForKey(context.Background(), tc.JWS, tc.JWK, request)
if tc.ExpectedProblem == nil && prob != nil {
t.Fatalf("Expected nil problem, got %#v\n", prob)
} else if tc.ExpectedProblem == nil {
test.AssertEquals(t, inputLogEvent.Payload, payload)
test.AssertEquals(t, string(outPayload), payload)
} else {
test.AssertMarshaledEquals(t, prob, tc.ExpectedProblem)
@ -1397,7 +1395,6 @@ func TestValidPOSTForAccount(t *testing.T) {
if tc.ExpectedProblem == nil && prob != nil {
t.Fatalf("Expected nil problem, got %#v\n", prob)
} else if tc.ExpectedProblem == nil {
test.AssertEquals(t, inputLogEvent.Payload, tc.ExpectedPayload)
test.AssertEquals(t, string(outPayload), tc.ExpectedPayload)
test.AssertMarshaledEquals(t, acct, tc.ExpectedAcct)
test.AssertMarshaledEquals(t, jws, tc.ExpectedJWS)
@ -1437,7 +1434,6 @@ func TestValidPOSTAsGETForAccount(t *testing.T) {
ExpectedProblem: probs.Malformed("POST-as-GET requests must have an empty payload"),
ExpectedLogEvent: web.RequestEvent{
Contacts: []string{"mailto:person@mail.com"},
Payload: "{}",
},
},
{
@ -1544,15 +1540,13 @@ func TestValidSelfAuthenticatedPOST(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
wfe.stats.joseErrorCount.Reset()
inputLogEvent := newRequestEvent()
outPayload, jwk, prob := wfe.validSelfAuthenticatedPOST(context.Background(), tc.Request, inputLogEvent)
outPayload, jwk, prob := wfe.validSelfAuthenticatedPOST(context.Background(), tc.Request)
if tc.ExpectedProblem == nil && prob != nil {
t.Fatalf("Expected nil problem, got %#v\n", prob)
} else if tc.ExpectedProblem == nil {
inThumb, _ := tc.ExpectedJWK.Thumbprint(crypto.SHA256)
outThumb, _ := jwk.Thumbprint(crypto.SHA256)
test.AssertDeepEquals(t, inThumb, outThumb)
test.AssertEquals(t, inputLogEvent.Payload, tc.ExpectedPayload)
test.AssertEquals(t, string(outPayload), tc.ExpectedPayload)
} else {
test.AssertMarshaledEquals(t, prob, tc.ExpectedProblem)

View File

@ -616,7 +616,7 @@ func (wfe *WebFrontEndImpl) NewAccount(
// NewAccount uses `validSelfAuthenticatedPOST` instead of
// `validPOSTforAccount` because there is no account to authenticate against
// until after it is created!
body, key, prob := wfe.validSelfAuthenticatedPOST(ctx, request, logEvent)
body, key, prob := wfe.validSelfAuthenticatedPOST(ctx, request)
if prob != nil {
// validSelfAuthenticatedPOST handles its own setting of logEvent.Errors
wfe.sendError(response, logEvent, prob, nil)
@ -815,6 +815,9 @@ func (wfe *WebFrontEndImpl) parseRevocation(
// Compute and record the serial number of the provided certificate
serial := core.SerialToString(parsedCertificate.SerialNumber)
logEvent.Extra["CertificateSerial"] = serial
if revokeRequest.Reason != nil {
logEvent.Extra["RevocationReason"] = *revokeRequest.Reason
}
beeline.AddFieldToTrace(ctx, "cert.serial", serial)
// Try to validate the signature on the provided cert using its corresponding
@ -828,8 +831,8 @@ func (wfe *WebFrontEndImpl) parseRevocation(
if err != nil {
return nil, 0, probs.NotFound("No such certificate")
}
logEvent.Extra["CertificateDNSNames"] = parsedCertificate.DNSNames
beeline.AddFieldToTrace(ctx, "cert.dnsnames", parsedCertificate.DNSNames)
logEvent.DNSNames = parsedCertificate.DNSNames
beeline.AddFieldToTrace(ctx, "dnsnames", parsedCertificate.DNSNames)
if parsedCertificate.NotAfter.Before(wfe.clk.Now()) {
return nil, 0, probs.Unauthorized("Certificate is expired")
@ -916,7 +919,7 @@ func (wfe *WebFrontEndImpl) revokeCertByCertKey(
// `validSelfAuthenticatedJWS` similar to new-reg and key rollover.
// We do *not* use `validSelfAuthenticatedPOST` here because we've already
// read the HTTP request body in `parseJWSRequest` and it is now empty.
jwsBody, jwk, prob := wfe.validSelfAuthenticatedJWS(ctx, outerJWS, request, logEvent)
jwsBody, jwk, prob := wfe.validSelfAuthenticatedJWS(ctx, outerJWS, request)
if prob != nil {
return prob
}
@ -2035,6 +2038,8 @@ func (wfe *WebFrontEndImpl) NewOrder(
return
}
logEvent.DNSNames = names
order, err := wfe.ra.NewOrder(ctx, &rapb.NewOrderRequest{
RegistrationID: acct.ID,
Names: names,
@ -2230,12 +2235,8 @@ func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.Req
return
}
logEvent.Extra["CSRDNSNames"] = csr.DNSNames
beeline.AddFieldToTrace(ctx, "csr.dnsnames", csr.DNSNames)
logEvent.Extra["CSREmailAddresses"] = csr.EmailAddresses
beeline.AddFieldToTrace(ctx, "csr.email_addrs", csr.EmailAddresses)
logEvent.Extra["CSRIPAddresses"] = csr.IPAddresses
beeline.AddFieldToTrace(ctx, "csr.ip_addrs", csr.IPAddresses)
logEvent.DNSNames = order.Names
beeline.AddFieldToTrace(ctx, "dnsnames", csr.DNSNames)
logEvent.Extra["KeyType"] = web.KeyTypeToString(csr.PublicKey)
beeline.AddFieldToTrace(ctx, "csr.key_type", web.KeyTypeToString(csr.PublicKey))