Skip to content

jwt blacklist stop play/publish using jwt #4847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

jwt blacklist stop play/publish using jwt #4847

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fc28eed
issue #4809 jwt blacklist implementation and stop play/ publish using…
Feb 23, 2023
7543ea1
issue #4809 jwt blacklist implementation and stop play/ publish using
Mar 2, 2023
02841c6
Merge branch 'master' into tokenBlacklist
Jun 13, 2023
2f5971d
refactor jwt blacklist rest
Jun 13, 2023
9002d6c
refactor integration test
Jun 13, 2023
2fc0ea8
jwt blacklist for inmemory db
Jun 18, 2023
c128bc6
Merge branch 'master' into tokenBlacklist
lastpeony Jun 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/java/io/antmedia/AntMediaApplicationAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1508,6 +1508,8 @@ public static boolean updateAppSettingsFile(String appName, AppSettings newAppse
store.put(AppSettings.SETTINGS_PUBLISH_JWT_CONTROL_ENABLED, String.valueOf(newAppsettings.isPublishJwtControlEnabled()));
store.put(AppSettings.SETTINGS_PLAY_JWT_CONTROL_ENABLED, String.valueOf(newAppsettings.isPlayJwtControlEnabled()));
store.put(AppSettings.SETTINGS_JWT_STREAM_SECRET_KEY, newAppsettings.getJwtStreamSecretKey() != null ? newAppsettings.getJwtStreamSecretKey() : "");
store.put(AppSettings.SETTINGS_JWT_BLACKLIST_ENABLED, String.valueOf(newAppsettings.isJwtBlacklistEnabled()));


store.put(AppSettings.SETTINGS_WEBRTC_ENABLED, String.valueOf(newAppsettings.isWebRTCEnabled()));
store.put(AppSettings.SETTINGS_WEBRTC_FRAME_RATE, String.valueOf(newAppsettings.getWebRTCFrameRate()));
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/io/antmedia/AppSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ public class AppSettings implements Serializable{
public static final String SETTINGS_JWT_SECRET_KEY = "settings.jwtSecretKey";

public static final String SETTINGS_JWT_CONTROL_ENABLED = "settings.jwtControlEnabled";
public static final String SETTINGS_JWT_BLACKLIST_ENABLED = "settings.jwtBlacklistEnabled";

public static final String SETTINGS_IP_FILTER_ENABLED = "settings.ipFilterEnabled";

Expand Down Expand Up @@ -1283,6 +1284,8 @@ public class AppSettings implements Serializable{
@Value( "${"+SETTINGS_JWT_CONTROL_ENABLED+":false}" )
private boolean jwtControlEnabled;

@Value( "${"+SETTINGS_JWT_BLACKLIST_ENABLED+":false}" )
private boolean jwtBlacklistEnabled;
/**
* Application IP Filter Enabled
*/
Expand Down Expand Up @@ -2600,6 +2603,13 @@ public boolean isJwtControlEnabled() {
return jwtControlEnabled;
}

public void setJwtBlacklistEnabled(boolean jwtBlacklistEnabled){
this.jwtBlacklistEnabled = jwtBlacklistEnabled;
}

public boolean isJwtBlacklistEnabled() {
return jwtBlacklistEnabled;
}
public void setJwtControlEnabled(boolean jwtControlEnabled) {
this.jwtControlEnabled = jwtControlEnabled;
}
Expand Down
36 changes: 35 additions & 1 deletion src/main/java/io/antmedia/datastore/db/DataStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import java.util.List;
import java.util.Map;

import io.antmedia.rest.model.Result;
import io.antmedia.security.ITokenService;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -424,6 +426,27 @@ public List<Token> listAllTokens (Map<String, String> tokenMap, String streamId,

public abstract boolean deleteToken (String tokenId);

/**
* Delete specific token from blacklist.
* @param tokenId id of the token
*/
public abstract boolean deleteTokenFromBlacklist (String tokenId);

/**
* Get all tokens from jwt blacklist.
*/
public abstract List<String> getJwtBlacklist();

/**
* Delete all expired tokens from jwt blacklist.
*/
public abstract Result deleteAllExpiredJwtFromBlacklist(ITokenService tokenService);

/**
* Delete all tokens from jwt blacklist.
*/
public abstract void clearJwtBlacklist();

/**
* retrieve specific token
* @param tokenId id of the token
Expand Down Expand Up @@ -1352,7 +1375,18 @@ public List<WebRTCViewerInfo> getWebRTCViewerList(Map<String, String> webRTCView
* @param metaData new meta data
*/
public abstract boolean updateStreamMetaData(String streamId, String metaData);


/**
* Add jwt token to black list.
* @param token which will be added to black list.
*/
public abstract boolean addTokenToBlacklist(Token token);

/**
* Get token from black list.
* @param tokenId id of the token.
*/
public abstract Token getTokenFromBlacklist(String tokenId);

//**************************************
//ATTENTION: Write function descriptions while adding new functions
Expand Down
41 changes: 34 additions & 7 deletions src/main/java/io/antmedia/datastore/db/InMemoryDataStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@

import java.io.File;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
import java.util.Map.Entry;
import java.util.Set;
import java.util.regex.Pattern;

import io.antmedia.rest.model.Result;
import io.antmedia.security.ITokenService;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
Expand Down Expand Up @@ -900,6 +897,26 @@ public boolean deleteToken(String tokenId) {

}

@Override
public boolean deleteTokenFromBlacklist(String tokenId) {
return false;
}

@Override
public List<String> getJwtBlacklist() {
return Collections.emptyList();
}

@Override
public Result deleteAllExpiredJwtFromBlacklist(ITokenService tokenService) {
return null;
}

@Override
public void clearJwtBlacklist() {
throw new UnsupportedOperationException("JWT blacklist must be stored as map based db on disk, not in memory.");
}

@Override
public Token getToken(String tokenId) {

Expand Down Expand Up @@ -1022,4 +1039,14 @@ public boolean updateStreamMetaData(String streamId, String metaData) {
}
return result;
}

@Override
public boolean addTokenToBlacklist(Token token) {
return false;
}

@Override
public Token getTokenFromBlacklist(String tokenId) {
return null;
}
}
90 changes: 90 additions & 0 deletions src/main/java/io/antmedia/datastore/db/MapBasedDataStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Pattern;

import io.antmedia.rest.model.Result;
import io.antmedia.security.ITokenService;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.exception.ExceptionUtils;
Expand Down Expand Up @@ -41,6 +44,8 @@ public abstract class MapBasedDataStore extends DataStore {
protected Map<String, String> vodMap;
protected Map<String, String> detectionMap;
protected Map<String, String> tokenMap;
protected Map<String, String> tokenBlacklistMap;

protected Map<String, String> subscriberMap;
protected Map<String, String> conferenceRoomMap;
protected Map<String, String> webRTCViewerMap;
Expand Down Expand Up @@ -943,6 +948,65 @@ public boolean deleteToken(String tokenId) {
return result;
}

@Override
public boolean deleteTokenFromBlacklist(String tokenId) {
boolean result;

synchronized (this) {
result = tokenBlacklistMap.remove(tokenId) != null;
}
return result;
}

@Override
public List<String> getJwtBlacklist(){

synchronized (this){
return new ArrayList<>(tokenBlacklistMap.keySet());

}

}

@Override
public Result deleteAllExpiredJwtFromBlacklist(ITokenService tokenService){
logger.info("Deleting all expired JWTs from black list.");
AtomicInteger deletedTokenCount = new AtomicInteger();

synchronized (this){
tokenBlacklistMap.forEach((key, value) -> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6% of developers fix this issue

THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method MapBasedDataStore.deleteAllExpiredJwtFromBlacklist(...) reads without synchronization from container this.tokenBlacklistMap via call to Map.forEach(...). Potentially races with write in method MapBasedDataStore.deleteTokenFromBlacklist(...).
Reporting because another access to the same memory occurs on a background thread, although this access may not.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Token token = gson.fromJson(value,Token.class);
String tokenId = token.getTokenId();
if(!tokenService.verifyJwt(tokenId,token.getStreamId(),token.getType())){
if(deleteTokenFromBlacklist(tokenId)){
deletedTokenCount.getAndIncrement();
}else{
logger.warn("Couldn't delete JWT:{}", tokenId);
}
}
});
}

if(deletedTokenCount.get() > 0){
final String successMsg = deletedTokenCount+" JWT deleted successfully from black list.";
logger.info(successMsg);
return new Result(true, successMsg);
}else{
final String failMsg = "No JWT deleted from black list.";
logger.warn(failMsg);
return new Result(false, failMsg);

}

}

@Override
public void clearJwtBlacklist(){
synchronized (this) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this as a synchronization point might not be a good choice as other classes can still hold a lock on this class. I would suggest using a non-public internal locking mechanism for synchronization purposes, which is more fine-grained and only applies to operations on the tokenMap, e.g. a ReadWriteLock or similar.

tokenBlacklistMap.clear();
}
}

@Override
public Token getToken(String tokenId) {
return super.getToken(tokenMap, tokenId, gson);
Expand Down Expand Up @@ -1055,4 +1119,30 @@ public Broadcast getBroadcastFromMap(String streamId)
return null;
}

@Override
public boolean addTokenToBlacklist(Token token) {
boolean result = false;

synchronized (this) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd revisit the usage of synchronized in this class, as there seems to be some overuse and overscoping. Using this as a monitor is not very fortunate, as it can be held externally. Here the monitor is obtained too early, only saveToken seems to be critical, but as far as I can see saveToken itself also takes care of synchronization using this, so this might be completely unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree


if (token.getStreamId() != null && token.getTokenId() != null) {

try {
tokenBlacklistMap.put(token.getTokenId(), gson.toJson(token));
result = true;
} catch (Exception e) {
logger.error(ExceptionUtils.getStackTrace(e));
}
}
}
return result;

}

@Override
public Token getTokenFromBlacklist(String tokenId) {
return super.getToken(tokenBlacklistMap, tokenId, gson);

}

}
16 changes: 10 additions & 6 deletions src/main/java/io/antmedia/datastore/db/MapDBStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
import java.io.IOException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.Map.Entry;

