Multiple clients in sasl-xoauth2.conf ?

[fragtion]

Hi,

Firstly huge thanks for this great module

While it seems possible to have multiple tokens (and assign those in sasl_passwd accordingly), it does not seem possible to have multiple client credentials in /etc/sasl-xoauth2.conf

If I have several Gmail accounts, each with its own OAuth2 "Installed application" (unique Client ID, Client Secret, Refresh Token and Access Tokens), how can those all be utilized currently if sasl-xoauth2.conf only seems to support a single Client ID and secret pair currently?

I guess I could ask, why need an /etc/sasl-xoauth2.conf anyway, if the tokens file could contain the Client ID & Client Secret also. Or similarly, why need individual tokens files, if we could populate /etc/sasl-xoauth2.conf with all token pairs as well ?

[tarickb]

Can those different Gmail accounts not share the same client credentials? In principle those client credentials identify the application, not the user, and should be usable by more than one user. That's why they're split between /etc/sasl-xoauth2.conf and the individual token files. Token files also may need to have different permissions from the config file.

[fragtion]

Oh brilliant, I didn't realize another user could authenticate to the existing user's app. But obviously that is possible... Silly me! Thanks for the response! I'll close this ticket.

[valinet]

I don’t think this should be closed; specifically, I don’t understand what you’re supposed to do if you need to provide OAuth for 2 providers, like Gmail and Yahoo!, which obviously requires providing 2 secrets, endpoints etc. What’s to do in this situation? The answer I could think right now is to compile the plugin twice with a different name, but this seems more like a workaround than a solution.

[tarickb]

Yeah, good point. I've added the ability to override client ID/secret and token endpoint in the token file itself. I'll commit that change in a few minutes.

[valinet]

Wow, sounds really good, thank you very much, and also, many thanks for this awesome project; I will check the new version later today.

[valinet]

Hi

I have tested the new version, it seems to work fine. I noticed an oddity though, I think. When I filled in the details in my token file, I put a wrong token_endpoint in there (I put in the authentication URL which returns the sign in page instead of the URL to the token API). This, of course, caused the refresh token request to fail, but it did not fail because of a non expected response from the server (which should log something like TokenStore::Refresh: exception=* Line 1, Column 1#012 Syntax error: value, object or array expected.#012), but because of an error in the RequestContext::Seek function. It seemed odd to me, so I took a look at it, and it returned 1, which is CURL_SEEKFUNC_FAIL because either origin was not SEEK_SET, either offset was not 0. After simple code edits, I determined it fails because origin was not SEEK_SET. In fact, origin was not 0, 1, or 2 (SEEK_SET, SEEK_CUR, SEEK_END), but rather "-1267132384" which looked very odd. To me, it looks like an address. Odd, so I looked at the prototype of your Seek function:

static int Seek(off_t offset, int origin, void *context)

Meanwhile, in curl/curl.h, they specify this:

typedef int (*curl_seek_callback)(void *instream,
                                  curl_off_t offset,
                                  int origin); /* 'whence' */

So, "instream" in curl's prototype is "offset" in yours. "instream" is suggested to be like an fd to the file, or in your implementation, it is an address to this class that represents this file, I think, same idea anyway. And indeed, curl seemed to use the prototype defined in its header because "offset" has that "weird" value. So, are you sure the prototype you supplied for RequestContext::Seek is correct? Shouldn't it be static int Seek(void *context, off_t offset, int origin) instead?

The curl docs say that the seek function being called "may happen when doing an HTTP PUT or POST with a multi-pass authentication method, or when an existing HTTP connection is reused too late and the server closes the connection". To me, it seems that under normal operation that seek function is never called because data fits in the packet size/MTU etc, but by me supplying that wrong token URL, I made curl have to call it and so I discovered this problem.

This is not a major issue, because as I said, that function, under normal conditions is never called, but I still suggest you look into it anyway, for the sake of correctness. Also, while we're at this, why does the function need offset to be 0 to succeed? To me, not checking for that, and calling Rewind(offset) seems more logical. Then, the Rewind function could be something like this:

  void Rewind(off_t offset) {
    to_server_next_ = to_server_.c_str() + offset;
    to_server_remaining_ = to_server_.size() - offset;
  }

The function just takes into account an offset where you want to go. In the constructor of RequestContext, just change Rewind() to Rewind(0) then at it compiles fine, although indeed, I haven't tested this too much. On the wrong page, syslog output showed that the reply was the sign in page, so maybe the POST request was actually complete that Google decided to reply with the sign in page after all? (syslog also trims the output to a couple of lines only).

Then, does the sasl-xoauth2.conf make sense anymore? I mean, the way I see it, it serves as a default and individual tokens can override the default, but I think a better approach would be to specify something in the token file like: provider: "google", or provider: "yahoo",, and then, in sasl-xoauth2.conf have something like:

  "providers": [
    {
      "provider": "google",
      "client_id": "id_google",
      "client_secret": "secret_google"
    },
    {
      "provider": "yahoo",
      "client_id": "id_yahoo",
      "client_secret": "secret_yahoo"
    }
  ],
  "log_full_trace_on_failure": "yes"

But I understand if you do not want/have the time to make the change, it is a bit of stuff to be done, and to be honest, if people organize their token files properly, they can use some shell scripts to mass replace outdated data in all tokens belonging to a certain provider. For me, I don't do "industrial mailing", I have a couple of addresses and the current implementation works more than fine for my needs, but yeah... I still think it would be a really good enhancement for this awesome project you have.

Lastly, on my install, I have to do curl_easy_setopt(curl, CURLOPT_CAPATH, "/etc/ssl/certs"); or the more dangerous curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, false);, otherwise it fails verifying the certificate of any provider. My question is, do you happen to know if the setting is taken automatically from postfix, and if so, how to set it in postfix? If not, should this then be added to your plugin as well?

