Skip to content

Commit

Permalink
[SECURITY-1878]
Browse files Browse the repository at this point in the history
  • Loading branch information
rsandell committed Dec 10, 2021
1 parent 7078a70 commit c069b79
Show file tree
Hide file tree
Showing 4 changed files with 301 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.domains.DomainRequirement;
import com.cloudbees.plugins.credentials.domains.HostnameRequirement;

import hudson.AbortException;
import hudson.EnvVars;
import hudson.Extension;
import hudson.FilePath;
import hudson.Launcher;
import hudson.Util;
import hudson.model.AbstractBuild;
import hudson.model.AbstractDescribableImpl;
Expand All @@ -41,17 +43,18 @@
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.remoting.VirtualChannel;
import hudson.slaves.WorkspaceList;
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
import jenkins.authentication.tokens.api.AuthenticationTokens;
import jenkins.model.Jenkins;

import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
Expand All @@ -62,12 +65,10 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static com.cloudbees.plugins.credentials.CredentialsMatchers.*;
import hudson.AbortException;
import hudson.EnvVars;
import hudson.Launcher;
import hudson.slaves.WorkspaceList;
import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
import static com.cloudbees.plugins.credentials.CredentialsMatchers.allOf;
import static com.cloudbees.plugins.credentials.CredentialsMatchers.firstOrNull;
import static com.cloudbees.plugins.credentials.CredentialsMatchers.withId;
import static org.jenkinsci.plugins.docker.commons.credentials.ImageNameValidator.validateUserAndRepo;

/**
* Encapsulates the endpoint of DockerHub and how to interact with it.
Expand Down Expand Up @@ -312,6 +313,12 @@ public String imageName(@Nonnull String userAndRepo) throws IOException {
if (userAndRepo == null) {
throw new IllegalArgumentException("Image name cannot be null.");
}

final FormValidation validation = validateUserAndRepo(userAndRepo);
if (validation.kind != FormValidation.Kind.OK) {
throw validation;
}

if (url == null) {
return userAndRepo;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
/*
* The MIT License
*
* Copyright (c) 2021, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.plugins.docker.commons.credentials;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.util.FormValidation;
import org.apache.commons.lang.StringUtils;

import javax.annotation.CheckForNull;
import java.util.Arrays;
import java.util.regex.Pattern;

public class ImageNameValidator {

private static /*almost final*/ boolean SKIP = Boolean.getBoolean(ImageNameValidator.class.getName() + ".SKIP");

/**
* If the validation is set to be skipped.
*
* I.e. the system property <code>org.jenkinsci.plugins.docker.commons.credentials.ImageNameValidator.SKIP</code>
* is set to <code>true</code>.
* When this is se to true {@link #validateName(String)}, {@link #validateTag(String)} and {@link #validateUserAndRepo(String)}
* returns {@link FormValidation#ok()} immediately without performing the validation.
*
* @return true if validation is skipped.
*/
public static boolean skipped() {
return SKIP;
}

/**
* Splits a repository id namespace/name into it's three components (repo/namespace[/*],name,tag)
*
* @param userAndRepo the repository ID namespace/name (ie. "jenkinsci/workflow-demo:latest").
* The namespace can have more than one path element.
* @return an array where position 0 is the namespace, 1 is the name and 2 is the tag.
* Any position could be <code>null</code>
*/
public static @NonNull String[] splitUserAndRepo(@NonNull String userAndRepo) {
String[] args = new String[3];
if (StringUtils.isEmpty(userAndRepo)) {
return args;
}
int slashIdx = userAndRepo.lastIndexOf('/');
int tagIdx = userAndRepo.lastIndexOf(':');
if (tagIdx == -1 && slashIdx == -1) {
args[1] = userAndRepo;
} else if (tagIdx < slashIdx) {
//something:port/something or something/something
args[0] = userAndRepo.substring(0, slashIdx);
args[1] = userAndRepo.substring(slashIdx + 1);
} else {
if (slashIdx != -1) {
args[0] = userAndRepo.substring(0, slashIdx);
args[1] = userAndRepo.substring(slashIdx + 1);
}
if (tagIdx > 0) {
int start = slashIdx > 0 ? slashIdx + 1 : 0;
args[1] = userAndRepo.substring(start, tagIdx);
if (tagIdx < userAndRepo.length() - 1) {
args[2] = userAndRepo.substring(tagIdx + 1);
}
}
}
return args;
}