import org.apache.commons.lang3.exception.ExceptionUtils;
import org.mapdb.DB;
Expand All @@ -16,9 +13,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.antmedia.datastore.db.types.Broadcast;
import io.antmedia.datastore.db.types.StreamInfo;
import io.antmedia.muxer.IAntMediaStreamHandler;
import io.vertx.core.Vertx;


Expand All @@ -33,6 +28,8 @@ public class MapDBStore extends MapBasedDataStore {
private static final String VOD_MAP_NAME = "VOD";
private static final String DETECTION_MAP_NAME = "DETECTION";
private static final String TOKEN = "TOKEN";
private static final String TOKEN_BLACKLIST = "TOKEN_BLACKLIST";

private static final String SUBSCRIBER = "SUBSCRIBER";
private static final String CONFERENCE_ROOM_MAP_NAME = "CONFERENCE_ROOM";
private static final String WEBRTC_VIEWER = "WEBRTC_VIEWER";
Expand Down Expand Up @@ -73,6 +70,10 @@ public MapDBStore(String dbName, Vertx vertx) {
webRTCViewerMap = db.treeMap(WEBRTC_VIEWER).keySerializer(Serializer.STRING).valueSerializer(Serializer.STRING)
.counterEnable().createOrOpen();

tokenBlacklistMap = db.treeMap(TOKEN_BLACKLIST).keySerializer(Serializer.STRING).valueSerializer(Serializer.STRING)
.counterEnable().createOrOpen();


timerId = vertx.setPeriodic(5000, id ->

vertx.executeBlocking(b -> {
Expand Down Expand Up @@ -124,7 +125,10 @@ public void close(boolean deleteDB) {
public void clearStreamInfoList(String streamId) {
//used in mongo for cluster mode. useless here.
}





@Override
public List<StreamInfo> getStreamInfoList(String streamId) {
return new ArrayList<>();
Expand Down
Loading