Negotiate/NTLM auth ignores an application-supplied `Authorization` header (duplicate `Authorization` sent)

[sdgh179]

I did this

We are upgrading from libcurl 8.19.0 to 8.21.0 and several of our CI tests started to fail with 401 status.
Investigation have shown that under certain conditions libcurl ignores existing "Autorization: Bearer ..."
(or any other auth header) on a request and injects "Authorization: Negotiate" regardless resulting in
multiple authorization headers.

Repro (attachments don't work, pasting inline):

#include <winsock2.h>
#include <ws2tcpip.h>
#include <windows.h>
#include <process.h>
#include <stdio.h>
#include <string.h>
#include <curl/curl.h>
#pragma comment(lib, "ws2_32.lib")

static SOCKET g_listen;
static int g_auth_count, g_has_negotiate;

static unsigned __stdcall server(void *a) {
    (void)a;
    SOCKET c = accept(g_listen, NULL, NULL);
    if (c == INVALID_SOCKET) return 0;
    char buf[8192]; int total = 0;
    for (;;) {
        int n = recv(c, buf + total, (int)sizeof(buf) - 1 - total, 0);
        if (n <= 0) break;
        total += n; buf[total] = 0;
        if (strstr(buf, "\r\n\r\n")) break;
    }
    char *line = buf;
    while (*line) {
        char *eol = strstr(line, "\r\n");
        int len = eol ? (int)(eol - line) : (int)strlen(line);
        if (len >= 14 && _strnicmp(line, "Authorization:", 14) == 0) {
            printf("  server received: %.*s\n", len, line);
            g_auth_count++;
            const char *v = line + 14; while (*v == ' ') v++;
            if (_strnicmp(v, "Negotiate", 9) == 0) g_has_negotiate = 1;
        }
        if (!eol) break;
        line = eol + 2;
    }
    const char *r = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
    send(c, r, (int)strlen(r), 0);
    closesocket(c);
    return 0;
}

static size_t sink(char *p, size_t s, size_t n, void *u){(void)p;(void)u;return s*n;}
static int trace(CURL *h, curl_infotype t, char *d, size_t n, void *u){
    (void)h;(void)u;
    if (t == CURLINFO_HEADER_OUT){ printf("  curl sent: "); fwrite(d,1,n,stdout);
        if(!n||d[n-1]!='\n')putchar('\n'); }
    return 0;
}

int main(void){
    WSADATA w; WSAStartup(MAKEWORD(2,2), &w);
    printf("%s\n\n", curl_version());
    g_listen = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    struct sockaddr_in a; memset(&a,0,sizeof(a));
    a.sin_family=AF_INET; a.sin_addr.s_addr=htonl(INADDR_LOOPBACK);
    bind(g_listen,(struct sockaddr*)&a,sizeof(a));
    int al=sizeof(a); getsockname(g_listen,(struct sockaddr*)&a,&al);
    unsigned short port=ntohs(a.sin_port);
    listen(g_listen,1);
    HANDLE th=(HANDLE)_beginthreadex(NULL,0,server,NULL,0,NULL);
    char url[64]; sprintf(url,"http://127.0.0.1:%u/",port);

    curl_global_init(CURL_GLOBAL_DEFAULT);
    CURL *e = curl_easy_init();
    curl_easy_setopt(e, CURLOPT_URL, url);
    curl_easy_setopt(e, CURLOPT_WRITEFUNCTION, sink);
    curl_easy_setopt(e, CURLOPT_VERBOSE, 1L);
    curl_easy_setopt(e, CURLOPT_DEBUGFUNCTION, trace);
    curl_easy_setopt(e, CURLOPT_HTTPAUTH, (long)CURLAUTH_NEGOTIATE);
    curl_easy_setopt(e, CURLOPT_USERNAME, "user");
    curl_easy_setopt(e, CURLOPT_PASSWORD, "pass");
    struct curl_slist *h = curl_slist_append(NULL, "Authorization: Bearer XYZ");
    curl_easy_setopt(e, CURLOPT_HTTPHEADER, h);

    printf("--- perform ---\n");
    curl_easy_perform(e);
    curl_slist_free_all(h);
    curl_easy_cleanup(e);
    curl_global_cleanup();
    closesocket(g_listen);
    WaitForSingleObject(th, 2000); CloseHandle(th); WSACleanup();

    printf("\n");
    if (g_auth_count > 1 && g_has_negotiate) {
        printf("FAIL: two Authorization headers on the request.\n");
        return 1;
    }
    printf("OK: single Authorization header.\n");
    return 0;
}

This reproes regardless on 8.19 and 8.21 (we have a more convoluted repro that reproduces exact behavior of our code, but I don't feel it's important here).

What fixed it for us is a following change in output_auth_headers() (lib/http.c):

#ifdef USE_SPNEGO
  if(authstatus->picked == CURLAUTH_NEGOTIATE) {
    /* Negotiate */
    if(proxy || !Curl_checkheaders(data, STRCONST("Authorization"))) {
      auth = "Negotiate";
      result = Curl_output_negotiate(data, conn, proxy);
      if(result)
        return result;
    }
    else
      /* the application set its own Authorization header */
      authstatus->done = TRUE;
  }
  else
#endif
#ifdef USE_NTLM
  if(authstatus->picked == CURLAUTH_NTLM) {
    /* NTLM */
    if(proxy || !Curl_checkheaders(data, STRCONST("Authorization"))) {
      auth = "NTLM";
      result = Curl_output_ntlm(data, proxy);
      if(result)
        return result;
    }
    else
      /* the application set its own Authorization header */
      authstatus->done = TRUE;
  }
  else
#endif

Note: unlike the single-shot Basic/Bearer branches (which set authstatus->done = TRUE
unconditionally), the done here is only set on the skip path: Curl_output_negotiate()
and Curl_output_ntlm() manage authstatus->done themselves across the multi-leg
handshake, so it must not be overwritten when they run.

Now on the more conceptual level: one could argue that setting both curl_easy_setopt(e, CURLOPT_HTTPAUTH, (long)CURLAUTH_NEGOTIATE); and curl_easy_setopt(e, CURLOPT_HTTPHEADER, curl_slist_append(NULL, "Authorization: Bearer XYZ")); is pointless.

While it certainly may look so, I have following to say:

1. We did have it in the production code for years because it's allowed and simpler (we have a "factory" for requests that prefill certain options, then each particular caller fills out the rest). And it worked before 8.21
2. It is allowed. CURLOPT_HTTPHEADER.md` has following:

If you provide a header that is otherwise generated and used by libcurl internally, your header alternative is used instead.

And Basic/Bearer branches in output_auth_headers already respect this.
3. There is no legitimate reason to ever have two Authorization headers anyway. If there is a user provided one - adding a new one just makes request malformed

I expected the following

Single Authorization: ... header

curl/libcurl version

libcurl 8.21.0

operating system

Windows 11/Windows Server 2025

[bagder]

Asking for CURLAUTH_NEGOTIATE and setting an Authorization header of course begs the question which one that should "win". I suppose both can actually get sent.

What does this test want to have accomplished here?

[sdgh179]

Test is the isolation of a regression in a much more convoluted production code we have. Our actual scenario involved reused connections and code that was masked by USE_NTLM macro (but weirdly not NTLM-related itself). As I admitted in the OP, our particular set of options does indeed seem strange, but we had it and it worked until v8.21, so I wasn't sure if I need to put fix in our code or in curl, but CURLOPT_HTTPHEADER documentation convinced me that fix belongs in libcurl.

Explicit user provided "Authorization: ..." per CURLOPT_HTTPHEADER.md should override any automatically injected ones:

If you provide a header that is otherwise generated and used by libcurl internally, your header alternative is used instead.

In fact Basic and Bearer branches of that same function already do exactly that.

IIUC generating multiple "Authorization" headers is a MUST NOT in RFC:

aside from the well-known exception noted below, a sender MUST NOT generate multiple field lines with the same name in a message

[bagder]

@sdgh179 any chance you can test the patch in #22622 and see if that fixes the problem for you?

[sdgh179]

@bagder Thanks for quick turnaround. I'll test it right now

[sdgh179]

@bagder Everything goes from red to green on our side with #22622

[bagder]

Excellent, thanks for confirming. Merging!