Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

carddav: not found #170

Closed
Marcool04 opened this issue Apr 22, 2021 · 6 comments
Closed

carddav: not found #170

Marcool04 opened this issue Apr 22, 2021 · 6 comments
Labels

Comments

@Marcool04
Copy link

Hi @emersion and all,

First of all, thanks for your great work on this very helpful floss alternative to the Protonmail bridge!
I've been using it for a while now with IMAP/SMTP with no problem, and I have just recently been trying to set up carddav with no such luck :/

I have set up Apache as a reverse proxy to forward incoming connections to the hydroxide carddav port. Upon setting up a client to access this server, I am simply getting the—rather terse— carddav: not found error mentioned in the title.
As far as I can tell from the source, this is problematic as the same error is thrown in three different places :

return "", errNotFound

return nil, errNotFound

return nil, errNotFound

The behavior I am seeing is very similar to #157
although there is no mention of openpgp in my case. By the way, I checked in the web interface to Protonmail, and my emails and contacts are encrypted with the same RSA key (the main one), which hydroxide obviously is handling fine since the emails are functioning ok on the SMTP/IMAP end.

On the server side I am seeing this:

2021/04/21 19:57:12 >> GET /api/contacts/export
2021/04/21 19:57:14 << GET /api/contacts/export
2021/04/21 19:57:14 &struct { protonmail.resp; Contacts []*protonmail.ContactExport; Total int }{resp:protonmail.resp{Code:1000, RawAPIError:(*protonmail.RawAPIError)(nil)}, Contacts:[]*protonmail.ContactExport{(*protonmail.ContactExport)(0x13dc438), (*protonmail.ContactExport)(0x13dc468), (*protonmail.ContactExport)(0x13dc498), [SNIP], (*protonmail.ContactExport)(0x10af710)}, Total:767}

and to give you an idea of the interactions the client is seeing, here is an example using the cli utility vdirsync (I have indented the xml for legibility, it is on a single line with literal \n's everywhere in the original):

$ vdirsyncer -v DEBUG sync
debug: Using 1 maximal workers.
Syncing my_contacts/[HOSTNAME]
debug: PROPFIND https://[HOSTNAME]/
debug: {'User-Agent': 'vdirsyncer/0.16.9.dev0+gb5dd092.d20201025', 'Content-Type': 'application/xml; charset=UTF-8', 'Depth': '1'}
debug: b'<?xml version="1.0" encoding="utf-8" ?>
<D:propfind xmlns:D="DAV:">
  <D:prop>
    <D:resourcetype/>
    <D:getcontenttype/>
    <D:getetag/>
  </D:prop>
</D:propfind>'
debug: Sending request...
debug: 207
debug: {'Date': 'Thu, 22 Apr 2021 06:54:23 GMT', 'Server': 'Apache', 'Strict-Transport-Security': 'max-age=15768000; includeSubDomains', 'Referrer-Policy': 'no-referrer', 'Www-Authenticate': 'Basic', 'Content-Type': 'text/xml; charset=utf-8', 'Keep-Alive': 'timeout=5, max=100', 'Connection': 'Keep-Alive', 'Transfer-Encoding': 'chunked'}
debug: b'<?xml version="1.0" encoding="UTF-8"?>
<multistatus xmlns="DAV:">
  <response xmlns="DAV:">
    <href>/</href>
    <propstat xmlns="DAV:">
      <prop xmlns="DAV:">
        <resourcetype xmlns="DAV:">
          <collection xmlns="DAV:"/>
          <addressbook xmlns="urn:ietf:params:xml:ns:carddav"/>
        </resourcetype>
      </prop>
      <status>HTTP/1.1 200 OK</status>
    </propstat>
    <propstat xmlns="DAV:">
      <prop xmlns="DAV:">
        <getcontenttype xmlns="DAV:"/>
        <getetag xmlns="DAV:"/>
      </prop>
      <status>HTTP/1.1 404 Not Found</status>
    </propstat>
    <status>HTTP/1.1 200 OK</status>
  </response>
  <response xmlns="DAV:">
    <href>/[CARDDAV_HASH].vcf</href>
    <propstat xmlns="DAV:">
      <prop xmlns="DAV:">
        <resourcetype xmlns="DAV:"/>
        <getcontenttype xmlns="DAV:">text/vcard</getcontenttype>
        <getetag xmlns="DAV:">"00"</getetag>
      </prop>
      <status>HTTP/1.1 200 OK</status>
    </propstat>
    <status>HTTP/1.1 200 OK</status>
  </response>
  <response xmlns="DAV:">
    <href>/[CARDDAV_HASH].vcf</href>
    <propstat xmlns="DAV:">
      <prop xmlns="DAV:">
        <resourcetype xmlns="DAV:"/>
        <getcontenttype xmlns="DAV:">text/vcard</getcontenttype>
        <getetag xmlns="DAV:">"00"</getetag>
      </prop>
      <status>HTTP/1.1 200 OK</status>
    </propstat>
    <status>HTTP/1.1 200 OK</status>
  </response>
  [SNIP]
  <response xmlns="DAV:">
    <href>/[CARDDAV_HASH].vcf</href>
    <propstat xmlns="DAV:">
      <prop xmlns="DAV:">
        <resourcetype xmlns="DAV:"/>
        <getcontenttype xmlns="DAV:">text/vcard</getcontenttype>
        <getetag xmlns="DAV:">"607995720"</getetag>
      </prop>
      <status>HTTP/1.1 200 OK</status>
    </propstat>
    <status>HTTP/1.1 200 OK</status>
  </response>
</multistatus>'
debug: Already normalized: '/'
debug: Skipping '/', is collection.
debug: Normalized URL from '/[CARDDAV_HASH].vcf' to '/[CARDDAV_URLENCODED_HASH].vcf'
[SNIP]
debug: Normalized URL from '/[CARDDAV_HASH].vcf' to '/[CARDDAV_URLENCODED_HASH].vcf'
debug: Already normalized: '/[CARDDAV_URLENCODED_HASH].vcf'
[SNIP]
debug: Already normalized: '/[CARDDAV_URLENCODED_HASH].vcf'
debug: REPORT https://[HOSTNAME]/
debug: {'User-Agent': 'vdirsyncer/0.16.9.dev0+gb5dd092.d20201025', 'Content-Type': 'application/xml; charset=UTF-8'}
debug: b'<?xml version="1.0" encoding="utf-8"?>
<C:addressbook-multiget xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:carddav">
  <D:prop>
    <D:getetag/>
    <C:address-data/>
  </D:prop>
  <D:href>/[CARDDAV_URLENCODED_HASH].vcf</D:href>
  <D:href>/[CARDDAV_URLENCODED_HASH].vcf</D:href>
  [SNIP]
  <D:href>/[CARDDAV_URLENCODED_HASH].vcf</D:href>
  <D:href>/[CARDDAV_URLENCODED_HASH].vcf</D:href>
</C:addressbook-multiget>'
debug: Sending request...
debug: 500
debug: {'Date': 'Thu, 22 Apr 2021 06:54:27 GMT', 'Server': 'Apache', 'Strict-Transport-Security': 'max-age=15768000; includeSubDomains', 'Referrer-Policy': 'no-referrer', 'Content-Type': 'text/plain; charset=utf-8', 'Www-Authenticate': 'Basic', 'X-Content-Type-Options': 'nosniff', 'Content-Length': '19', 'Connection': 'close'}
debug: b'carddav: not found\n'

Similar behavior is also happening with Thunderbird add-on Cardbook, and with the macOS Contacts app: they discover the service ok, create an account but then show nothing in the list of contacts when trying to sync (mind you neither one throws an error though…)
Let me know if there is anything else I can do to help debug.
Best regards,
Mark.

@emersion
Copy link
Owner

Maybe hydroxide doesn't cope well with these transformations:

debug: Normalized URL from '/[CARDDAV_HASH].vcf' to '/[CARDDAV_URLENCODED_HASH].vcf'

Can you give an example of a mismatch?

Additionally, hydroxide should send individual per-vCard errors instead of a global error, this is somewhat tracked in emersion/go-webdav#25.

@emersion emersion added the bug label Apr 23, 2021
@Marcool04
Copy link
Author

Thanks for the quick response. That's a good idea! Here is an example of what changes during vdirsyncer's URL "normalization":

Normalized URL from
'/T3xWSH8Za7JDHnRxEv0v980P8qqBz0I5xhCF1jEMqFteH4a343dMIVhf8N83Z5ARaSkWlv6-LE9vETYuTehvpQ==.vcf' to
'/T3xWSH8Za7JDHnRxEv0v980P8qqBz0I5xhCF1jEMqFteH4a343dMIVhf8N83Z5ARaSkWlv6-LE9vETYuTehvpQ%3D%3D.vcf'

In order to try and eliminate the carddav client variable, I just tried to see if the encoding makes any difference in the following way:

$ curl -u '[USERNAME]:[HYDROXIDE_TOKEN]' -H "Content-Type: text/xml" -H "Depth: 1" "https://[HOSTNAME]/T3xWSH8Za7JDHnRxEv0v980P8qqBz0I5xhCF1jEMqFteH4a343dMIVhf8N83Z5ARaSkWlv6-LE9vETYuTehvpQ%3D%3D.vcf"
BEGIN:VCARD
VERSION:4.0
item1.EMAIL;PREF=1:[EMAIL]
FN:
PRODID:-//ProtonMail//ProtonMail vCard 1.0.0//EN
UID:proton-legacy-0cae5244-6da9-4c2f-9ef4-fc7a079771b3
END:VCARD

$ curl -u '[USERNAME]:[HYDROXIDE_TOKEN]' -H "Content-Type: text/xml" -H "Depth: 1" "https://[HOSTNAME]/T3xWSH8Za7JDHnRxEv0v980P8qqBz0I5xhCF1jEMqFteH4a343dMIVhf8N83Z5ARaSkWlv6-LE9vETYuTehvpQ==.vcf"
BEGIN:VCARD
VERSION:4.0
item1.EMAIL;PREF=1:[EMAIL]
FN:
PRODID:-//ProtonMail//ProtonMail vCard 1.0.0//EN
UID:proton-legacy-0cae5244-6da9-4c2f-9ef4-fc7a079771b3
END:VCARD

But then, I guess all that means is curl is handling the encoding ok.

Could the issue then reside in the fact that vdirsyncer is trying to get a bunch of vCards at once (all 700+ maybe)?

@emersion
Copy link
Owner

Hm, so URL encoding wouldn't make a difference. Not sure what's going on then. Maybe try to add some log.Println to see what code-path is hit.

@Marcool04
Copy link
Author

Ok, so I did a bit more digging.
As far as my actual bug report in this issue goes: this is not a bug. Hydroxide is actually working fine, and I can now use a (well-behaved) client such as vdirsync to get contacts from ProtonMail through hydroxide with no issue.
However, there are some issues I uncovered.
I applied this patch:

diff --git a/carddav/carddav.go b/carddav/carddav.go
index 76c9b8d..8e680ba 100644
--- a/carddav/carddav.go
+++ b/carddav/carddav.go
@@ -18,9 +18,6 @@ import (
 	"github.com/emersion/hydroxide/protonmail"
 )
 
-// TODO: use a HTTP error
-var errNotFound = errors.New("carddav: not found")
-
 var (
 	cleartextCardProps = []string{vcard.FieldVersion, vcard.FieldProductID, "X-PM-LABEL", "X-PM-GROUP"}
 	signedCardProps    = []string{vcard.FieldVersion, vcard.FieldProductID, vcard.FieldFormattedName, vcard.FieldUID, vcard.FieldEmail}
@@ -83,8 +80,15 @@ func formatCard(card vcard.Card, privateKey *openpgp.Entity) (*protonmail.Contac
 func parseAddressObjectPath(p string) (string, error) {
 	dirname, filename := path.Split(p)
 	ext := path.Ext(filename)
-	if dirname != "/" || ext != ".vcf" {
-		return "", errNotFound
+	if dirname != "/" {
+		// TODO: use a HTTP error
+		errMsg := fmt.Sprintf("hydroxide/carddav: malformed address, dirname != /: %s", dirname)
+		return "", errors.New(errMsg)
+	}
+	if ext != ".vcf" {
+		// TODO: use a HTTP error
+		errMsg := fmt.Sprintf("hydroxide/carddav: malformed address, extension is not .vcf but: %s", ext)
+		return "", errors.New(errMsg)
 	}
 	return strings.TrimSuffix(filename, ext), nil
 }
@@ -182,12 +186,13 @@ func (b *backend) GetAddressObject(path string, req *carddav.AddressDataRequest)
 	contact, ok := b.getCache(id)
 	if !ok {
 		if b.cacheComplete() {
-			return nil, errNotFound
+			errMsg := fmt.Sprintf("hydroxide/carddav: could not find contact id %s in cache, despite cacheComplete", id)
+			return nil, errors.New(errMsg)
 		}
 
 		contact, err = b.c.GetContact(id)
 		if apiErr, ok := err.(*protonmail.APIError); ok && apiErr.Code == 13051 {
-			return nil, errNotFound
+			return nil, errors.New("hydroxide/carddav: got API error code 13051 (whatever that means…)")
 		} else if err != nil {
 			return nil, err
 		}
@@ -291,7 +296,8 @@ func (b *backend) PutAddressObject(path string, card vcard.Card) (loc string, er
 			return "", err
 		}
 		if len(resps) != 1 {
-			return "", errors.New("hydroxide/carddav: expected exactly one response when creating contact")
+			errMsg := fmt.Sprintf("hydroxide/carddav: expected exactly one response when creating contact, got %x", len(resps))
+			return "", errors.New(errMsg)
 		}
 		resp := resps[0]
 		if err := resp.Err(); err != nil {
@@ -316,7 +322,8 @@ func (b *backend) DeleteAddressObject(path string) error {
 		return err
 	}
 	if len(resps) != 1 {
-		return errors.New("hydroxide/carddav: expected exactly one response when deleting contact")
+		errMsg := fmt.Sprintf("hydroxide/carddav: expected exactly one response when deleting contact, got %x", len(resps))
+		return errors.New(errMsg)
 	}
 	resp := resps[0]
 	// TODO: decrement b.total if necessary

Which allowed me to see that quite a few clients (in particular Thundirbird with CardBook extension), perform a:

PROPFIND /.well-known/carddav HTTP/1.1

initially to set up the connection and therefore fail, since – as per discussion in #44 and emersion/go-webdav#30 – hydroxide doesn't implement the .well-known paths yet. This in fact can be easily worked around with a redirect at the webserver level (which might be work mentioning in the installation instructions), something like this for nginx:

server {
    …
    rewrite ^/.well-known/carddav$ https://[servername]/ permanent;
    …
}

Other issue I discovered with macOS contacts handling of CardDAV: there is a checkbox that can be ticked to "use SSL", buuuut unticking this makes the traffic into garbage. I watched in Wireshark as macOS attempted to contact the server, and no http traffic was generated at all, just giberich that made the reverse proxy answer with "Invalid request".
To get CardDAV to work on macOS, one therefore needs to setup a self-signed certificate (or letsencrypt, or whatever), so that the reverse proxy is talking HTTPS to the macOS client 🤷‍♂️ Note caveats if you use a self-signed cert: they will need trusting. In Thunderbird/CardBook this is documented in section 3. "Configuration with https and a self-signed certificate" of these instructions: https://gitlab.com/CardBook/CardBook/-/wikis/server-configuration#--2

That's about all from me. Closing as technically it's possible to get everything working with hydroxide just the way it is now.

Maybe the improvements to the error messages I made could be used? (although I'm not exactly golang fluent – the code above compiled and worked as expected, but it may well contain errors).

In any case, I think it would serve others if both the redirect for .well-known and the possible importance of SSL for a number of common clients were documented directly in the installation instructions in README.md?

@emersion
Copy link
Owner

emersion commented May 3, 2021

I applied this patch:

Pro tip: you can use fmt.Errorf instead of doing it in two steps.

Maybe the improvements to the error messages I made could be used?

Yup -- PRs welcome.

In any case, I think it would serve others if both the redirect for .well-known and the possible importance of SSL for a number of common clients were documented directly in the installation instructions in README.md?

I'd rather get well-known implemented, than documenting a workaround in the README. I'd rather not add client-specific instructions to the README, either.

@Marcool04
Copy link
Author

Pro tip: you can use fmt.Errorf instead of doing it in two steps.

Oh thanks ☺️ shows the extent of my golang knowledge… I'll do that and put it all in a PR.

I'd rather get well-known implemented, than documenting a workaround in the README. I'd rather not add client-specific instructions to the README, either.

I understand that. But wouldn't a warning about the occasional need for SSL be helpful? Since it's not really something that can be debugged from the messages that hydroxide will produce and people will be thinking (I would imagine) that it is hydroxide's fault, when it has to do with the client requiring SSL? I realize that is certainly a bug on the part of the client but hey 🤷‍♂️ I gave up reporting stuff to Apple a looong time ago!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants