Closed Marcool04 closed 3 years ago
emersion
commented
3 years ago 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 https://github.com/emersion/go-webdav/issues/25.
Marcool04
commented
3 years ago 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:VCARDBut 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
commented
3 years ago 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
commented
3 years ago 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 necessaryWhich allowed me to see that quite a few clients (in particular Thundirbird with CardBook extension), perform a:
PROPFIND /.well-known/carddav HTTP/1.1initially to set up the connection and therefore fail, since – as per discussion in https://github.com/emersion/hydroxide/issues/44 and https://github.com/emersion/go-webdav/issues/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
commented
3 years ago 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
commented
3 years ago Pro tip: you can use
fmt.Errorfinstead 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!
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 founderror 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 : https://github.com/emersion/hydroxide/blob/624359393914bf558053881d3b111ed77f1cba34/carddav/carddav.go#L87 https://github.com/emersion/hydroxide/blob/624359393914bf558053881d3b111ed77f1cba34/carddav/carddav.go#L185 https://github.com/emersion/hydroxide/blob/624359393914bf558053881d3b111ed77f1cba34/carddav/carddav.go#L190The behavior I am seeing is very similar to https://github.com/emersion/hydroxide/issues/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:
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):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.