Skip to content
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.

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

add schema file #343

Closed
wants to merge 1 commit into from
Closed

add schema file #343

wants to merge 1 commit into from

Conversation

romanojd
Copy link
Contributor

@romanojd romanojd commented Sep 1, 2019

I validated the oc2ls schema against the test messages that Brian Berliner developed: https://github.com/bberliner/openc2-json-schema/tree/master/src/test/resources. I did not have a chance to validate it against Danny Martinez's test messages.

The results are as follows:

fail/bberliner/commands/empty_object.json
fail/bberliner/commands/empty_array.json
fail/bberliner/responses/empty_object.json
fail/bberliner/responses/empty_array.json

  • Empty object ({}) and empty array ([]) don't fail like they should, but I think that is an issue with the python validator that I am using.

fail/bberliner/commands/query_features_ext_args_nox-.json

  • This should and does PASS the validation against oc2ls schema. The oc2ls schema has no knowledge of other Actuator Profiles and therefore should PASS any NSID that conforms to the extension rules.

fail/bberliner/responses/results_unknown_profile.json

  • The "results" property contains the NSID of the Actuator with an empty object ({..., "results": {"mycompany": {}}}). The oc2ls schema validation will PASS because the oc2ls schema doesn't know anything about the valid Actuator Profile results.

fail/bberliner/responses/status_asdouble.json

  • The oc2ls schema validation will PASS the status code represented as a double. I believe that the python validator converts the the number 200.0 to an integer.

pass/bberliner/commands/query_features_extension_args_number.json
pass/bberliner/commands/query_features_ext_args_all.json

  • The Language Specification does not specify this, but I believe that the NSID and the extended names should begin with a letter. Numbers or underscores should not begin NSIDs or extended names. It's perfectly fine if the Language Subcommittee comes to a different conclusion. The "patternProperties" is easily changed. This affects the definitions of Target, Actuator, Args, and Results.
    • Begin with letter: [A-Za-z]\\w*
    • Begin with letter or underscore: [A-Za-z_]\\w*
    • Begin with letter or number: [A-Za-z0-9]\\w*
    • Begin with letter, number, or underscore: \\w+

pass/bberliner/responses/results_empty.json

  • This test message should represent a failure case. The results property of a Response is optional, but if present, it should contain a minimum of one property.

pass/bberliner/responses/results_ext_single.json
pass/bberliner/responses/results_ext_multiple.json

  • These two test messages fail because the status code is set to 201, which was not defined in the Language Specification. When set to 200, these messages are both verified to pass. If we were to allow status code 201 then we would need to either add it to the definition or all for extensions.

My validation code can be found at https://github.com/romanojd/openc2-schema-test.

@romanojd
Copy link
Contributor Author

romanojd commented Sep 1, 2019

I'm going to leave it up to the LSC to merge this pull request. I would recommend merging it to the working branch as the initial schema and then everybody can suggest more changes and improvements to it at that point.

@romanojd
Copy link
Contributor Author

romanojd commented Sep 1, 2019

You could also include separate Command and Response schema files that reference the core schema file. I have examples here:

@davaya
Copy link
Contributor

davaya commented Sep 6, 2019

The JSON schema automatically generated from the property tables, using the standard Python validator (jsonschema v3.0.2, Julian Berman) correctly rejects empty object and empty array:
commands/bad/empty_array: Fail: [] is not of type 'object'
commands/bad/empty_object: Fail: 'action' is a required property
responses/bad/empty_array: Fail: [] is not of type 'object'
responses/bad/empty_object: Fail: 'status' is a required property

commands/bad/query_features_ext_args_nox-.json: Fail: Additional properties are not allowed ('mycompany' was unexpected)
responses/bad/results_unknown_profile: Fail: Additional properties are not allowed ('mycompany' was unexpected)

The goal is schizophrenic here - it intends to be generic but it rejects profiles like slpf and mycompany unless they are explicitly shown in the property tables. As written, the LS shows slpf as an example but does not show mycompany as an example, so slpf messages are accepted and mycompany messages are rejected using the automatically generated JSON Schema. The schema used by every OpenC2 product MUST list all profiles that it supports in the property tables. But if there is an intent to create a generic schema then profiles not shown in the property tables should also be accepted.

responses/bad/results_asdouble: Brian has mischaracterized this as a bad response.

Unlike CBOR, JSON doesn't have an integer type, so 200.0 is the same number as 200. JSON Schema section 6.3 says "For consistency, integer JSON numbers SHOULD NOT be encoded with a fractional part.", but if the encoder doesn't follow that advice, the decoder has no choice but to accept it as a valid number.

commands/good/query_features_extension_args_number: The property table JSON schema accepts x-395 as a valid property name (it is not an NSID - see #342) if it is listed in the property tables and rejects it if it is not listed. If a schema is intended to be generic, then it should accept any property name.

responses/good/results_empty: Brian has mischaracterized this as a good response. The language spec requires at least one result, and the auto-generated schema correctly fails this "good" response: Fail: {} does not have enough properties.

responses/good/results_ext_multiple:
responses/good/results_ext_single: According to the property tables, these are bad responses. But 201 Create is used often enough that we should add it to the property tables.

@dlemire60
Copy link
Contributor

dlemire60 commented Oct 7, 2020

This PR was discussed at the 7 Oct 2020 L-SC meeting. Consensus was to leave the PR open, but it should be reviewed / revisited / updated by currently active TC members. @davaya noted that more flexibility is needed, to request the desired schema.

Meeting notes in Lucid, under agenda item 2.0: https://meet.lucidmeetings.com/lucid/meeting/264285.

@dlemire60
Copy link
Contributor

This PR has become OBE with the publication of the JADN specification & CN. Per discussion / agreement at 29 Nov '23 working meeting it is being closed w/o merging.

@dlemire60 dlemire60 closed this Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants