melloware
commented
1 year ago @ketola this is exactly the feedback I am looking for! I really like all your ideas let me work on something today and I will have you review it!
Closed ketola closed 1 year ago
melloware
commented
1 year ago @ketola this is exactly the feedback I am looking for! I really like all your ideas let me work on something today and I will have you review it!
melloware
commented
1 year ago @ketola here is what I came up with.
Remove ALL exception throwing.
Created a ScanResult object.
public class AntivirusScanResult {
/**
* Status code mirrors HTTP status codes for signaling what occurred so the downstream system can act. 200 = OK.
*/
private int status;
/**
* Scan engine that produced this result.
*/
private String engine;
/**
* Name of the file that was scanned.
*/
private String fileName;
/**
* Human-readable message explaining what happened.
*/
private String message;
/**
* Payload from the engine, for example if its an HTTP service it might be the JSON response raw.
*/
private String payload;
}// scan the file and check the results
List<AntivirusScanResult> results = antivirus.scan(fileName, inputStream);
for (AntivirusScanResult result : results) {
if (result.getStatus() != Response.Status.OK.getStatusCode()) {
throw new WebApplicationException(result.getMessage(), result.getStatus());
}
}@all-contributors add @ketola for test,ideas
allcontributors[bot]
commented
1 year ago @melloware
I've put up a pull request to add @ketola! :tada:
melloware
commented
1 year ago 0.0.4 is now in Maven Central if you want to test and submit any PR's or feedback!
melloware
commented
1 year ago Here is what the output looks like if you just run a scan with both engines.
[
{
"status": 200,
"engine": "ClamAV",
"fileName": "valid.txt",
"message": "stream: OK",
"payload": "stream: OK"
},
{
"status": 200,
"engine": "VirusTotal",
"fileName": "valid.txt",
"message": "OK",
"payload": "{ 'data': { 'attributes': { 'type_description': 'JavaScript', 'vhash': '9eecb7db59d16c80417c72d1e1f4fbf1', 'type_tags': [ 'source', 'javascript', 'js' ], 'names': [ 'valid.txt' ], 'last_modification_date': 1697837987, 'type_tag': 'javascript', 'times_submitted': 1, 'total_votes': { 'harmless': 0, 'malicious': 0 }, 'size': 17, 'type_extension': 'js', 'last_submission_date': 1697830761, 'last_analysis_results': { 'Bkav': { 'category': 'undetected', 'engine_name': 'Bkav', 'engine_version': '2.0.0.1', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Lionic': { 'category': 'undetected', 'engine_name': 'Lionic', 'engine_version': '7.5', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Elastic': { 'category': 'type-unsupported', 'engine_name': 'Elastic', 'engine_version': '4.0.112', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'ClamAV': { 'category': 'undetected', 'engine_name': 'ClamAV', 'engine_version': '1.2.0.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'CMC': { 'category': 'undetected', 'engine_name': 'CMC', 'engine_version': '2.4.2022.1', 'result': null, 'method': 'blacklist', 'engine_update': '20230822' }, 'CAT-QuickHeal': { 'category': 'undetected', 'engine_name': 'CAT-QuickHeal', 'engine_version': '22.00', 'result': null, 'method': 'blacklist', 'engine_update': '20231019' }, 'Skyhigh': { 'category': 'undetected', 'engine_name': 'Skyhigh', 'engine_version': 'v2021.2.0+4045', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'ALYac': { 'category': 'undetected', 'engine_name': 'ALYac', 'engine_version': '1.1.3.1', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Malwarebytes': { 'category': 'undetected', 'engine_name': 'Malwarebytes', 'engine_version': '4.5.5.54', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'VIPRE': { 'category': 'undetected', 'engine_name': 'VIPRE', 'engine_version': '6.0.0.35', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Sangfor': { 'category': 'undetected', 'engine_name': 'Sangfor', 'engine_version': '2.23.0.0', 'result': null, 'method': 'blacklist', 'engine_update': '20230926' }, 'K7AntiVirus': { 'category': 'undetected', 'engine_name': 'K7AntiVirus', 'engine_version': '12.120.49950', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Alibaba': { 'category': 'type-unsupported', 'engine_name': 'Alibaba', 'engine_version': '0.3.0.5', 'result': null, 'method': 'blacklist', 'engine_update': '20190527' }, 'K7GW': { 'category': 'undetected', 'engine_name': 'K7GW', 'engine_version': '12.120.49949', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Trustlook': { 'category': 'type-unsupported', 'engine_name': 'Trustlook', 'engine_version': '1.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Baidu': { 'category': 'undetected', 'engine_name': 'Baidu', 'engine_version': '1.0.0.2', 'result': null, 'method': 'blacklist', 'engine_update': '20190318' }, 'VirIT': { 'category': 'undetected', 'engine_name': 'VirIT', 'engine_version': '9.5.562', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'SymantecMobileInsight': { 'category': 'type-unsupported', 'engine_name': 'SymantecMobileInsight', 'engine_version': '2.0', 'result': null, 'method': 'blacklist', 'engine_update': '20230119' }, 'Symantec': { 'category': 'undetected', 'engine_name': 'Symantec', 'engine_version': '1.21.0.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'tehtris': { 'category': 'type-unsupported', 'engine_name': 'tehtris', 'engine_version': null, 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'ESET-NOD32': { 'category': 'undetected', 'engine_name': 'ESET-NOD32', 'engine_version': '28105', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'APEX': { 'category': 'type-unsupported', 'engine_name': 'APEX', 'engine_version': '6.466', 'result': null, 'method': 'blacklist', 'engine_update': '20231019' }, 'TrendMicro-HouseCall': { 'category': 'undetected', 'engine_name': 'TrendMicro-HouseCall', 'engine_version': '10.0.0.1040', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Avast': { 'category': 'undetected', 'engine_name': 'Avast', 'engine_version': '23.9.8494.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Cynet': { 'category': 'undetected', 'engine_name': 'Cynet', 'engine_version': '4.0.0.27', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Kaspersky': { 'category': 'undetected', 'engine_name': 'Kaspersky', 'engine_version': '22.0.1.28', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'BitDefender': { 'category': 'undetected', 'engine_name': 'BitDefender', 'engine_version': '7.2', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'NANO-Antivirus': { 'category': 'undetected', 'engine_name': 'NANO-Antivirus', 'engine_version': '1.0.146.25796', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'ViRobot': { 'category': 'undetected', 'engine_name': 'ViRobot', 'engine_version': '2014.3.20.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'MicroWorld-eScan': { 'category': 'undetected', 'engine_name': 'MicroWorld-eScan', 'engine_version': '14.0.409.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Tencent': { 'category': 'undetected', 'engine_name': 'Tencent', 'engine_version': '1.0.0.1', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'TACHYON': { 'category': 'undetected', 'engine_name': 'TACHYON', 'engine_version': '2023-10-20.02', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Emsisoft': { 'category': 'undetected', 'engine_name': 'Emsisoft', 'engine_version': '2022.6.0.32461', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'F-Secure': { 'category': 'undetected', 'engine_name': 'F-Secure', 'engine_version': '18.10.1547.307', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'DrWeb': { 'category': 'undetected', 'engine_name': 'DrWeb', 'engine_version': '7.0.61.8090', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Zillya': { 'category': 'undetected', 'engine_name': 'Zillya', 'engine_version': '2.0.0.4978', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'TrendMicro': { 'category': 'undetected', 'engine_name': 'TrendMicro', 'engine_version': '11.0.0.1006', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'SentinelOne': { 'category': 'type-unsupported', 'engine_name': 'SentinelOne', 'engine_version': '23.4.0.1', 'result': null, 'method': 'blacklist', 'engine_update': '20231018' }, 'Trapmine': { 'category': 'type-unsupported', 'engine_name': 'Trapmine', 'engine_version': '4.0.14.94', 'result': null, 'method': 'blacklist', 'engine_update': '20231006' }, 'FireEye': { 'category': 'undetected', 'engine_name': 'FireEye', 'engine_version': '35.24.1.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Sophos': { 'category': 'undetected', 'engine_name': 'Sophos', 'engine_version': '2.3.1.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Paloalto': { 'category': 'type-unsupported', 'engine_name': 'Paloalto', 'engine_version': '0.9.0.1003', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Avast-Mobile': { 'category': 'type-unsupported', 'engine_name': 'Avast-Mobile', 'engine_version': '231020-00', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Jiangmin': { 'category': 'undetected', 'engine_name': 'Jiangmin', 'engine_version': '16.0.100', 'result': null, 'method': 'blacklist', 'engine_update': '20231019' }, 'Webroot': { 'category': 'type-unsupported', 'engine_name': 'Webroot', 'engine_version': '1.0.0.403', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Varist': { 'category': 'undetected', 'engine_name': 'Varist', 'engine_version': '6.5.1.2', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Avira': { 'category': 'undetected', 'engine_name': 'Avira', 'engine_version': '8.3.3.16', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Antiy-AVL': { 'category': 'undetected', 'engine_name': 'Antiy-AVL', 'engine_version': '3.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Kingsoft': { 'category': 'undetected', 'engine_name': 'Kingsoft', 'engine_version': '2023.8.30.1', 'result': null, 'method': 'blacklist', 'engine_update': '20230906' }, 'Microsoft': { 'category': 'undetected', 'engine_name': 'Microsoft', 'engine_version': '1.1.23090.2007', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Gridinsoft': { 'category': 'undetected', 'engine_name': 'Gridinsoft', 'engine_version': '1.0.143.174', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Xcitium': { 'category': 'undetected', 'engine_name': 'Xcitium', 'engine_version': '36101', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Arcabit': { 'category': 'undetected', 'engine_name': 'Arcabit', 'engine_version': '2022.0.0.18', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'SUPERAntiSpyware': { 'category': 'undetected', 'engine_name': 'SUPERAntiSpyware', 'engine_version': '5.6.0.1032', 'result': null, 'method': 'blacklist', 'engine_update': '20231017' }, 'ZoneAlarm': { 'category': 'undetected', 'engine_name': 'ZoneAlarm', 'engine_version': '1.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'GData': { 'category': 'undetected', 'engine_name': 'GData', 'engine_version': 'A:25.36690B:27.33569', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Google': { 'category': 'undetected', 'engine_name': 'Google', 'engine_version': '1697824834', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'BitDefenderFalx': { 'category': 'type-unsupported', 'engine_name': 'BitDefenderFalx', 'engine_version': '2.0.936', 'result': null, 'method': 'blacklist', 'engine_update': '20230921' }, 'AhnLab-V3': { 'category': 'undetected', 'engine_name': 'AhnLab-V3', 'engine_version': '3.24.0.10447', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Acronis': { 'category': 'undetected', 'engine_name': 'Acronis', 'engine_version': '1.2.0.121', 'result': null, 'method': 'blacklist', 'engine_update': '20230828' }, 'McAfee': { 'category': 'undetected', 'engine_name': 'McAfee', 'engine_version': '6.0.6.653', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'MAX': { 'category': 'undetected', 'engine_name': 'MAX', 'engine_version': '2023.1.4.1', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'DeepInstinct': { 'category': 'type-unsupported', 'engine_name': 'DeepInstinct', 'engine_version': '3.1.0.15', 'result': null, 'method': 'blacklist', 'engine_update': '20231019' }, 'VBA32': { 'category': 'undetected', 'engine_name': 'VBA32', 'engine_version': '5.0.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Cylance': { 'category': 'type-unsupported', 'engine_name': 'Cylance', 'engine_version': '2.0.0.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231004' }, 'Zoner': { 'category': 'undetected', 'engine_name': 'Zoner', 'engine_version': '2.2.2.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Rising': { 'category': 'undetected', 'engine_name': 'Rising', 'engine_version': '25.0.0.27', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Yandex': { 'category': 'undetected', 'engine_name': 'Yandex', 'engine_version': '5.5.2.24', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Ikarus': { 'category': 'undetected', 'engine_name': 'Ikarus', 'engine_version': '6.2.4.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'MaxSecure': { 'category': 'undetected', 'engine_name': 'MaxSecure', 'engine_version': '1.0.0.1', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Fortinet': { 'category': 'undetected', 'engine_name': 'Fortinet', 'engine_version': 'None', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'BitDefenderTheta': { 'category': 'undetected', 'engine_name': 'BitDefenderTheta', 'engine_version': '7.2.37796.0', 'result': null, 'method': 'blacklist', 'engine_update': '20230928' }, 'AVG': { 'category': 'undetected', 'engine_name': 'AVG', 'engine_version': '23.9.8494.0', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'Cybereason': { 'category': 'type-unsupported', 'engine_name': 'Cybereason', 'engine_version': '1.2.449', 'result': null, 'method': 'blacklist', 'engine_update': '20231011' }, 'Panda': { 'category': 'undetected', 'engine_name': 'Panda', 'engine_version': '4.6.4.2', 'result': null, 'method': 'blacklist', 'engine_update': '20231020' }, 'CrowdStrike': { 'category': 'type-unsupported', 'engine_name': 'CrowdStrike', 'engine_version': '1.0', 'result': null, 'method': 'blacklist', 'engine_update': null } }, 'trid': [ { 'file_type': 'file seems to be plain text/ASCII', 'probability': 0.0 } ], 'sha256': '1edba444e6495f08038239284a2808c93492d5039bf59d3e9ce7edcfd6d28981', 'tags': [ 'javascript' ], 'last_analysis_date': 1697830761, 'unique_sources': 1, 'first_submission_date': 1697830761, 'ssdeep': '3:OQA:/A', 'md5': 'd29d42212c2356eb0400ae4f9d2aca3d', 'sha1': '385f08e86ab6e0d75490a8fcdf6289f50c85ba4b', 'magic': 'ASCII text, with no line terminators', 'last_analysis_stats': { 'harmless': 0, 'type-unsupported': 16, 'suspicious': 0, 'confirmed-timeout': 0, 'timeout': 0, 'failure': 0, 'malicious': 0, 'undetected': 60 }, 'meaningful_name': 'valid.txt', 'reputation': 0 }, 'type': 'file', 'id': '1edba444e6495f08038239284a2808c93492d5039bf59d3e9ce7edcfd6d28981', 'links': { 'self': 'https://www.virustotal.com/api/v3/files/1edba444e6495f08038239284a2808c93492d5039bf59d3e9ce7edcfd6d28981' } }}"
}
]
ketola
commented
1 year ago Yes, this is close to what I had in mind, but I still see a possibility to use this result so that it could be interpreted a false negative.
In your example you loop through the results and if one of the results contains a non-ok status, then you consider the file infected and throw an exception. The same scenario that I described in the issue would lead to a false negative with this code: if there are no scanners, there would be no results and you might think that everyting is ok. You would need to additionally check that the list size is larger than zero to know that the file was scanned. That would work but it would be error-prone for a developer. I think the result object could be improved to make the result easier to interpret.
For example if the result would look like this:
public record ScanResult(
/**
* Scan status. Tells if the file was scanned succesfully or not.
*/
ScanStatus scanStatus,
/**
* Tells if the file was infected or not. If at least one plugin reported the file as infected then the value
* is true.
* Value is empty if the file was not scanned.
*/
Optional<Boolean> fileIsInfected,
/**
* List of results from individual plugins.
*/
List<PluginResult> pluginResults
) {
public enum ScanStatus {
OK, FILE_WAS_NOT_SCANNED
}
}Then you would always have to explicitly handle the situation where the file has not been scanned because you have to handle the possible Optional.empty for example by scanResult.fileIsInfected().orElseThrow(). But the more I think about this, the best option would be if the scanner could not work without at least one plugin. Then you would not need to handle this case at all and the result object and interpreting it would be simpler. The user of this extension could create a dummy plugin if they would want to skip the actual scanning for example in a dev environment.
melloware
commented
1 year ago I log a WARN if no scanners are detected
You can if results.empty() and throw an exception if you want. I don't want to constrain how people use it.
I return all the results for the same reason. Say ClamAV says it is OK but VirusTotal reports it has no info on this file? Some users may want this to be a failure and some might consider this OK. My example code where I throw an error after the first scanner is just an example.
Basically I return the results and it up to the consumer what they consider a failure. I think this makes it the most flexible and take all decision making out of the plug-in.
Those were my thoughts with this design.
melloware
commented
1 year ago With all that said if you want to submit a PR with your suggestions we can review it?
ketola
commented
1 year ago With all that said if you want to submit a PR with your suggestions we can review it?
I'd be happy to contribute code also but I'd rather first discuss and agree with you what kind of features and changes you would be interested to add to the extension.
melloware
commented
1 year ago OK yeah I guess I am just struggling with your proposal because the code in your case:
if (scanResult.getScanStatus() == FILE_WAS_NOT_SCANNED) {
throw new AntivirusException("No scanners were configured");
}is an improvement on the current:
if (results.isEmpty()) {
throw new AntivirusException("No scanners were configured");
}
First of all, this a very cool extension and something I've been looking for. Thanks for sharing this. I was able to successfully test the extension and here's my first thoughts:
In my first test the scanner appeared to be working ok until I created a test with the eicar test virus - which did not cause an exception that I was expecting. The reason was that I had not configured the
quarkus.antivirus.clamav.enabledproperty. Based on this, there's a risk that you could by accident misconfigure the extension and it would still seem to be working even though it's not doing any scanning at all.My ideas how to improve this.
Make the scan method return a value always. The returned object could contain information about the scan status and the plugins that were used to make the scan and their return values. I think this would be the best as it would be very clear when the file has been ok'd and how it was determined.
Second best would be to throw an exception if there are no scanners configured. The bad side with this is that if you would want to disable the scanners in some special scenario you would need to handle the exception.
Log a warning if there are no scanners configured. This would help a bit but of course it's easy to miss and might be that your log level would hide the message.
I would be interested in contributing if this leads to something that requires implementation.