/**
* Validates the string as <code>[registry/repo/]name[:tag]</code>
* @param userAndRepo the image id
* @return if it is valid or not, or OK if set to {@link #SKIP}.
*
* @see #VALID_NAME_COMPONENT
* @see #VALID_TAG
*/
public static @NonNull FormValidation validateUserAndRepo(@NonNull String userAndRepo) {
if (SKIP) {
return FormValidation.ok();
}
final String[] args = splitUserAndRepo(userAndRepo);
if (StringUtils.isBlank(args[0]) && StringUtils.isBlank(args[1]) && StringUtils.isBlank(args[2])) {
return FormValidation.error("Bad imageName format: %s", userAndRepo);
}
final FormValidation name = validateName(args[1]);
final FormValidation tag = validateTag(args[2]);
if (name.kind == FormValidation.Kind.OK && tag.kind == FormValidation.Kind.OK) {
return FormValidation.ok();
}
if (name.kind == FormValidation.Kind.OK) {
return tag;
}
if (tag.kind == FormValidation.Kind.OK) {
return name;
}
return FormValidation.aggregate(Arrays.asList(name, tag));
}

/**
* Calls {@link #validateUserAndRepo(String)} and if the result is not OK throws it as an exception.
*
* @param userAndRepo the image id
* @throws FormValidation if not OK
*/
public static void checkUserAndRepo(@NonNull String userAndRepo) throws FormValidation {
final FormValidation validation = validateUserAndRepo(userAndRepo);
if (validation.kind != FormValidation.Kind.OK) {
throw validation;
}
}

/**
* A tag name must be valid ASCII and may contain
* lowercase and uppercase letters, digits, underscores, periods and dashes.
* A tag name may not start with a period or a dash and may contain a maximum of 128 characters.
*
* @see <a href="https://docs.docker.com/engine/reference/commandline/tag/">docker tag</a>
*/
public static final Pattern VALID_TAG = Pattern.compile("^[a-zA-Z0-9_]([a-zA-Z0-9_.-]){0,127}");

This comment has been minimized.

Copy link
@jsnod

jsnod Jan 14, 2022

@rsandell This new pattern appears to be rejecting valid tags: my-service:PR-513-7

This comment has been minimized.

Copy link
@j3t

j3t Jan 17, 2022

Member

and also digests: my-service@sha256:040e7d6bfb56514b55fba4889adf92e8c621285cbd226af0e62f3858f6a01a96

This comment has been minimized.

Copy link
@j3t

j3t Jan 17, 2022

Member

see #93



/**
* Validates a tag is following the rules.
*
* If the tag is null or the empty string it is considered valid.
*
* @param tag the tag to validate.
* @return the validation result
* @see #VALID_TAG
*/
public static @NonNull FormValidation validateTag(@CheckForNull String tag) {
if (SKIP) {
return FormValidation.ok();
}
if (StringUtils.isEmpty(tag)) {
return FormValidation.ok();
}
if (tag.length() > 128) {
return FormValidation.error("Tag length > 128");
}
if (VALID_TAG.matcher(tag).matches()) {
return FormValidation.ok();
} else {
return FormValidation.error("Tag must follow the pattern '%s'", VALID_TAG.pattern());
}
}

/**
* Calls {@link #validateTag(String)} and if not OK throws the exception.
*
* @param tag the tag
* @throws FormValidation if not OK
*/
public static void checkTag(@CheckForNull String tag) throws FormValidation {
final FormValidation validation = validateTag(tag);
if (validation.kind != FormValidation.Kind.OK) {
throw validation;
}
}

/**
* Name components may contain lowercase letters, digits and separators.
* A separator is defined as a period, one or two underscores, or one or more dashes.
* A name component may not start or end with a separator.
*
* @see <a href="https://docs.docker.com/engine/reference/commandline/tag/">docker tag</a>
*/
public static final Pattern VALID_NAME_COMPONENT = Pattern.compile("^[a-zA-Z0-9]+((\\.|_|__|-+)[a-zA-Z0-9]+)*$");

