diff options
author | Nick Zolnierz <nicholas.zolnierz@mongodb.com> | 2020-05-04 11:48:14 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-05-06 15:50:10 +0000 |
commit | e7b5473adb156afc20e8ff3b53446d5e17bec2d2 (patch) | |
tree | aa7dc323f38db83079c20f0baaa277af7c90ee23 | |
parent | ff38a192f588a01718f0ca259c147707819bd4cb (diff) | |
download | mongo-e7b5473adb156afc20e8ff3b53446d5e17bec2d2.tar.gz |
SERVER-45514 Reject document validators with encryption-related keywords if the validationAction is "warn" or validationLevel is "moderate"
This commit also contains a fix from SERVER-47834 which will not be
backported to 4.2.
-rw-r--r-- | jstests/core/doc_validation_encrypt_keywords.js | 98 | ||||
-rw-r--r-- | src/mongo/db/catalog/coll_mod.cpp | 39 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection.h | 2 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_impl.cpp | 36 | ||||
-rw-r--r-- | src/mongo/db/catalog/collection_impl.h | 2 | ||||
-rw-r--r-- | src/mongo/db/catalog/database_impl.cpp | 14 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/matcher/expression_parser.h | 3 | ||||
-rw-r--r-- | src/mongo/db/matcher/schema/json_schema_parser.cpp | 135 | ||||
-rw-r--r-- | src/mongo/db/matcher/schema/json_schema_parser.h | 9 | ||||
-rw-r--r-- | src/mongo/db/matcher/schema/json_schema_parser_test.cpp | 33 | ||||
-rw-r--r-- | src/mongo/db/pipeline/pipeline.h | 4 |
12 files changed, 300 insertions, 77 deletions
diff --git a/jstests/core/doc_validation_encrypt_keywords.js b/jstests/core/doc_validation_encrypt_keywords.js new file mode 100644 index 00000000000..de8303fe59d --- /dev/null +++ b/jstests/core/doc_validation_encrypt_keywords.js @@ -0,0 +1,98 @@ +// Verify encryption-related keywords are only allowed in document validators if the action is +// 'error' and validation level is 'strict'. +// +// Cannot implicitly shard accessed collections because of collection existing when none +// expected. +// @tags: [assumes_no_implicit_collection_creation_after_drop, requires_non_retryable_commands] +(function() { +"use strict"; + +var collName = "doc_validation_encrypt_keywords"; +var coll = db[collName]; +coll.drop(); + +const encryptSchema = { + $jsonSchema: {properties: {_id: {encrypt: {}}}} +}; +const nestedEncryptSchema = { + $jsonSchema: {properties: {user: {type: "object", properties: {ssn: {encrypt: {}}}}}} +}; +const encryptMetadataSchema = { + $jsonSchema: {encryptMetadata: {algorithm: "AEAD_AES_256_CBC_HMAC_SHA_512-Deterministic"}} +}; + +assert.commandFailedWithCode( + db.createCollection(collName, {validator: encryptSchema, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.createCollection(collName, {validator: nestedEncryptSchema, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.createCollection(collName, {validator: encryptMetadataSchema, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); + +assert.commandFailedWithCode( + db.createCollection(collName, {validator: encryptSchema, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.createCollection(collName, {validator: nestedEncryptSchema, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.createCollection(collName, {validator: encryptMetadataSchema, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); + +// Create the collection with a valid document validator and action 'warn'. +assert.commandWorked(db.createCollection( + collName, {validator: {$jsonSchema: {required: ["_id"]}}, validationAction: "warn"})); + +// Verify that we can't collMod the validator to include an encryption-related keyword. +assert.commandFailedWithCode(db.runCommand({collMod: collName, validator: encryptSchema}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode(db.runCommand({collMod: collName, validator: nestedEncryptSchema}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode(db.runCommand({collMod: collName, validator: encryptMetadataSchema}), + ErrorCodes.QueryFeatureNotAllowed); +coll.drop(); + +// Create the collection with an encrypted validator and action 'error'. +assert.commandWorked( + db.createCollection(collName, {validator: encryptSchema, validationAction: "error"})); + +// Verify that we can't collMod the validation action to 'warn' since the schema contains an +// encryption-related keyword. +assert.commandFailedWithCode(db.runCommand({collMod: collName, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); + +// Verify that we can't collMod the validation level to 'moderate' since the schema contains an +// encryption-related keyword. +assert.commandFailedWithCode(db.runCommand({collMod: collName, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); +coll.drop(); + +// Create the collection without a document validator. +assert.commandWorked(db.createCollection(collName)); + +// Verify that we can't collMod with an encrypted validator and validation action 'warn' or level +// 'moderate'. +assert.commandFailedWithCode( + db.runCommand({collMod: collName, validator: encryptSchema, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.runCommand({collMod: collName, validator: nestedEncryptSchema, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.runCommand({collMod: collName, validator: encryptMetadataSchema, validationAction: "warn"}), + ErrorCodes.QueryFeatureNotAllowed); + +assert.commandFailedWithCode( + db.runCommand({collMod: collName, validator: encryptSchema, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.runCommand({collMod: collName, validator: nestedEncryptSchema, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); +assert.commandFailedWithCode( + db.runCommand( + {collMod: collName, validator: encryptMetadataSchema, validationLevel: "moderate"}), + ErrorCodes.QueryFeatureNotAllowed); +coll.drop(); +})(); diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index acdb9aefab4..58d90f10234 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -69,9 +69,9 @@ struct CollModRequest { BSONElement indexExpireAfterSeconds = {}; BSONElement viewPipeLine = {}; std::string viewOn = {}; - BSONElement collValidator = {}; - std::string collValidationAction = {}; - std::string collValidationLevel = {}; + boost::optional<BSONObj> collValidator; + boost::optional<std::string> collValidationAction; + boost::optional<std::string> collValidationLevel; }; StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, @@ -181,15 +181,14 @@ StatusWith<CollModRequest> parseCollModRequest(OperationContext* opCtx, ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) { maxFeatureCompatibilityVersion = currentFCV; } - auto statusW = coll->parseValidator(opCtx, - e.Obj(), - MatchExpressionParser::kDefaultSpecialFeatures, - maxFeatureCompatibilityVersion); - if (!statusW.isOK()) { - return statusW.getStatus(); + auto swValidator = coll->parseValidator(opCtx, + e.Obj(), + MatchExpressionParser::kDefaultSpecialFeatures, + maxFeatureCompatibilityVersion); + if (!swValidator.isOK()) { + return swValidator.getStatus(); } - - cmr.collValidator = e; + cmr.collValidator = e.embeddedObject().getOwned(); } else if (fieldName == "validationLevel" && !isView) { auto status = coll->parseValidationLevel(e.String()); if (!status.isOK()) @@ -358,20 +357,22 @@ Status _collModInternal(OperationContext* opCtx, } } - // Save previous TTL index expiration. ttlInfo = TTLCollModInfo{Seconds(newExpireSecs.safeNumberLong()), Seconds(oldExpireSecs.safeNumberLong()), cmr.idx->indexName()}; } - // The Validator, ValidationAction and ValidationLevel are already parsed and must be OK. - if (!cmr.collValidator.eoo()) - invariant(coll->setValidator(opCtx, cmr.collValidator.Obj())); - if (!cmr.collValidationAction.empty()) - invariant(coll->setValidationAction(opCtx, cmr.collValidationAction)); - if (!cmr.collValidationLevel.empty()) - invariant(coll->setValidationLevel(opCtx, cmr.collValidationLevel)); + if (cmr.collValidator) { + uassertStatusOK(coll->setValidator(opCtx, std::move(*cmr.collValidator))); + } + if (cmr.collValidationAction) + uassertStatusOKWithContext(coll->setValidationAction(opCtx, *cmr.collValidationAction), + "Failed to set validationAction"); + if (cmr.collValidationLevel) { + uassertStatusOKWithContext(coll->setValidationLevel(opCtx, *cmr.collValidationLevel), + "Failed to set validationLevel"); + } // Upgrade unique indexes if (upgradeUniqueIndexes) { diff --git a/src/mongo/db/catalog/collection.h b/src/mongo/db/catalog/collection.h index 861bb372aa3..994b24b183f 100644 --- a/src/mongo/db/catalog/collection.h +++ b/src/mongo/db/catalog/collection.h @@ -389,7 +389,7 @@ public: * An empty validator removes all validation. * Requires an exclusive lock on the collection. */ - virtual Status setValidator(OperationContext* const opCtx, const BSONObj validator) = 0; + virtual Status setValidator(OperationContext* opCtx, BSONObj validator) = 0; virtual Status setValidationLevel(OperationContext* const opCtx, const StringData newLevel) = 0; virtual Status setValidationAction(OperationContext* const opCtx, diff --git a/src/mongo/db/catalog/collection_impl.cpp b/src/mongo/db/catalog/collection_impl.cpp index e9aea25010e..9b1a80d2bb7 100644 --- a/src/mongo/db/catalog/collection_impl.cpp +++ b/src/mongo/db/catalog/collection_impl.cpp @@ -59,6 +59,7 @@ #include "mongo/db/index/index_access_method.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/keypattern.h" +#include "mongo/db/matcher/expression_always_boolean.h" #include "mongo/db/matcher/expression_parser.h" #include "mongo/db/op_observer.h" #include "mongo/db/operation_context.h" @@ -270,6 +271,12 @@ void CollectionImpl::init(OperationContext* opCtx) { // Enforce that the validator can be used on this namespace. uassertStatusOK(checkValidatorCanBeUsedOnNs(_validatorDoc, ns(), _uuid)); + + // Make sure to parse the action and level before the MatchExpression, since certain features + // are not supported with certain combinations of action and level. + _validationAction = uassertStatusOK(_parseValidationAction(collectionOptions.validationAction)); + _validationLevel = uassertStatusOK(_parseValidationLevel(collectionOptions.validationLevel)); + // Store the result (OK / error) of parsing the validator, but do not enforce that the result is // OK. This is intentional, as users may have validators on disk which were considered well // formed in older versions but not in newer versions. @@ -280,8 +287,6 @@ void CollectionImpl::init(OperationContext* opCtx) { warning() << "Collection " << _ns << " has malformed validator: " << _swValidator.getStatus() << startupWarningsLog; } - _validationAction = uassertStatusOK(_parseValidationAction(collectionOptions.validationAction)); - _validationLevel = uassertStatusOK(_parseValidationLevel(collectionOptions.validationLevel)); getIndexCatalog()->init(opCtx).transitional_ignore(); infoCache()->init(opCtx); @@ -383,6 +388,12 @@ StatusWithMatchExpression CollectionImpl::parseValidator( // Enforce a maximum feature version if requested. expCtx->maxFeatureCompatibilityVersion = maxFeatureCompatibilityVersion; + // If the validation action is "warn" or the level is "moderate", then disallow any encryption + // keywords. This is to prevent any plaintext data from showing up in the logs. + if (_validationAction == CollectionImpl::ValidationAction::WARN || + _validationLevel == CollectionImpl::ValidationLevel::MODERATE) + allowedFeatures &= ~MatchExpressionParser::AllowedFeatures::kEncryptKeywords; + auto statusWithMatcher = MatchExpressionParser::parse(validator, expCtx, ExtensionsCallbackNoop(), allowedFeatures); @@ -942,6 +953,17 @@ Status CollectionImpl::setValidationLevel(OperationContext* opCtx, StringData ne auto oldValidationLevel = _validationLevel; _validationLevel = levelSW.getValue(); + // Reparse the validator as there are some features which are only supported with certain + // validation levels. + auto allowedFeatures = MatchExpressionParser::kAllowAllSpecialFeatures; + if (_validationLevel == CollectionImpl::ValidationLevel::MODERATE) + allowedFeatures &= ~MatchExpressionParser::AllowedFeatures::kEncryptKeywords; + + _swValidator = parseValidator(opCtx, _validatorDoc, allowedFeatures); + if (!_swValidator.isOK()) { + return _swValidator.getStatus(); + } + DurableCatalog::get(opCtx)->updateValidator( opCtx, ns(), _validatorDoc, getValidationLevel(), getValidationAction()); opCtx->recoveryUnit()->onRollback( @@ -961,6 +983,16 @@ Status CollectionImpl::setValidationAction(OperationContext* opCtx, StringData n auto oldValidationAction = _validationAction; _validationAction = actionSW.getValue(); + // Reparse the validator as there are some features which are only supported with certain + // validation actions. + auto allowedFeatures = MatchExpressionParser::kAllowAllSpecialFeatures; + if (_validationAction == CollectionImpl::ValidationAction::WARN) + allowedFeatures &= ~MatchExpressionParser::AllowedFeatures::kEncryptKeywords; + + _swValidator = parseValidator(opCtx, _validatorDoc, allowedFeatures); + if (!_swValidator.isOK()) { + return _swValidator.getStatus(); + } DurableCatalog::get(opCtx)->updateValidator( opCtx, ns(), _validatorDoc, getValidationLevel(), getValidationAction()); diff --git a/src/mongo/db/catalog/collection_impl.h b/src/mongo/db/catalog/collection_impl.h index 61e1a9cabd3..b0872609b9a 100644 --- a/src/mongo/db/catalog/collection_impl.h +++ b/src/mongo/db/catalog/collection_impl.h @@ -273,7 +273,7 @@ public: * An empty validator removes all validation. * Requires an exclusive lock on the collection. */ - Status setValidator(OperationContext* opCtx, BSONObj validator) final; + Status setValidator(OperationContext* opCtx, BSONObj validatorDoc) final; Status setValidationLevel(OperationContext* opCtx, StringData newLevel) final; Status setValidationAction(OperationContext* opCtx, StringData newAction) final; diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index 8642fd8068b..4a24a5907a4 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -860,8 +860,18 @@ Status DatabaseImpl::userCreateNS(OperationContext* opCtx, currentFCV != ServerGlobalParams::FeatureCompatibility::Version::kFullyUpgradedTo42) { expCtx->maxFeatureCompatibilityVersion = currentFCV; } - auto statusWithMatcher = - MatchExpressionParser::parse(collectionOptions.validator, std::move(expCtx)); + + // If the validation action is "warn" or the level is "moderate", then disallow any + // encryption keywords. This is to prevent any plaintext data from showing up in the logs. + auto allowedFeatures = MatchExpressionParser::kDefaultSpecialFeatures; + if (collectionOptions.validationAction == "warn" || + collectionOptions.validationLevel == "moderate") + allowedFeatures &= ~MatchExpressionParser::AllowedFeatures::kEncryptKeywords; + + auto statusWithMatcher = MatchExpressionParser::parse(collectionOptions.validator, + std::move(expCtx), + ExtensionsCallbackNoop(), + allowedFeatures); // We check the status of the parse to see if there are any banned features, but we don't // actually need the result for now. diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 98a64e2d4bd..8c583de95d1 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -389,7 +389,7 @@ StatusWithMatchExpression parseJSONSchema(StringData name, } return JSONSchemaParser::parse( - expCtx, elem.Obj(), internalQueryIgnoreUnknownJSONSchemaKeywords.load()); + expCtx, elem.Obj(), allowedFeatures, internalQueryIgnoreUnknownJSONSchemaKeywords.load()); } template <class T> diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index 40aa8eb965a..b50aef788ce 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -98,13 +98,14 @@ public: kJavascript = 1 << 2, kExpr = 1 << 3, kJSONSchema = 1 << 4, + kEncryptKeywords = 1 << 5, }; using AllowedFeatureSet = unsigned long long; static constexpr AllowedFeatureSet kBanAllSpecialFeatures = 0; static constexpr AllowedFeatureSet kAllowAllSpecialFeatures = std::numeric_limits<unsigned long long>::max(); static constexpr AllowedFeatureSet kDefaultSpecialFeatures = - AllowedFeatures::kExpr | AllowedFeatures::kJSONSchema; + AllowedFeatures::kExpr | AllowedFeatures::kJSONSchema | AllowedFeatures::kEncryptKeywords; /** * Parses PathAcceptingKeyword from 'typeElem'. Returns 'defaultKeyword' if 'typeElem' diff --git a/src/mongo/db/matcher/schema/json_schema_parser.cpp b/src/mongo/db/matcher/schema/json_schema_parser.cpp index 72ed89bf3f6..b6771ab9304 100644 --- a/src/mongo/db/matcher/schema/json_schema_parser.cpp +++ b/src/mongo/db/matcher/schema/json_schema_parser.cpp @@ -66,6 +66,7 @@ namespace mongo { using PatternSchema = InternalSchemaAllowedPropertiesMatchExpression::PatternSchema; using Pattern = InternalSchemaAllowedPropertiesMatchExpression::Pattern; +using AllowedFeatureSet = MatchExpressionParser::AllowedFeatureSet; namespace { @@ -93,6 +94,7 @@ constexpr StringData kNamePlaceholder = "i"_sd; StatusWithMatchExpression _parse(const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, BSONObj schema, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords); /** @@ -303,6 +305,7 @@ template <class T> StatusWithMatchExpression parseLogicalKeyword(const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, BSONElement logicalElement, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { if (logicalElement.type() != BSONType::Array) { return {ErrorCodes::TypeMismatch, @@ -326,7 +329,8 @@ StatusWithMatchExpression parseLogicalKeyword(const boost::intrusive_ptr<Express << elem.type()}; } - auto nestedSchemaMatch = _parse(expCtx, path, elem.embeddedObject(), ignoreUnknownKeywords); + auto nestedSchemaMatch = + _parse(expCtx, path, elem.embeddedObject(), allowedFeatures, ignoreUnknownKeywords); if (!nestedSchemaMatch.isOK()) { return nestedSchemaMatch.getStatus(); } @@ -460,6 +464,7 @@ StatusWithMatchExpression parseProperties(const boost::intrusive_ptr<ExpressionC BSONElement propertiesElt, InternalSchemaTypeExpression* typeExpr, const StringDataSet& requiredProperties, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { if (propertiesElt.type() != BSONType::Object) { return {Status(ErrorCodes::TypeMismatch, @@ -480,6 +485,7 @@ StatusWithMatchExpression parseProperties(const boost::intrusive_ptr<ExpressionC auto nestedSchemaMatch = _parse(expCtx, property.fieldNameStringData(), property.embeddedObject(), + allowedFeatures, ignoreUnknownKeywords); if (!nestedSchemaMatch.isOK()) { return nestedSchemaMatch.getStatus(); @@ -520,6 +526,7 @@ StatusWithMatchExpression parseProperties(const boost::intrusive_ptr<ExpressionC StatusWith<std::vector<PatternSchema>> parsePatternProperties( const boost::intrusive_ptr<ExpressionContext>& expCtx, BSONElement patternPropertiesElt, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { std::vector<PatternSchema> patternProperties; if (!patternPropertiesElt) { @@ -545,8 +552,11 @@ StatusWith<std::vector<PatternSchema>> parsePatternProperties( // Parse the nested schema using a placeholder as the path, since we intend on using the // resulting match expression inside an ExpressionWithPlaceholder. - auto nestedSchemaMatch = - _parse(expCtx, kNamePlaceholder, patternSchema.embeddedObject(), ignoreUnknownKeywords); + auto nestedSchemaMatch = _parse(expCtx, + kNamePlaceholder, + patternSchema.embeddedObject(), + allowedFeatures, + ignoreUnknownKeywords); if (!nestedSchemaMatch.isOK()) { return nestedSchemaMatch.getStatus(); } @@ -563,6 +573,7 @@ StatusWith<std::vector<PatternSchema>> parsePatternProperties( StatusWithMatchExpression parseAdditionalProperties( const boost::intrusive_ptr<ExpressionContext>& expCtx, BSONElement additionalPropertiesElt, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { if (!additionalPropertiesElt) { // The absence of the 'additionalProperties' keyword is identical in meaning to the presence @@ -588,8 +599,11 @@ StatusWithMatchExpression parseAdditionalProperties( // Parse the nested schema using a placeholder as the path, since we intend on using the // resulting match expression inside an ExpressionWithPlaceholder. - auto nestedSchemaMatch = _parse( - expCtx, kNamePlaceholder, additionalPropertiesElt.embeddedObject(), ignoreUnknownKeywords); + auto nestedSchemaMatch = _parse(expCtx, + kNamePlaceholder, + additionalPropertiesElt.embeddedObject(), + allowedFeatures, + ignoreUnknownKeywords); if (!nestedSchemaMatch.isOK()) { return nestedSchemaMatch.getStatus(); } @@ -608,6 +622,7 @@ StatusWithMatchExpression parseAllowedProperties( BSONElement patternPropertiesElt, BSONElement additionalPropertiesElt, InternalSchemaTypeExpression* typeExpr, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { // Collect the set of properties named by the 'properties' keyword. StringDataSet propertyNames; @@ -619,14 +634,14 @@ StatusWithMatchExpression parseAllowedProperties( propertyNames.insert(propertyNamesVec.begin(), propertyNamesVec.end()); } - auto patternProperties = - parsePatternProperties(expCtx, patternPropertiesElt, ignoreUnknownKeywords); + auto patternProperties = parsePatternProperties( + expCtx, patternPropertiesElt, allowedFeatures, ignoreUnknownKeywords); if (!patternProperties.isOK()) { return patternProperties.getStatus(); } - auto otherwiseExpr = - parseAdditionalProperties(expCtx, additionalPropertiesElt, ignoreUnknownKeywords); + auto otherwiseExpr = parseAdditionalProperties( + expCtx, additionalPropertiesElt, allowedFeatures, ignoreUnknownKeywords); if (!otherwiseExpr.isOK()) { return otherwiseExpr.getStatus(); } @@ -693,11 +708,12 @@ StatusWithMatchExpression translateSchemaDependency( const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, BSONElement dependency, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { invariant(dependency.type() == BSONType::Object); auto nestedSchemaMatch = - _parse(expCtx, path, dependency.embeddedObject(), ignoreUnknownKeywords); + _parse(expCtx, path, dependency.embeddedObject(), allowedFeatures, ignoreUnknownKeywords); if (!nestedSchemaMatch.isOK()) { return nestedSchemaMatch.getStatus(); } @@ -775,6 +791,7 @@ StatusWithMatchExpression translatePropertyDependency(StringData path, BSONEleme StatusWithMatchExpression parseDependencies(const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, BSONElement dependencies, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { if (dependencies.type() != BSONType::Object) { return {ErrorCodes::TypeMismatch, @@ -794,7 +811,8 @@ StatusWithMatchExpression parseDependencies(const boost::intrusive_ptr<Expressio } auto dependencyExpr = (dependency.type() == BSONType::Object) - ? translateSchemaDependency(expCtx, path, dependency, ignoreUnknownKeywords) + ? translateSchemaDependency( + expCtx, path, dependency, allowedFeatures, ignoreUnknownKeywords) : translatePropertyDependency(path, dependency); if (!dependencyExpr.isOK()) { return dependencyExpr.getStatus(); @@ -832,6 +850,7 @@ StatusWith<boost::optional<long long>> parseItems( const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, BSONElement itemsElt, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords, InternalSchemaTypeExpression* typeExpr, AndMatchExpression* andExpr) { @@ -853,8 +872,11 @@ StatusWith<boost::optional<long long>> parseItems( // We want to make an ExpressionWithPlaceholder for $_internalSchemaMatchArrayIndex, // so we use our default placeholder as the path. - auto parsedSubschema = - _parse(expCtx, kNamePlaceholder, subschema.embeddedObject(), ignoreUnknownKeywords); + auto parsedSubschema = _parse(expCtx, + kNamePlaceholder, + subschema.embeddedObject(), + allowedFeatures, + ignoreUnknownKeywords); if (!parsedSubschema.isOK()) { return parsedSubschema.getStatus(); } @@ -878,8 +900,11 @@ StatusWith<boost::optional<long long>> parseItems( // When "items" is an object, generate a single AllElemMatchFromIndex that applies to every // element in the array to match. The parsed expression is intended for an // ExpressionWithPlaceholder, so we use the default placeholder as the path. - auto nestedItemsSchema = - _parse(expCtx, kNamePlaceholder, itemsElt.embeddedObject(), ignoreUnknownKeywords); + auto nestedItemsSchema = _parse(expCtx, + kNamePlaceholder, + itemsElt.embeddedObject(), + allowedFeatures, + ignoreUnknownKeywords); if (!nestedItemsSchema.isOK()) { return nestedItemsSchema.getStatus(); } @@ -909,6 +934,7 @@ Status parseAdditionalItems(const boost::intrusive_ptr<ExpressionContext>& expCt StringData path, BSONElement additionalItemsElt, boost::optional<long long> startIndexForAdditionalItems, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords, InternalSchemaTypeExpression* typeExpr, AndMatchExpression* andExpr) { @@ -923,8 +949,11 @@ Status parseAdditionalItems(const boost::intrusive_ptr<ExpressionContext>& expCt emptyPlaceholder, stdx::make_unique<AlwaysFalseMatchExpression>()); } } else if (additionalItemsElt.type() == BSONType::Object) { - auto parsedOtherwiseExpr = _parse( - expCtx, kNamePlaceholder, additionalItemsElt.embeddedObject(), ignoreUnknownKeywords); + auto parsedOtherwiseExpr = _parse(expCtx, + kNamePlaceholder, + additionalItemsElt.embeddedObject(), + allowedFeatures, + ignoreUnknownKeywords); if (!parsedOtherwiseExpr.isOK()) { return parsedOtherwiseExpr.getStatus(); } @@ -956,12 +985,14 @@ Status parseAdditionalItems(const boost::intrusive_ptr<ExpressionContext>& expCt Status parseItemsAndAdditionalItems(StringMap<BSONElement>& keywordMap, const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords, InternalSchemaTypeExpression* typeExpr, AndMatchExpression* andExpr) { boost::optional<long long> startIndexForAdditionalItems; if (auto itemsElt = keywordMap[JSONSchemaParser::kSchemaItemsKeyword]) { - auto index = parseItems(expCtx, path, itemsElt, ignoreUnknownKeywords, typeExpr, andExpr); + auto index = parseItems( + expCtx, path, itemsElt, allowedFeatures, ignoreUnknownKeywords, typeExpr, andExpr); if (!index.isOK()) { return index.getStatus(); } @@ -973,6 +1004,7 @@ Status parseItemsAndAdditionalItems(StringMap<BSONElement>& keywordMap, path, additionalItemsElt, startIndexForAdditionalItems, + allowedFeatures, ignoreUnknownKeywords, typeExpr, andExpr); @@ -995,10 +1027,11 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, AndMatchExpression* andExpr, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { if (auto allOfElt = keywordMap[JSONSchemaParser::kSchemaAllOfKeyword]) { - auto allOfExpr = - parseLogicalKeyword<AndMatchExpression>(expCtx, path, allOfElt, ignoreUnknownKeywords); + auto allOfExpr = parseLogicalKeyword<AndMatchExpression>( + expCtx, path, allOfElt, allowedFeatures, ignoreUnknownKeywords); if (!allOfExpr.isOK()) { return allOfExpr.getStatus(); } @@ -1006,8 +1039,8 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, } if (auto anyOfElt = keywordMap[JSONSchemaParser::kSchemaAnyOfKeyword]) { - auto anyOfExpr = - parseLogicalKeyword<OrMatchExpression>(expCtx, path, anyOfElt, ignoreUnknownKeywords); + auto anyOfExpr = parseLogicalKeyword<OrMatchExpression>( + expCtx, path, anyOfElt, allowedFeatures, ignoreUnknownKeywords); if (!anyOfExpr.isOK()) { return anyOfExpr.getStatus(); } @@ -1016,7 +1049,7 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, if (auto oneOfElt = keywordMap[JSONSchemaParser::kSchemaOneOfKeyword]) { auto oneOfExpr = parseLogicalKeyword<InternalSchemaXorMatchExpression>( - expCtx, path, oneOfElt, ignoreUnknownKeywords); + expCtx, path, oneOfElt, allowedFeatures, ignoreUnknownKeywords); if (!oneOfExpr.isOK()) { return oneOfExpr.getStatus(); } @@ -1031,7 +1064,8 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, << notElt.type()}; } - auto parsedExpr = _parse(expCtx, path, notElt.embeddedObject(), ignoreUnknownKeywords); + auto parsedExpr = + _parse(expCtx, path, notElt.embeddedObject(), allowedFeatures, ignoreUnknownKeywords); if (!parsedExpr.isOK()) { return parsedExpr.getStatus(); } @@ -1065,6 +1099,7 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, Status translateArrayKeywords(StringMap<BSONElement>& keywordMap, const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords, InternalSchemaTypeExpression* typeExpr, AndMatchExpression* andExpr) { @@ -1095,7 +1130,7 @@ Status translateArrayKeywords(StringMap<BSONElement>& keywordMap, } return parseItemsAndAdditionalItems( - keywordMap, expCtx, path, ignoreUnknownKeywords, typeExpr, andExpr); + keywordMap, expCtx, path, allowedFeatures, ignoreUnknownKeywords, typeExpr, andExpr); } /** @@ -1116,6 +1151,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, StringData path, InternalSchemaTypeExpression* typeExpr, AndMatchExpression* andExpr, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { StringDataSet requiredProperties; if (auto requiredElt = keywordMap[JSONSchemaParser::kSchemaRequiredKeyword]) { @@ -1127,8 +1163,13 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, } if (auto propertiesElt = keywordMap[JSONSchemaParser::kSchemaPropertiesKeyword]) { - auto propertiesExpr = parseProperties( - expCtx, path, propertiesElt, typeExpr, requiredProperties, ignoreUnknownKeywords); + auto propertiesExpr = parseProperties(expCtx, + path, + propertiesElt, + typeExpr, + requiredProperties, + allowedFeatures, + ignoreUnknownKeywords); if (!propertiesExpr.isOK()) { return propertiesExpr.getStatus(); } @@ -1148,6 +1189,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, patternPropertiesElt, additionalPropertiesElt, typeExpr, + allowedFeatures, ignoreUnknownKeywords); if (!allowedPropertiesExpr.isOK()) { return allowedPropertiesExpr.getStatus(); @@ -1183,8 +1225,8 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, } if (auto dependenciesElt = keywordMap[JSONSchemaParser::kSchemaDependenciesKeyword]) { - auto dependenciesExpr = - parseDependencies(expCtx, path, dependenciesElt, ignoreUnknownKeywords); + auto dependenciesExpr = parseDependencies( + expCtx, path, dependenciesElt, allowedFeatures, ignoreUnknownKeywords); if (!dependenciesExpr.isOK()) { return dependenciesExpr.getStatus(); } @@ -1309,6 +1351,7 @@ Status translateScalarKeywords(StringMap<BSONElement>& keywordMap, Status translateEncryptionKeywords(StringMap<BSONElement>& keywordMap, const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, + AllowedFeatureSet allowedFeatures, AndMatchExpression* andExpr) { auto encryptElt = keywordMap[JSONSchemaParser::kSchemaEncryptKeyword]; auto encryptMetadataElt = keywordMap[JSONSchemaParser::kSchemaEncryptMetadataKeyword]; @@ -1323,6 +1366,11 @@ Status translateEncryptionKeywords(StringMap<BSONElement>& keywordMap, << feature_compatibility_version_documentation::kUpgradeLink << "."); } + if ((allowedFeatures & MatchExpressionParser::AllowedFeatures::kEncryptKeywords) == 0u && + (encryptElt || encryptMetadataElt)) + return Status(ErrorCodes::QueryFeatureNotAllowed, + "Encryption-related validator keywords are not allowed in this context"); + if (encryptElt && encryptMetadataElt) { return Status(ErrorCodes::FailedToParse, str::stream() << "Cannot specify both $jsonSchema keywords '" @@ -1408,6 +1456,7 @@ Status validateMetadataKeywords(StringMap<BSONElement>& keywordMap) { StatusWithMatchExpression _parse(const boost::intrusive_ptr<ExpressionContext>& expCtx, StringData path, BSONObj schema, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { // Map from JSON Schema keyword to the corresponding element from 'schema', or to an empty // BSONElement if the JSON Schema keyword is not specified. @@ -1532,25 +1581,36 @@ StatusWithMatchExpression _parse(const boost::intrusive_ptr<ExpressionContext>& return translationStatus; } - translationStatus = translateArrayKeywords( - keywordMap, expCtx, path, ignoreUnknownKeywords, typeExpr.get(), andExpr.get()); + translationStatus = translateArrayKeywords(keywordMap, + expCtx, + path, + allowedFeatures, + ignoreUnknownKeywords, + typeExpr.get(), + andExpr.get()); if (!translationStatus.isOK()) { return translationStatus; } - translationStatus = translateEncryptionKeywords(keywordMap, expCtx, path, andExpr.get()); + translationStatus = + translateEncryptionKeywords(keywordMap, expCtx, path, allowedFeatures, andExpr.get()); if (!translationStatus.isOK()) { return translationStatus; } - translationStatus = translateObjectKeywords( - keywordMap, expCtx, path, typeExpr.get(), andExpr.get(), ignoreUnknownKeywords); + translationStatus = translateObjectKeywords(keywordMap, + expCtx, + path, + typeExpr.get(), + andExpr.get(), + allowedFeatures, + ignoreUnknownKeywords); if (!translationStatus.isOK()) { return translationStatus; } - translationStatus = - translateLogicalKeywords(keywordMap, expCtx, path, andExpr.get(), ignoreUnknownKeywords); + translationStatus = translateLogicalKeywords( + keywordMap, expCtx, path, andExpr.get(), allowedFeatures, ignoreUnknownKeywords); if (!translationStatus.isOK()) { return translationStatus; } @@ -1617,10 +1677,11 @@ StatusWith<MatcherTypeSet> JSONSchemaParser::parseTypeSet(BSONElement typeElt, StatusWithMatchExpression JSONSchemaParser::parse( const boost::intrusive_ptr<ExpressionContext>& expCtx, BSONObj schema, + AllowedFeatureSet allowedFeatures, bool ignoreUnknownKeywords) { LOG(5) << "Parsing JSON Schema: " << schema.jsonString(); try { - auto translation = _parse(expCtx, ""_sd, schema, ignoreUnknownKeywords); + auto translation = _parse(expCtx, ""_sd, schema, allowedFeatures, ignoreUnknownKeywords); if (shouldLog(logger::LogSeverity::Debug(5)) && translation.isOK()) { LOG(5) << "Translated schema match expression: " << translation.getValue()->debugString(); diff --git a/src/mongo/db/matcher/schema/json_schema_parser.h b/src/mongo/db/matcher/schema/json_schema_parser.h index 463b15d72af..8b178cdf782 100644 --- a/src/mongo/db/matcher/schema/json_schema_parser.h +++ b/src/mongo/db/matcher/schema/json_schema_parser.h @@ -87,9 +87,12 @@ public: * Converts a JSON schema, represented as BSON, into a semantically equivalent match expression * tree. Returns a non-OK status if the schema is invalid or cannot be parsed. */ - static StatusWithMatchExpression parse(const boost::intrusive_ptr<ExpressionContext>& expCtx, - BSONObj schema, - bool ignoreUnknownKeywords = false); + static StatusWithMatchExpression parse( + const boost::intrusive_ptr<ExpressionContext>& expCtx, + BSONObj schema, + MatchExpressionParser::AllowedFeatureSet allowedFeatures = + MatchExpressionParser::kAllowAllSpecialFeatures, + bool ignoreUnknownKeywords = false); /** * Builds a set of type aliases from the given type element using 'aliasMap'. Returns a non-OK diff --git a/src/mongo/db/matcher/schema/json_schema_parser_test.cpp b/src/mongo/db/matcher/schema/json_schema_parser_test.cpp index 809710f5682..3b573621586 100644 --- a/src/mongo/db/matcher/schema/json_schema_parser_test.cpp +++ b/src/mongo/db/matcher/schema/json_schema_parser_test.cpp @@ -1536,53 +1536,67 @@ TEST(JSONSchemaParserTest, UniqueItemsTranslatesCorrectlyWithTypeArray) { TEST(JSONSchemaParserTest, CorrectlyIgnoresUnknownKeywordsParameterIsSet) { const auto ignoreUnknownKeywords = true; + const auto allowedFeatures = MatchExpressionParser::kAllowAllSpecialFeatures; auto schema = fromjson("{ignored_keyword: 1}"); - ASSERT_OK(JSONSchemaParser::parse(new ExpressionContextForTest(), schema, ignoreUnknownKeywords) + ASSERT_OK(JSONSchemaParser::parse( + new ExpressionContextForTest(), schema, allowedFeatures, ignoreUnknownKeywords) .getStatus()); schema = fromjson("{properties: {a: {ignored_keyword: 1}}}"); - ASSERT_OK(JSONSchemaParser::parse(new ExpressionContextForTest(), schema, ignoreUnknownKeywords) + ASSERT_OK(JSONSchemaParser::parse( + new ExpressionContextForTest(), schema, allowedFeatures, ignoreUnknownKeywords) .getStatus()); schema = fromjson("{properties: {a: {oneOf: [{ignored_keyword: {}}]}}}"); - ASSERT_OK(JSONSchemaParser::parse(new ExpressionContextForTest(), schema, ignoreUnknownKeywords) + ASSERT_OK(JSONSchemaParser::parse( + new ExpressionContextForTest(), schema, allowedFeatures, ignoreUnknownKeywords) .getStatus()); } TEST(JSONSchemaParserTest, FailsToParseUnsupportedKeywordsWhenIgnoreUnknownParameterIsSet) { const auto ignoreUnknownKeywords = true; + const auto allowedFeatures = MatchExpressionParser::kAllowAllSpecialFeatures; - auto result = JSONSchemaParser::parse( - new ExpressionContextForTest(), fromjson("{default: {}}"), ignoreUnknownKeywords); + auto result = JSONSchemaParser::parse(new ExpressionContextForTest(), + fromjson("{default: {}}"), + allowedFeatures, + ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword 'default' is not currently supported"); result = JSONSchemaParser::parse(new ExpressionContextForTest(), fromjson("{definitions: {numberField: {type: 'number'}}}"), + allowedFeatures, ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword 'definitions' is not currently supported"); - result = JSONSchemaParser::parse( - new ExpressionContextForTest(), fromjson("{format: 'email'}"), ignoreUnknownKeywords); + result = JSONSchemaParser::parse(new ExpressionContextForTest(), + fromjson("{format: 'email'}"), + allowedFeatures, + ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword 'format' is not currently supported"); - result = JSONSchemaParser::parse( - new ExpressionContextForTest(), fromjson("{id: 'someschema.json'}"), ignoreUnknownKeywords); + result = JSONSchemaParser::parse(new ExpressionContextForTest(), + fromjson("{id: 'someschema.json'}"), + allowedFeatures, + ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword 'id' is not currently supported"); result = JSONSchemaParser::parse(new ExpressionContextForTest(), BSON("$ref" << "#/definitions/positiveInt"), + allowedFeatures, ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword '$ref' is not currently supported"); result = JSONSchemaParser::parse(new ExpressionContextForTest(), fromjson("{$schema: 'hyper-schema'}"), + allowedFeatures, ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword '$schema' is not currently supported"); @@ -1590,6 +1604,7 @@ TEST(JSONSchemaParserTest, FailsToParseUnsupportedKeywordsWhenIgnoreUnknownParam result = JSONSchemaParser::parse(new ExpressionContextForTest(), fromjson("{$schema: 'http://json-schema.org/draft-04/schema#'}"), + allowedFeatures, ignoreUnknownKeywords); ASSERT_STRING_CONTAINS(result.getStatus().reason(), "$jsonSchema keyword '$schema' is not currently supported"); diff --git a/src/mongo/db/pipeline/pipeline.h b/src/mongo/db/pipeline/pipeline.h index 06c35e36c1c..b6dc48a9ccd 100644 --- a/src/mongo/db/pipeline/pipeline.h +++ b/src/mongo/db/pipeline/pipeline.h @@ -75,7 +75,8 @@ public: static constexpr MatchExpressionParser::AllowedFeatureSet kAllowedMatcherFeatures = MatchExpressionParser::AllowedFeatures::kText | MatchExpressionParser::AllowedFeatures::kExpr | - MatchExpressionParser::AllowedFeatures::kJSONSchema; + MatchExpressionParser::AllowedFeatures::kJSONSchema | + MatchExpressionParser::AllowedFeatures::kEncryptKeywords; /** * The match expression features allowed when running a pipeline with $geoNear. @@ -84,6 +85,7 @@ public: MatchExpressionParser::AllowedFeatures::kText | MatchExpressionParser::AllowedFeatures::kExpr | MatchExpressionParser::AllowedFeatures::kJSONSchema | + MatchExpressionParser::AllowedFeatures::kEncryptKeywords | MatchExpressionParser::AllowedFeatures::kGeoNear; /** |