Anyway, what do you think? Sorry for bombarding you like that, if you want me to, I can open separate issues.

Again, thank you very much for your work and your support.

[Wasca]

@valinet

Not sure what environment you're using this in but in Ubuntu 20.04 when Postfix is started or restarted it copies a bunch of certificates into /var/spool/postfix/etc/ssl/certs/

It's this script that is called at start up that does the copying /usr/lib/postfix/configure-instance.sh. It looks like it does not copy over the ca-certificates.crt file which appears to be needed. I wonder if your certificate verification failure issue is related to that.

I created a post here about it. #13

[valinet]

@valinet

Not sure what environment you're using this in but in Ubuntu 20.04 when Postfix is started or restarted it copies a bunch of certificates into /var/spool/postfix/etc/ssl/certs/

It's this script that is called at start up that does the copying /usr/lib/postfix/configure-instance.sh. It looks like it does not copy over the ca-certificates.crt file which appears to be needed. I wonder if your certificate verification failure issue is related to that.

I created a post here about it. #13

Thanks, I saw your post some time ago, I implemented your changes but did not work fully. I stopped at a certain point to be honest, for now, everything works with that curl_easy_setopt(curl, CURLOPT_CAPATH, "/etc/ssl/certs"); line in this plugin and I am fine with it, I don't want to waste any more time on this. Anyway, thank you for the heads-up.

[tarickb]

Thanks @valinet for pointing out the issue with the seek callback! I'd copied that from another project of mine where I also got it wrong. I'll avoid implementing the seek-for-a-non-zero-offset capability for now because I don't think it's actually ever required.

As for the CA certificate file/path problem, I'll open a separate issue for that. I'm thinking a new sasl-xoauth2.conf option specifying a value for either CURLOPT_CAPATH or CURLOPT_CAINFO (depending on the user's config) is the right way to go.

And I like your suggestion to specify a provider dictionary in the config file. I'll open another issue for that and I'll try to get around to it sometimes soon.

[valinet]

@tarickb Thank you very much for your support and for taking a look at this.