Skip to content

Commit

Permalink
Fix guessable work item ID weakness (MID-5291)
Browse files Browse the repository at this point in the history
In addition to the work item number we expect and check SHA256 hash
of some parts of the work item. The attacker does not know them,
so he is unable to create/guess the respective URL.
  • Loading branch information
mederly committed Apr 18, 2019
1 parent 39b305b commit 4fa1771
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 10 deletions.
Expand Up @@ -28,6 +28,8 @@
import com.evolveum.midpoint.web.component.data.column.IconColumn;
import com.evolveum.midpoint.web.component.data.column.LinkColumn;
import com.evolveum.midpoint.web.page.admin.workflow.PageWorkItem;
import com.evolveum.midpoint.web.page.admin.workflow.PageWorkItems;
import com.evolveum.midpoint.web.page.admin.workflow.dto.ProtectedWorkItemId;
import com.evolveum.midpoint.web.page.admin.workflow.dto.WorkItemDtoProvider;
import com.evolveum.midpoint.web.page.admin.workflow.dto.WorkItemDto;
import com.evolveum.midpoint.web.session.UserProfileStorage;
Expand Down Expand Up @@ -163,7 +165,8 @@ private AbstractColumn<WorkItemDto, String> createNameColumn() {
@Override
public void onClick(AjaxRequestTarget target, IModel<WorkItemDto> rowModel) {
PageParameters parameters = new PageParameters();
parameters.add(OnePageParameterEncoder.PARAMETER, rowModel.getObject().getWorkItemId());
parameters.add(OnePageParameterEncoder.PARAMETER,
ProtectedWorkItemId.createExternalForm(rowModel.getObject().getWorkItem()));
getPageBase().navigateToNext(PageWorkItem.class, parameters);
}
};
Expand Down
Expand Up @@ -43,6 +43,7 @@
import com.evolveum.midpoint.web.component.DefaultAjaxSubmitButton;
import com.evolveum.midpoint.web.component.util.VisibleBehaviour;
import com.evolveum.midpoint.web.page.admin.workflow.dto.WorkItemDto;
import com.evolveum.midpoint.web.page.admin.workflow.dto.ProtectedWorkItemId;
import com.evolveum.midpoint.web.util.OnePageParameterEncoder;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
import org.apache.wicket.RestartResponseException;
Expand Down Expand Up @@ -99,12 +100,12 @@ public class PageWorkItem extends PageAdminWorkItems {
private static final String ID_CANCEL = "cancel";

private LoadableModel<WorkItemDto> workItemDtoModel;
private String taskId;
private String externalizedProtectedId;

public PageWorkItem(PageParameters parameters) {

taskId = parameters.get(OnePageParameterEncoder.PARAMETER).toString();
if (taskId == null) {
externalizedProtectedId = parameters.get(OnePageParameterEncoder.PARAMETER).toString();
if (externalizedProtectedId == null) {
throw new IllegalStateException("Work item ID not specified.");
}

Expand All @@ -131,25 +132,27 @@ private WorkItemDto loadWorkItemDtoIfNecessary() {
OperationResult result = task.getResult();
WorkItemDto workItemDto = null;
try {
final ObjectQuery query = QueryBuilder.queryFor(WorkItemType.class, getPrismContext())
.item(F_EXTERNAL_ID).eq(taskId)
ProtectedWorkItemId protectedWorkItemId = ProtectedWorkItemId.fromExternalForm(externalizedProtectedId);
final ObjectQuery query = QueryBuilder.queryFor(WorkItemType.class, getPrismContext())
.item(F_EXTERNAL_ID).eq(protectedWorkItemId.id)
.build();
final Collection<SelectorOptions<GetOperationOptions>> options =
resolveItemsNamed(F_ASSIGNEE_REF, F_ORIGINAL_ASSIGNEE_REF);
List<WorkItemType> workItems = getModelService().searchContainers(WorkItemType.class, query, options, task, result);
if (workItems.size() > 1) {
throw new SystemException("More than one work item with ID of " + taskId);
throw new SystemException("More than one work item with ID of " + protectedWorkItemId.id);
} else if (workItems.size() == 0) {
throw new ObjectNotFoundException("No work item with ID of " + taskId);
throw new ObjectNotFoundException("No work item with ID of " + protectedWorkItemId.id);
}
final WorkItemType workItem = workItems.get(0);

final String taskOid = WfContextUtil.getTaskOid(workItem);
String taskOid = WfContextUtil.getTaskOid(workItem);
if (taskOid == null) {
// this is a problem ... most probably we will not be able to do anything reasonable - let's give it up
result.recordFatalError(getString("PageWorkItem.noRequest"));
showResult(result, false);
throw redirectBackViaRestartResponseException();
} else if (!protectedWorkItemId.isCorrect(workItem)) {
throw new IllegalArgumentException("Wrong work item hash");
}
TaskType taskType = null;
List<TaskType> relatedTasks = new ArrayList<>();
Expand Down
@@ -0,0 +1,73 @@
/*
* Copyright (c) 2010-2019 Evolveum
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.evolveum.midpoint.web.page.admin.workflow.dto;

import com.evolveum.midpoint.prism.xml.XmlTypeConverter;
import com.evolveum.midpoint.schema.util.WfContextUtil;
import com.evolveum.midpoint.util.MiscUtil;
import com.evolveum.midpoint.util.exception.SystemException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.WorkItemType;
import org.jetbrains.annotations.NotNull;

import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

/**
* Protected version of work item ID in the form of "id:hash".
*
* It is to be used as part of URL used to display the work item. The hash is computed from selected parts of
* the work item so it is impossible to guess.
*/
public class ProtectedWorkItemId {

@NotNull public final String id;
@NotNull public final String hash;

private ProtectedWorkItemId(@NotNull String id, @NotNull String hash) {
this.id = id;
this.hash = hash;
}

public static ProtectedWorkItemId fromExternalForm(@NotNull String externalForm) {
int i = externalForm.indexOf(':');
if (i < 0) {
throw new IllegalArgumentException("Wrong work item ID format");
}
return new ProtectedWorkItemId(externalForm.substring(0, i), externalForm.substring(i+1));
}

private static String createWorkItemHash(WorkItemType workItem) {
try {
String valueToHash = workItem.getExternalId() + ":" +
WfContextUtil.getTaskOid(workItem) + ":" +
XmlTypeConverter.toMillis(workItem.getCreateTimestamp());
byte[] hashBytes = MessageDigest.getInstance("SHA-256").digest(valueToHash.getBytes(StandardCharsets.UTF_8));
return MiscUtil.binaryToHex(hashBytes);
} catch (NoSuchAlgorithmException e) {
throw new SystemException("Couldn't compute message digest: " + e.getMessage(), e);
}
}

public static String createExternalForm(WorkItemType workItem) {
return workItem.getExternalId() + ":" + createWorkItemHash(workItem);
}

public boolean isCorrect(WorkItemType workItem) {
return hash.equals(createWorkItemHash(workItem));
}
}

0 comments on commit 4fa1771

Please sign in to comment.