Base64 encoded string escaping is broken on secret post and patch in 1.0.0-beta4

[Bas83]

Basically the same issue as already reported in #233, note that even the solution is already provided here, as far as I can tell the assessment is correct.

The issue is that Base64 padding characters, meaning the = sign, are replaced by \u003d
But I'd like to add that this happens for both create and update. And I have made a test class that fully reproduces the issue just using the client with version 1.0.0-beta4.

Note that the problem only occurs when the base64 encoded string is PADDED with = or ==. This means that it does work for many possible values. The example values in my test class are of course failing.

The problem occurs in 2 places in this test:

For the 'auth' object in create, even when using the easier stringData way of passing the config of the secret.

The request being made:

{"apiVersion":"v1","kind":"Secret","metadata":{"name":"myregistrysecret1543901145"},"stringData":{".dockerconfigjson":"{"auths":{"docker.io":{"username":"1x","password":"1x","auth":"MXg6MXg\u003d"}}}"},"type":"kubernetes.io/dockerconfigjson"}

Note the \u003d in the request. Also note this actually silently fails. Even pulling an image will still work as kubernetes doesn't seem to look at the auth object but rather the username and password field.

For the whole dockerconfigjson object when trying to patch the secret.

The request being made:

[{"op":"replace","path":"/data","value":{".dockerconfigjson":"eyJhdXRocyI6eyJkb2NrZXIuaW8iOnsidXNlcm5hbWUiOiJYMiIsInBhc3N3b3JkIjoiWDJ4IiwiYXV0aCI6IldESTZXREo0In19fQ\u003d\u003d"}}]

Note the \u003d in the request.

The error:

