From 66831f9238837fdcaa6c25a7a9922024d24fdbcf Mon Sep 17 00:00:00 2001 From: kmille Date: Sun, 14 Mar 2021 15:30:47 +0100 Subject: [PATCH 1/4] Export selector and DNS response via Verification This makes debugging much easier. It can be difficult to debug issues as the verification struct does not contain all information used for the verification like the public key or the selector. If you change the DNS record and don't know if DNS is still cached further information can help. --- cmd/dkim-verify/main.go | 4 ++-- dkim/verify.go | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/cmd/dkim-verify/main.go b/cmd/dkim-verify/main.go index 74af505..814b5fc 100644 --- a/cmd/dkim-verify/main.go +++ b/cmd/dkim-verify/main.go @@ -15,9 +15,9 @@ func main() { for _, v := range verifications { if v.Err == nil { - log.Printf("Valid signature for %v", v.Domain) + log.Printf("Valid signature for %v (selector=%s) (algo=%s)", v.Domain, v.Selector, v.QueryResult.KeyAlgo) } else { - log.Printf("Invalid signature for %v: %v", v.Domain, v.Err) + log.Printf("Valid signature for %v (selector=%s) (algo=%s): %v", v.Domain, v.Selector, v.QueryResult.KeyAlgo, v.Err) } } } diff --git a/dkim/verify.go b/dkim/verify.go index b02625a..eced7a2 100644 --- a/dkim/verify.go +++ b/dkim/verify.go @@ -67,6 +67,10 @@ type Verification struct { // The SDID claiming responsibility for an introduction of a message into the // mail stream. Domain string + + // The Selector is the s value of the DKIM-Signature header + Selector string + // The Agent or User Identifier (AUID) on behalf of which the SDID is taking // responsibility. Identifier string @@ -79,6 +83,9 @@ type Verification struct { // The expiration time. If the signature doesn't expire, it's set to zero. Expiration time.Time + // The QueryResult holds the parsed DNS response + QueryResult *queryResult + // Err is nil if the signature is valid. Err error } @@ -221,6 +228,7 @@ func verify(h header, r io.Reader, sigField, sigValue string, options *VerifyOpt return verif, permFailError("incompatible signature version") } + verif.Selector = stripWhitespace(params["s"]) verif.Domain = stripWhitespace(params["d"]) for _, tag := range requiredTags { @@ -286,11 +294,13 @@ func verify(h header, r io.Reader, sigField, sigValue string, options *VerifyOpt break } } + if err != nil { return verif, err } else if res == nil { return verif, permFailError("unsupported public key query method") } + verif.QueryResult = res // Parse algos algos := strings.SplitN(stripWhitespace(params["a"]), "-", 2) From f7a5ed3a97024a883a4856f57bd3cf925021cf26 Mon Sep 17 00:00:00 2001 From: kmille Date: Sun, 14 Mar 2021 15:41:02 +0100 Subject: [PATCH 2/4] Fix copy paste typo in print --- cmd/dkim-verify/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/dkim-verify/main.go b/cmd/dkim-verify/main.go index 814b5fc..699b65b 100644 --- a/cmd/dkim-verify/main.go +++ b/cmd/dkim-verify/main.go @@ -17,7 +17,7 @@ func main() { if v.Err == nil { log.Printf("Valid signature for %v (selector=%s) (algo=%s)", v.Domain, v.Selector, v.QueryResult.KeyAlgo) } else { - log.Printf("Valid signature for %v (selector=%s) (algo=%s): %v", v.Domain, v.Selector, v.QueryResult.KeyAlgo, v.Err) + log.Printf("Invalid signature for %v (selector=%s) (algo=%s): %v", v.Domain, v.Selector, v.QueryResult.KeyAlgo, v.Err) } } } From b23726d2fb946b15a9aef9dcb43c84b172566579 Mon Sep 17 00:00:00 2001 From: kmille Date: Sun, 14 Mar 2021 15:47:18 +0100 Subject: [PATCH 3/4] Update verify example in the README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4d5286c..92ddd30 100644 --- a/README.md +++ b/README.md @@ -41,9 +41,9 @@ if err != nil { for _, v := range verifications { if v.Err == nil { - log.Println("Valid signature for:", v.Domain) + log.Printf("Valid signature for %v (selector=%s) (algo=%s)", v.Domain, v.Selector, v.QueryResult.KeyAlgo) } else { - log.Println("Invalid signature for:", v.Domain, v.Err) + log.Printf("Invalid signature for %v (selector=%s) (algo=%s): %v", v.Domain, v.Selector, v.QueryResult.KeyAlgo, v.Err) } } ``` From f5c752432b214595f39e3899454b93c27e7a362f Mon Sep 17 00:00:00 2001 From: kmille Date: Wed, 17 Mar 2021 14:23:42 +0100 Subject: [PATCH 4/4] Fix tests after adding QueryResult to Verification struct --- dkim/query.go | 10 +++++----- dkim/query_test.go | 2 +- dkim/verify.go | 4 ++-- dkim/verify_test.go | 22 ++++++++++++++++++++++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/dkim/query.go b/dkim/query.go index 8fd6ecb..13356bd 100644 --- a/dkim/query.go +++ b/dkim/query.go @@ -45,7 +45,7 @@ func (v ed25519Verifier) Verify(hash crypto.Hash, hashed, sig []byte) error { return nil } -type queryResult struct { +type QueryResult struct { Verifier verifier KeyAlgo string HashAlgos []string @@ -63,13 +63,13 @@ const ( ) type txtLookupFunc func(domain string) ([]string, error) -type queryFunc func(domain, selector string, txtLookup txtLookupFunc) (*queryResult, error) +type queryFunc func(domain, selector string, txtLookup txtLookupFunc) (*QueryResult, error) var queryMethods = map[QueryMethod]queryFunc{ QueryMethodDNSTXT: queryDNSTXT, } -func queryDNSTXT(domain, selector string, txtLookup txtLookupFunc) (*queryResult, error) { +func queryDNSTXT(domain, selector string, txtLookup txtLookupFunc) (*QueryResult, error) { var txts []string var err error if txtLookup != nil { @@ -90,13 +90,13 @@ func queryDNSTXT(domain, selector string, txtLookup txtLookupFunc) (*queryResult return parsePublicKey(txt) } -func parsePublicKey(s string) (*queryResult, error) { +func parsePublicKey(s string) (*QueryResult, error) { params, err := parseHeaderParams(s) if err != nil { return nil, permFailError("key syntax error: " + err.Error()) } - res := new(queryResult) + res := new(QueryResult) if v, ok := params["v"]; ok && v != "DKIM1" { return nil, permFailError("incompatible public key version") diff --git a/dkim/query_test.go b/dkim/query_test.go index 6e86de4..c0a0485 100644 --- a/dkim/query_test.go +++ b/dkim/query_test.go @@ -16,7 +16,7 @@ func init() { queryMethods["dns/txt"] = queryTest } -func queryTest(domain, selector string, txtLookup txtLookupFunc) (*queryResult, error) { +func queryTest(domain, selector string, txtLookup txtLookupFunc) (*QueryResult, error) { record := selector + "._domainkey." + domain switch record { case "brisbane._domainkey.example.com", "brisbane._domainkey.example.org", "test._domainkey.football.example.com": diff --git a/dkim/verify.go b/dkim/verify.go index eced7a2..2c94ba6 100644 --- a/dkim/verify.go +++ b/dkim/verify.go @@ -84,7 +84,7 @@ type Verification struct { Expiration time.Time // The QueryResult holds the parsed DNS response - QueryResult *queryResult + QueryResult *QueryResult // Err is nil if the signature is valid. Err error @@ -283,7 +283,7 @@ func verify(h header, r io.Reader, sigField, sigValue string, options *VerifyOpt if methodsStr, ok := params["q"]; ok { methods = parseTagList(methodsStr) } - var res *queryResult + var res *QueryResult for _, method := range methods { if query, ok := queryMethods[QueryMethod(method)]; ok { if options != nil { diff --git a/dkim/verify_test.go b/dkim/verify_test.go index 228eff6..754208e 100644 --- a/dkim/verify_test.go +++ b/dkim/verify_test.go @@ -65,7 +65,17 @@ Joe. var testVerification = &Verification{ Domain: "example.com", Identifier: "joe@football.example.com", + Selector: "brisbane", HeaderKeys: []string{"Received", "From", "To", "Subject", "Date", "Message-ID"}, + Time: time.Time{}, + Expiration: time.Time{}, + Err: nil, +} + +func newRSATestVerification() *Verification { + testQueryResult, _ := parsePublicKey(dnsPublicKey) + testVerification.QueryResult = testQueryResult + return testVerification } func TestVerify(t *testing.T) { @@ -79,7 +89,10 @@ func TestVerify(t *testing.T) { } v := verifications[0] + testVerification := newRSATestVerification() + if !reflect.DeepEqual(testVerification, v) { + t.Errorf("Expected verification to be \n%+v\n but got \n%+v", testVerification, v) } } @@ -95,6 +108,7 @@ func TestVerifyWithOption(t *testing.T) { } v := verifications[0] + testVerification := newRSATestVerification() if !reflect.DeepEqual(testVerification, v) { t.Errorf("Expected verification to be \n%+v\n but got \n%+v", testVerification, v) } @@ -144,10 +158,17 @@ Joe.` var testEd25519Verification = &Verification{ Domain: "football.example.com", Identifier: "@football.example.com", + Selector: "brisbane", HeaderKeys: []string{"from", "to", "subject", "date", "message-id", "from", "subject", "date"}, Time: time.Unix(1528637909, 0), } +func newEC25519TestVerification() *Verification { + testQueryResult, _ := parsePublicKey(dnsEd25519PublicKey) + testEd25519Verification.QueryResult = testQueryResult + return testEd25519Verification +} + func TestVerify_ed25519(t *testing.T) { r := newMailStringReader(verifiedEd25519MailString) @@ -159,6 +180,7 @@ func TestVerify_ed25519(t *testing.T) { } v := verifications[0] + testEd25519Verification := newEC25519TestVerification() if !reflect.DeepEqual(testEd25519Verification, v) { t.Errorf("Expected verification to be \n%+v\n but got \n%+v", testEd25519Verification, v) }