Skip to content

Conversation

@rrrooommmaaa
Copy link
Contributor

Closes #3003

First checks for policy file, then extracts the key and checks for email match.
Advanced method is now supported.

Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, a small change regarding reporting

@tomholub
Copy link
Collaborator

It's really likely misconfiguration on his end. Policy has no CORS:

$ curl --header 'Host: somehost.com' -I https://metacode.biz/.well-known/openpgpkey/policy
HTTP/2 200 
server: nginx
date: Sat, 17 Oct 2020 12:22:00 GMT
content-type: application/octet-stream
content-length: 0
last-modified: Fri, 16 Feb 2018 12:16:51 GMT
etag: "5a86cbb3-0"
strict-transport-security: max-age=31536001; includeSubDomains; preload
x-xss-protection: 1; mode=block; report=https://metacode.biz/collector/log
x-content-type-options: nosniff
x-frame-options: DENY
content-security-policy-report-only: default-src https: data:; object-src 'none'; script-src https: 'unsafe-inline'; style-src https: 'unsafe-inline'; report-uri https://metacode.biz/collector/log
expect-ct: max-age=86400,report-uri="https://metacode.biz/collector/log"
vary: Accept
accept-ranges: bytes

But the end file has proper cors:

$ curl --header 'Host: somehost.com' -I https://metacode.biz/.well-known/openpgpkey/hu/gebusffkx9g581i6ch4t3ewgwd6dctmp?l=wiktor
HTTP/2 200 
server: nginx
date: Sat, 17 Oct 2020 12:23:26 GMT
content-type: application/pgp-keys
content-length: 6141
last-modified: Wed, 04 Dec 2019 11:02:39 GMT
etag: "5de7924f-17fd"
content-disposition: inline; filename="D8E8F074.gpg"
access-control-allow-origin: *
accept-ranges: bytes

Therefore we should still be polling the policy file if we want to follow the spec.

To be fair, if a lot of WKD servers are misconfigured also, we may drop reading the policy file. For now, let's try to follow.

@tomholub
Copy link
Collaborator

I will switch it to test another email

tomholub
tomholub previously approved these changes Oct 18, 2020
@tomholub
Copy link
Collaborator

Thanks! I'll have a look at the remaining error report, it may be hard for you to hunt down.

@tomholub tomholub self-assigned this Oct 18, 2020
}
return { pubkey: null, pgpClient: null };
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rrrooommmaaa I think the error was here: if I'm reading it correctly, it was possible for the for loop to finish and validUrl would remain undefined. Then the lines below would continue calling that URL like undefined/hu/... causing the error reports.

I recommend to try to avoid for loops that do accounting in this manner unless one really cannot avoid them. It can be hard to spot bugs. I've rewritten it as a method that gets called two times explicitly with two different parameters (advanced vs direct url), which I think will make for an easier to debug structure.

@tomholub
Copy link
Collaborator

Btw I like this test

image

@tomholub
Copy link
Collaborator

I'll do some more improvements to the browser tests in general while I'm at it (our tests are a bit complex, sorry about that)

@tomholub tomholub merged commit 3520f8d into master Oct 18, 2020
@tomholub tomholub deleted the issue-3003-advanced-wkd branch November 2, 2020 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

should also search on advanced WKD url + use policy file

4 participants