Skip to content

Commit

Permalink
[FIXED JENKINS-45185] Align all base64 handling on commons-codec to c…
Browse files Browse the repository at this point in the history 
…orrectly handle chunked and url-safe variants
  • Loading branch information
@stephenc
stephenc committed Jul 20, 2017
1 parent 7d0b95e commit 8d4352e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 14 deletions.
12 changes: 5 additions & 7 deletions src/main/java/com/cloudbees/plugins/credentials/SecretBytes.java
View file
Expand Up @@ -38,17 +38,15 @@
import hudson.util.Secret;
import java.io.Serializable;
import java.security.GeneralSecurityException;
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import javax.crypto.Cipher;
import jcifs.util.Base64;
import jenkins.security.ConfidentialStore;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang.IllegalClassException;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.Stapler;

/**
* An analogue of {@link Secret} to be used for efficient storage of {@link byte[]}. The serialized form will embed the
Expand Down Expand Up @@ -276,13 +274,13 @@ public static SecretBytes fromString(String data) {
try {
int len = data.length();
if (len >= 2 && ENCRYPTED_VALUE_PATTERN.matcher(data).matches()) {
byte[] decoded = Base64.decode(data.substring(1, len - 1));
byte[] decoded = Base64.decodeBase64(data.substring(1, len - 1));
s = decrypt(decoded);
if (s != null) {
return s;
}
}
s = new SecretBytes(false, Base64.decode(data));
s = new SecretBytes(false, Base64.decodeBase64(data));
} catch (StringIndexOutOfBoundsException e) {
// wasn't valid Base64
s = new SecretBytes(false, new byte[0]);
Expand All @@ -301,7 +299,7 @@ public static boolean isSecretBytes(String data) {
if (len >= 2 && ENCRYPTED_VALUE_PATTERN.matcher(data).matches()) {
byte[] decoded;
try {
decoded = Base64.decode(data.substring(1, len - 1));
decoded = Base64.decodeBase64(data.substring(1, len - 1));
} catch (StringIndexOutOfBoundsException e) {
// invalid Base64
return false;
Expand All @@ -316,7 +314,7 @@ public static boolean isSecretBytes(String data) {
*/
@Override
public String toString() {
return "{" + Base64.encode(getEncryptedData()) + "}";
return "{" + Base64.encodeBase64String(getEncryptedData()) + "}";
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/cloudbees/plugins/credentials/impl/CertificateCredentialsImpl.java
View file
Expand Up @@ -35,7 +35,6 @@
import hudson.Util;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import hudson.remoting.Base64;
import hudson.remoting.Channel;
import hudson.util.FormValidation;
import hudson.util.HttpResponses;
Expand All @@ -61,6 +60,7 @@
import java.util.logging.Logger;
import javax.servlet.ServletException;
import net.jcip.annotations.GuardedBy;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.fileupload.FileItem;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
Expand Down Expand Up @@ -602,7 +602,7 @@ public static class DescriptorImpl extends KeyStoreSourceDescriptor {
@NonNull
public static byte[] toByteArray(@Nullable Secret secret) {
if (secret != null) {
byte[] decoded = Base64.decode(secret.getPlainText());
byte[] decoded = Base64.decodeBase64(secret.getPlainText());
if (null != decoded) {
return decoded;
}
Expand All @@ -623,7 +623,7 @@ public static byte[] toByteArray(@Nullable Secret secret) {
public static Secret toSecret(@Nullable byte[] contents) {
return contents == null || contents.length == 0
? null
: Secret.fromString(Base64.encode(contents));
: Secret.fromString(Base64.encodeBase64String(contents));
}

/**
Expand Down
37 changes: 33 additions & 4 deletions src/test/java/com/cloudbees/plugins/credentials/SecretBytesTest.java
View file
Expand Up @@ -23,10 +23,11 @@
*/
package com.cloudbees.plugins.credentials;

import hudson.remoting.Base64;
import java.nio.charset.StandardCharsets;
import java.util.Random;
import jenkins.model.Jenkins;
import jenkins.security.ConfidentialStoreRule;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang.RandomStringUtils;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -55,7 +56,7 @@ public void encrypt() throws Exception {
public void encryptedValuePattern() {
Random entropy = new Random();
for (int i = 1; i < 100; i++) {
String plaintext = Base64.encode(RandomStringUtils.random(entropy.nextInt(i)).getBytes());
String plaintext = Base64.encodeBase64String(RandomStringUtils.random(entropy.nextInt(i)).getBytes());
SecretBytes secretBytes = SecretBytes.fromString(plaintext);
String ciphertext = secretBytes.toString();
// System.out.printf("%s%n → %s%n → %s%n", plaintext, ciphertext, secretBytes);
Expand Down Expand Up @@ -105,7 +106,7 @@ public void noAccidentalDecrypt() throws Exception {
public void serialization() throws Exception {
SecretBytes s = SecretBytes.fromBytes("Mr.Jenkins".getBytes());
String xml = Jenkins.XSTREAM.toXML(s);
assertThat(xml, not(containsString(Base64.encode("Mr.Jenkins".getBytes()))));
assertThat(xml, not(containsString(Base64.encodeBase64String("Mr.Jenkins".getBytes()))));
Object o = Jenkins.XSTREAM.fromXML(xml);
assertThat(o, is((Object)s));
}
Expand All @@ -120,13 +121,41 @@ public static class Foo {
@Test
public void testCompatibilityFromString() {
String tagName = Foo.class.getName().replace("$", "_-");
String xml = String.format("<%s><password>%s</password></%s>", tagName, Base64.encode("secret".getBytes()), tagName);
String xml = String.format("<%s><password>%s</password></%s>", tagName, Base64.encodeBase64String("secret".getBytes()), tagName);
Foo foo = new Foo();
Jenkins.XSTREAM.fromXML(xml, foo);
assertThat(SecretBytes.getPlainData(foo.password), is("secret".getBytes()));
//System.out.println(Jenkins.XSTREAM.toXML(foo));
//System.out.println(Jenkins.XSTREAM.toXML(foo));
}

@Test
public void largeRawString__noChunking__noUrlSafe() throws Exception {
byte[] data = new byte[2048];
new Random().nextBytes(data);
assertThat(SecretBytes.fromString(new String(org.apache.commons.codec.binary.Base64.encodeBase64(data, false, false), StandardCharsets.US_ASCII)).getPlainData(), is(data));
}

@Test
public void largeRawString__chunking__noUrlSafe() throws Exception {
byte[] data = new byte[2048];
new Random().nextBytes(data);
assertThat(SecretBytes.fromString(new String(org.apache.commons.codec.binary.Base64.encodeBase64(data, true, false), StandardCharsets.US_ASCII)).getPlainData(), is(data));
}

@Test
public void largeRawString__noChunking__urlSafe() throws Exception {
byte[] data = new byte[2048];
new Random().nextBytes(data);
assertThat(SecretBytes.fromString(new String(org.apache.commons.codec.binary.Base64.encodeBase64(data, false, true), StandardCharsets.US_ASCII)).getPlainData(), is(data));
}

@Test
public void largeRawString__chunking__urlSafe() throws Exception {
byte[] data = new byte[2048];
new Random().nextBytes(data);
assertThat(SecretBytes.fromString(new String(org.apache.commons.codec.binary.Base64.encodeBase64(data, true, true), StandardCharsets.US_ASCII)).getPlainData(), is(data));
}


}

0 comments on commit 8d4352e

Please sign in to comment.