JOSM sends authentication info from one WMTS source into another

[ssundell]

What steps will reproduce the problem?

1. I have two WMTS sources that require (basic) authentication.
2. I add a layer from source A. JOSM shows an authentication dialog with information I've filled previously and stored.
3. I add the default layer from source B. JOSM shows an authentication dialog with empty fields since I have not stored authentication info for this source. I enter the username and password.

What is the expected result?

Layers from both sources A and B are retrieved and shown using their respective authentication details.

What happens instead?

When source B is selected, JOSM immediately launches a series of GET requests for the WMTS tiles, using authentication data for source A. The server responds with 401 and ends up blocking me altogether for trying to brute force my way in... :D

Please provide any additional information below. Attach a screenshot if possible.

I have a personal mapproxy to cushion bumps in imagery availability, and after getting blocked by an overeager firewall I noticed its logs has connection attempts from JOSM using a username from another, unrelated WMTS service.

This seems similar to #7086 except with two WMTS sources instead of OSM and WMS of the original ticket.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2025-02-03 20:59:54 +0100 (Mon, 03 Feb 2025)
Revision:19307
Build-Date:2025-02-04 02:30:32
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (19307 en) Linux Mint 21.3
Memory Usage: 384 MB / 23808 MB (78 MB allocated, but free)
Java version: 21.0.6+7-Ubuntu-122.04.1, Ubuntu, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920x1080x[Multi depth]@60Hz (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: en_US.UTF-8
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_US
Numbers with default locale: 1234567890 -> 1234567890
Desktop environment: X-Cinnamon
Java package: openjdk-21-jre:amd64-21.0.6+7-1~22.04.1
fonts-noto: fonts-noto:all-20201225-1build1
VM arguments: [--module-path=/usr/share/openjfx/lib, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, -Djosm.restart=true, -Djava.net.useSystemProxies=true, -XX:MaxRAMPercentage=75.0, --add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED]
Dataset consistency test: No problems found

Plugins:
+ ImageIO (36379)
+ buildings_tools (36382)
+ utilsplugin2 (36379)

Map paint styles:
+ https://josm.openstreetmap.de/josmfile?page=Styles/iD&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Surface&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Surface-DataEntry&zip=1

Last errors/warnings:
- 00031.302 W: No default layer selected, choosing first layer.
- 00050.469 W: No default layer selected, choosing first layer.
- 00101.173 W: java.net.SocketTimeoutException: Connect timed out
- 00101.173 W: java.net.SocketTimeoutException: Connect timed out
- 00412.497 W: No default layer selected, choosing first layer.

[stoecker]

Sounds serious, but I don't know of a single WMS/WMTS needing authentication, so I can test at all and I don't have time to setup a specific test environment.

[ssundell]

Ok, I'm making a bit too many assumptions here since I'm not yet familiar with the codebase, and I'm still missing a piece here, but after some digging, here are some clues:

For starters, CredentialsAgent defines itself as being used for two kinds of authentication: authentication for OSM API and authentication for a proxy server. As evidenced by the doc:

A CredentialsAgent manages two credentials:

• the credential for {@link RequestorType#SERVER} which is equal to the OSM API credentials in JOSM
• the credential for {@link RequestorType#PROXY} which is equal to the credentials for an optional HTTP proxy server a user may use

Except!

The default implementation, JosmPreferencesCredentialAgent, stores any credentials; it just creates a prefs key based on the host name and tries to find stored credentials.

Also, if HttpClient encounters an unauthorized URL, it stores the host into DefaultAuthenticator

if (DefaultAuthenticator.getInstance().isEnabled() && cr.getResponseCode() == HttpURLConnection.HTTP_UNAUTHORIZED) {
    DefaultAuthenticator.getInstance().addFailedCredentialHost(url.getHost());
}

... and `DefaultAuthenticator stores the host with type SERVER:

failedCredentials.add(Pair.create(host, RequestorType.SERVER));

Now then, what happens in credential lookup?

AbstractCredentialsAgent:

        /*
         * Last request was successful and there was no credentials stored in file (or only the username is stored).
         * -> Try to recall credentials that have been entered manually in this session.
         */
        if (!noSuccessWithLastResponse && memoryCredentialsCache.containsKey(requestorType) &&
                (credentials == null || credentials.getPassword() == null || credentials.getPassword().length == 0)) {
            PasswordAuthentication pa = memoryCredentialsCache.get(requestorType);
            response.setUsername(pa.getUserName());
            response.setPassword(pa.getPassword());
            response.setCanceled(false);
        /*
         * Prompt the user for credentials. This happens the first time each
         * josm start if the user does not save the credentials to preference
         * file (username=="") and each time after authentication failed
         * (noSuccessWithLastResponse == true).
         */
        } else if (noSuccessWithLastResponse || username.isEmpty() || password.isEmpty()) {
          ...

The problem here is that memoryCredentialsCache is only accessed using the RequestorType, so if there are multiple servers that require authentication and one of them is authenticated but not stored in the preferences file - or if its username or password is empty there - those cached credentials could be blindly used to any host that requires authentication.

The piece that I'm missing: there's the clause !noSuccessWithLastResponse when accessing memoryCredentialsCache, and that is set based on whether the host returned 401. That info gets cleared after the credentials have been retrieved (whether from user, from file or from cache), but at that point the correct info should be already in place.

That's, of course, assuming things work synchronously, which is what the code here expects, and considering it's HTTP requests, might not be completely true. Or I might be missing something, or be completely on a wrong track :D

In any case, even if my speculation here isn't the culprit for this bug, I think the memoryCredentialsCache should be changed to also take into account the host; even if everything worked perfectly, in the current form if you have two authenticated sources and you don't store their authentication info in preferences, only one of them will work since the password prompt will always overwrite the previous contents of the cache for RequestorType.SERVER.

[ssundell]

Thinking about this a bit more, there's at least one way to "circumvent" the !noSuccessWithLastResponse clause: if both authenticated services are using the memory cache.

So

1. Authenticate to service A, don't store credentials
2. Authenticate to service B, don't store credentials
   • This overwrites service A's credentials in memoryCredentialsCache
3. Service A is contacted using service B's credentials in Authorization header.

Also, to note: "don't store credentials" is not the only thing that triggers this: even if the credentials are stored, the username.isEmpty() || password.isEmpty() clause in the next if check means that user is presented with authentication dialog if, for example, the stored credentials have no password, and the result is stored in memoryCredentialsCache.

[stoecker]

I think the memoryCredentialsCache should be changed to also take into account the host

I didn't yet verify the other parts of what you wrote, but in any case this sounds like a sane proposal. Can you provide a patch?

[stoecker]

While it is a security risk, it is outside the typical use as OSM editor, so still only normal priority.

[ssundell]

I'll take a look at it; the fix itself seems simple, the only problem is the purge method and the fact that it's defined in the interface, which could, in theory, be implemented by some plugin. Maybe I'll add a default method that calls the current implementation, just to be safe...

[ssundell]

Ok, here goes. I changed the cache to have a key that contains both the requestor type and the host. I added a method for purging just singles hosts into the CredentialsAgent interface and added a default method that calls the old method to make sure this doesn't break possible plugins that implement the interface. It looks like the old method could be deprecated, since the only legitimate use for it (removing all entries of a specific requestor type) seem to be in tearing down tests.

I gave this a spin on my environment and it seems to work as it should, ie. authenticating both services that require credentials as should, and it didn't leak credentials from one service to another anymore.

For some reason JOSM still occasionally calls an authenticated service without credentials, but that's minor and separate from this.

[stoecker]

Don't use import java.util.*;, otherwise looks good on first review.

[ssundell]

A patch for extending memoryCredentialsCache to store credentials for each host separately.

[ssundell]

Fixed, and noticed the checkstyle, so removed a couple of extra empty lines as well.

[stoecker]

In 19345/josm:

don't send authentication oinformation to wrong server, fix #24149, patch by ssundell