/**
* Validates a docker image name that it is following the rules as a single name component.
*
* If the name is null or the empty string it is not considered valid.
*
* @param name the name
* @return the validation result
* @see #VALID_NAME_COMPONENT
*/
public static @NonNull FormValidation validateName(@CheckForNull String name) {
if (SKIP) {
return FormValidation.ok();
}
if (StringUtils.isEmpty(name)) {
return FormValidation.error("Missing name.");
}
if (VALID_NAME_COMPONENT.matcher(name).matches()) {
return FormValidation.ok();
} else {
return FormValidation.error("Name must follow the pattern '%s'", VALID_NAME_COMPONENT.pattern());
}
}

/**
* Calls {@link #validateName(String)} and if not OK throws the exception.
*
* @param name the name
* @throws FormValidation if not OK
*/
public static void checkName(String name) throws FormValidation {
final FormValidation validation = validateName(name);
if (validation.kind != FormValidation.Kind.OK) {
throw validation;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ public void testParseWithTags() throws Exception {
public void testParseFullyQualifiedImageName() throws Exception {
assertEquals("private-repo:5000/test-image", new DockerRegistryEndpoint("http://private-repo:5000/", null).imageName("private-repo:5000/test-image"));
assertEquals("private-repo:5000/test-image", new DockerRegistryEndpoint("http://private-repo:5000/", null).imageName("test-image"));
assertEquals("private-repo:5000/test-image:dev", new DockerRegistryEndpoint("http://private-repo:5000/", null).imageName("private-repo:5000/test-image:dev"));
assertEquals("private-repo:5000/test-image:dev", new DockerRegistryEndpoint("http://private-repo:5000/", null).imageName("test-image:dev"));
}

@Issue("JENKINS-39181")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package org.jenkinsci.plugins.docker.commons.credentials;

import hudson.util.FormValidation;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import static org.junit.Assert.*;

/**
* Tests various inputs to {@link ImageNameValidator#validateUserAndRepo(String)}.
*/
@RunWith(Parameterized.class)
public class ImageNameValidatorTest {

@Parameterized.Parameters(name = "{index}:{0}") public static Object[][] data(){
return new Object[][] {
{"jenkinsci/workflow-demo", FormValidation.Kind.OK},
{"docker:80/jenkinsci/workflow-demo", FormValidation.Kind.OK},
{"jenkinsci/workflow-demo:latest", FormValidation.Kind.OK},
{"docker:80/jenkinsci/workflow-demo:latest", FormValidation.Kind.OK},
{"workflow-demo:latest", FormValidation.Kind.OK},
{"workflow-demo", FormValidation.Kind.OK},
{":tag", FormValidation.Kind.ERROR},
{"name:tag", FormValidation.Kind.OK},
{"name:.tag", FormValidation.Kind.ERROR},
{"name:-tag", FormValidation.Kind.ERROR},
{"name:.tag.", FormValidation.Kind.ERROR},
{"name:tag.", FormValidation.Kind.OK},
{"name:tag-", FormValidation.Kind.OK},
{"_name:tag", FormValidation.Kind.ERROR},
{"na___me:tag", FormValidation.Kind.ERROR},
{"na__me:tag", FormValidation.Kind.OK},
{"name:tag\necho hello", FormValidation.Kind.ERROR},
{"name\necho hello:tag", FormValidation.Kind.ERROR},
{"name:tag$BUILD_NUMBER", FormValidation.Kind.ERROR},
{"name$BUILD_NUMBER:tag", FormValidation.Kind.ERROR},
{null, FormValidation.Kind.ERROR},
{"", FormValidation.Kind.ERROR},
{":", FormValidation.Kind.ERROR},
{" ", FormValidation.Kind.ERROR},

};
}

private final String userAndRepo;
private final FormValidation.Kind expected;

public ImageNameValidatorTest(final String userAndRepo, final FormValidation.Kind expected) {
this.userAndRepo = userAndRepo;
this.expected = expected;
}

@Test
public void test() {
assertSame(expected, ImageNameValidator.validateUserAndRepo(userAndRepo).kind);
}
}

0 comments on commit c069b79

Please sign in to comment.