{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"v1.Secret: ObjectMeta: v1.ObjectMeta: TypeMeta: Kind: Data: decode base64: illegal base64 data at input byte 102, parsing 163 ...03d\u003d"... at {"apiVersion":"v1","data":{".dockerconfigjson":"eyJhdXRocyI6eyJkb2NrZXIuaW8iOnsidXNlcm5hbWUiOiJYMiIsInBhc3N3b3JkIjoiWDJ4IiwiYXV0aCI6IldESTZXREo0In19fQ\u003d\u003d"},"kind":"Secret","metadata":{"name":"myregistrysecret1543901145","namespace":"default","uid":"b7140c25-42d7-11e8-85fb-00155d380106","resourceVersion":"186505","creationTimestamp":"2018-04-18T07:11:29Z"},"type":"kubernetes.io/dockerconfigjson"}","code":500}

Here is my code:

TestApp.zip

And as text:

import com.google.gson.JsonObject;
import io.kubernetes.client.ApiClient;
import io.kubernetes.client.Configuration;
import io.kubernetes.client.apis.CoreV1Api;
import io.kubernetes.client.models.V1ObjectMeta;
import io.kubernetes.client.models.V1Secret;
import io.kubernetes.client.util.Config;

import java.util.*;

public class TestApp {

    private final static String KUBE_CONFIG_LOCATION = "C:\\\\Users\\\\Sebastiaan\\\\.kube\\\\config";
    private final static String SECRET_NAME = "myregistrysecret" + new Random().nextInt();;

    public static void main(String[] args) throws Exception {
        ApiClient client = Config.fromConfig(KUBE_CONFIG_LOCATION);
        client.setDebugging(true);
        Configuration.setDefaultApiClient(client);
        String username = "1x";
        String password = "1x";
        createSecret(username, password);
        username = "X2";
        password = "X2x";
        updateSecret(username, password);
    }

    private static void createSecret(String username, String password) throws Exception {
        CoreV1Api api = new CoreV1Api();
        V1Secret secret = new V1Secret();
        secret.setApiVersion("v1");
        secret.setKind("Secret");
        secret.setType("kubernetes.io/dockerconfigjson");
        V1ObjectMeta secretMeta = new V1ObjectMeta();
        secretMeta.setName(SECRET_NAME);
        secret.setMetadata(secretMeta);

        String dockerConfigJson = getDockerConfigJson(username, password).toString();

        Map<String, String> data = new HashMap<>();
        data.put(".dockerconfigjson", dockerConfigJson);
        secret.setStringData(data);
        System.out.println("docker config json on create:" + dockerConfigJson);
        api.createNamespacedSecret("default", secret, "false");
    }

    private static void updateSecret(String username, String password) throws  Exception {
        CoreV1Api api = new CoreV1Api();
        ArrayList<JsonObject> patchObjects = new ArrayList<>();
        JsonObject patchObject = new JsonObject();
        JsonObject dataObject = new JsonObject();
        String nonEncodedConfig = getDockerConfigJson(username, password).toString();
        String encodedConfig = Base64.getEncoder().encodeToString(nonEncodedConfig.getBytes());
        System.out.println("docker config json in patch not encoded:" + nonEncodedConfig);
        System.out.println("docker config json in patch:" + encodedConfig);
        dataObject.addProperty(".dockerconfigjson", encodedConfig);
        patchObject.addProperty("op", "replace");
        patchObject.addProperty("path", "/data");
        patchObject.add("value", dataObject);
        patchObjects.add(patchObject);
        System.out.println("patch objects tostring: " + patchObjects.toString());
        api.patchNamespacedSecret(SECRET_NAME, "default", patchObjects, "false");
    }

    private static JsonObject getDockerConfigJson(String username, String password) {
        String host = "docker.io";

        JsonObject authObject = new JsonObject();
        authObject.addProperty("username", username);
        authObject.addProperty("password", password);
        String auth = username + ":" + password;
        String authEncoded = Base64.getEncoder().encodeToString(auth.getBytes());
        System.out.println("auth encoded: " + authEncoded);
        authObject.addProperty("auth", authEncoded);
        JsonObject registryObject = new JsonObject();
        registryObject.add(host, authObject);
        JsonObject configJsonObject = new JsonObject();
        configJsonObject.add("auths", registryObject);
        return configJsonObject;
    }
}

[brendandburns]

Closing as duplicate of #233

[archcosmo]

For the creation of the secret, use a byte[] and setData instead of stringData

Replace

Map<String, String> data = new HashMap<>();
data.put(".dockerconfigjson", dockerConfigJson);
secret.setStringData(data);

With

Map<String, byte[]> data = new HashMap<>();
data.put(".dockerconfigjson", dockerConfigJson.getBytes());
secret.setData(data);

This will use the ByteArrayAdapter in #240 and will avoid escaping the '='.
For the patch, the problem is that html unsafe characters are escaped from Strings in Json objects, which probably shouldn't change for security's sake.

I think to address the issue properly at its root, Kubernetes needs to do some checking for escaped '=' in Base64 on the server end.

[Bas83]

To be honest the POST is the least of my worries because there kubernetes doesn't even use the 'auth' object, it just uses the username and password which do arrive just fine if I use string data.

The main problem is with PATCH as I haven't found any way to pass this data correctly. There has to be some way to update a secret right?

If this is really something that has to be fixed on the kubernetes server side of things, I'll just file an issue with them...

[archcosmo]

This approach for patch seems to work for me

    static class Patch {
        String op;
        String path;
        Map<String ,Object> value;

        public Patch(String op, String path) {
            this.op = op;
            this.path = path;
            value = new HashMap<>();
        }

        public void setValue(String key, Object val) {
            value.put(key, val);
        }
    }

    private static void updateSecret(String username, String password) throws  Exception {
        CoreV1Api api = new CoreV1Api();
        ArrayList<Patch> patchObjects = new ArrayList<>();
        String nonEncodedConfig = getDockerConfigJson(username, password).toString();
        String encodedConfig = Base64.getEncoder().encodeToString(nonEncodedConfig.getBytes());
        System.out.println("docker config json in patch not encoded:" + nonEncodedConfig);
        System.out.println("docker config json in patch:" + encodedConfig);

        Patch patch = new Patch("replace", "/data");
        patch.setValue(".dockerconfigjson", nonEncodedConfig.getBytes());
        patchObjects.add(patch);
        System.out.println("patch objects tostring: " + patchObjects.toString());
        api.patchNamespacedSecret(SECRET_NAME, "default", patchObjects, "false");
    }

[Bas83]

That does work, thanks a lot! So do you think the problem was simply in my code/from my understanding of whether I should be encoding the patch data myself? I see that the data is now being sent to kubernetes without escaping the = signs, so would this be a security risk in your opinion?

[archcosmo]

When you give the client api a byte string to encode it will make sure to do it without escaping ‘=‘, when you give it a raw string it will need to escape unsafe characters because not doing so could potentially lead to other issues. For example if a “ finds its way into a string to be encoded and isnt escaped, it could be interpreted as part of the Json syntax and result in invalid data being sent to Kubernetes. From a security standpoint, its good to escape these characters because not doing so could open vulnerabilities, however the sanitisation should be done on the server end anyway. On 23 Apr 2018, at 09:40, Bas83 <notifications@github.com<mailto:notifications@github.com>> wrote: That does work, thanks a lot! So do you think the problem was simply in my code/from my understanding of whether I should be encoding the patch data myself? I see that the data is now being sent to kubernetes without escaping the = signs, so would this be a security risk in your opinion? — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#239 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AYen3OREfyGcU2JkX38oG9h5qArItOZ7ks5trZNigaJpZM4TZjhS>.

[Bas83]

Right... well I'd say that anything that is a valid JSON string (according to http://json.org/ which mentions only a handful of control characters) should be passed and any additional sanitizing that is needed should indeed be done server-side, but ONLY there. You can't trust a client anyway so it has to be done at least there, so why change perfectly valid JSON on the client part?

[archcosmo]

Sorry I misread your earlier question, no I dont think that escaping ‘=‘ in The Base64 is a security risk.
However disabling html escaping for all encoded Strings probably isnt a good idea. (For byte arrays escaping is turned off because this data is always encoded to base64 in this case, so it should be safe to not escape)
However, Strings in general are likely used in a lot of cases, and disabling html escaping for all Strings could have some negative effect on some other function, which is also why the html escaping is restored to its original value after byte arrays are written.

As far as the Server end, I am suggesting that base64 recieved should attempt to unescape any ‘=‘ in the case that escaped data is sent, just how you have done when setting the secret data as a string.

[Bas83]

I meant if not escaping it would be a security risk. But yeah I think this is not a client’s responsibility. Thanks a lot for your help in avoiding the whole encoding myself, that will be fine for my case :) On 23 Apr 2018, at 18:37, archcosmo <notifications@github.com<mailto:notifications@github.com>> wrote: Sorry I misread your earlier question, no I dont think that escaping ‘=‘ in The Base64 is a security risk. However disabling html escaping for all encoded Strings probably isnt a good idea. (For byte arrays escaping is turned off because this data is always encoded to base64 in this case, so it should be safe to not escape) However, Strings in general are likely used in a lot of cases, and disabling html escaping for all Strings could have some negative effect on some other function, which is also why the html escaping is restored to its original value after byte arrays are written. As far as the Server end, I am suggesting that base64 recieved should attempt to unescape any ‘=‘ in the case that escaped data is sent, just how you have done when setting the secret data as a string. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#239 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ADsoZjjJF4i873t_QfeUnt-WP5DtfPI9ks5tra7agaJpZM4TZjhS>.