-
Notifications
You must be signed in to change notification settings - Fork 118
Security-Baseline #571
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
Security-Baseline #571
Conversation
Add RFC definintions.
Add RFC references.
Remove CRL requirements on TLS client authentication.
Simplify requirements.
doc/SecurityBaseline.xml
Outdated
| <tbody valign="top"> | ||
| <row> | ||
| <entry> | ||
| <para>Assymetric Encryption</para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <para>Assymetric Encryption</para> | |
| <para>Asymmetric Encryption</para> |
doc/SecurityBaseline.xml
Outdated
| </chapter> | ||
| <chapter> | ||
| <title>Overview</title> | ||
| <para>The content of this is document bases on state of the art technology as published by the American institute NIST and the German department BSI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <para>The content of this is document bases on state of the art technology as published by the American institute NIST and the German department BSI. | |
| <para>The content of this document is based on state of the art technology as published by the American institute NIST and the German department BSI. |
doc/SecurityBaseline.xml
Outdated
| <title>Asymmetric Encryption Schemes and Key Agreement</title> | ||
| <para/> | ||
| <table xml:id="_Ref395173040"> | ||
| <title>Assymmetric Key Schemes</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <title>Assymmetric Key Schemes</title> | |
| <title>Asymmetric Key Schemes</title> |
doc/SecurityBaseline.xml
Outdated
| <entry><para/></entry> | ||
| </row> | ||
| <row> | ||
| <entry><para>sha256WithRSAEncrpytion</para></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure how we are supposed to interpret this one. It seems to stem from the Security requirement about certificate signatures validation. I'm not sure I would put it in the asymmetric encryption section.
Also, devices don't really sign those certificates themselves right? My understanding is they are merely expected to validate the signatures seen in certification chains, so I'm not sure we need a size information there at all.
On the subject of certificate chains, I'm seeing a lot more variety in general on the internet. For example, the cert of github.com has this:
- github.com -> ecdsa-with-sha256
- intermediate -> ecdsa-with-sha384
- CA -> rsa-sha384
If we want to mandate some, I think we need to cover much more than just this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'm not quite sure on how to handle this in an elegant way as many of the algorithms have many instantiations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that it needs to be removed from the section on encryption. The of course remains the question what to do in the signature section.
| <entry/> | ||
| </row> | ||
| <row> | ||
| <entry><para>secp256r1</para></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how precise we want to be, but I'm tempted to split the Curves themselves, and the algorithm used for them.
For example, we would mandate secp256r1 (/ NIST P-256), and then we would say that all those curves need to support ECDH operations (and ECDSA).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, curves should probably have their own sub-section, especially if we want to support more than just P256. at least P384 & P521
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was to mandate a single baseline per algorithm. Having a multiple doesn't help much with EC as client and server sets must intersect otherwise no interoperability. Taking the currently lowest acceptable key size or type has the risk that it is outdated within a very short time frame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how/if we have ideas on conformance testing, but my suggestion would be to allow vendors to prove conformance to things "stronger" than baseline - which means that security minded customers could know beforehand that there is a path forward when the old algorithms become outdated even before we have had time to release a new baseline. I understand that this is straying from the title of the document which means it might be something that has to be discussed next week (which I can't join unfortunately).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Via capabilities like supported curves, hashes etc. an ONVIF compliant device can signal that it supports stronger algorithm. The problem with the baseline is that a device must support it to be compatible and is not allowed to reject e.g. a certificate with a week signature which may be tempered. The only escape-route out of this dilemma would be a device API to disable some of the algorithm so that it is up to the customer whether to operate the device in a full backward compatible mode or a more secure one. We have such an API for TLS version selection. Maybe we could add some kind of device defined security algorithm presets...
doc/SecurityBaseline.xml
Outdated
| <entry><para>FIPS 180-4</para></entry> | ||
| </row> | ||
| <row> | ||
| <entry><para>sha256WithRSAEncryption</para></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the asymmetric section, not sure I'd mention that in the hashing section since it's more of a capability thing: Supporting SHA-256 + RSA when validating certificates, feels weird to mix it with algorithms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the entry and instead point to the Asymmetric Key section?
doc/SecurityBaseline.xml
Outdated
| <para>SHA-2</para> | ||
| </entry> | ||
| <entry> | ||
| <para>>= 384 Bit</para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're specifying SHA-256 in a couple places, seems weird to say >= 384 here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also strange in my view to not have SHA-2 on the same requirement as SHA-3 unless I've missed something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHA with 256 bit seems to have a very limited lifetime. Agree shat SHA2 and SHA3 should have the same bitlength.
For me the most problematic ones are PKCS12 with the defacto standard hmacWithSHA256 and even worse PKCS8 with pbeWithSHAAnd3-KeyTripleDES-CBC which is still hanging in the security spec. My personal preference would be to drop PKCS8 but keep widely used PKCS12. To be clarified what to do about the hash size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HansBusch I further agree with your suggestion to drop "PKCS8".
| <entry><para>FIPS 202</para></entry> | ||
| </row> | ||
| <row> | ||
| <entry><para>RSASSA-PSS</para></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really a hashing algorithm either. It's SHA-256 + RSA and some padding... Not quite sure where it would fit though
| <entry><para>RFC 8017</para></entry> | ||
| </row> | ||
| <row> | ||
| <entry><para>ECDSA</para></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's also a signature algorithm, it would fit better next to wherever we put ECDH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I think we should have a signature algorithm section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to avoid lots of mini-tables I combined hashes and signatures. ECDH is for me a key agreement scheme and not a hash or I'm wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the tables might become a bit messy because reality is.
EC curves/primitives (P256) can be used to construct both ECDH (key agreement scheme) and ECDSA (signature scheme).
At the same time hashing (SHA2, SHA3 etc) is generally required for signature schemes but not for key agreement schemes
doc/SecurityBaseline.xml
Outdated
| <para>PBKDF2</para> | ||
| </entry> | ||
| <entry><para>RFC 8018</para></entry> | ||
| <entry><para>Preferred</para></entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't say preferred, it's a bit usecase dependant. It's the one to use when you have a password as input (and not some random bytes). When you get random-ish bytes as input, HKDF is usually a better choice.
We also don't seem to mention it anywhere in the actual specs today?
| </info> | ||
| <chapter> | ||
| <title>Scope</title> | ||
| <para>This document defines the security baseline for ONVIF specifications. Its content is based on state of the art technology as published by NIST or BSI. </para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: The algorithms should be standardized, state of the art technology - then they can be either in NIST, IETF, ISO, etc as usual.
Then we have the recommendations, these can be for example from BSI, NIST, etc. maybe a nit pick but it confused me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Further question is, what should be our policy for following the different recommendations? should we have the highest common denominator or the lowest common denominator as a requirement?
e.g. BSI wants RSA3072, but ANSSI is ok with RSA2048, which one do we pick?
Remember, there will be a lot of organisations issuing guidance soon: US:NIST, DE:BSI, FR:ANSSI, UK:NCSC, SE:NCSC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically the publications from NIST & Co are released on different dates. Having a common view is nice, but out of our control. Additionally there are hardware constraints and the security depends on the use cases. Suggest to strive for the securest common denominator as long as it is feasable for our industry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds very reasonable, however given the increased divergence we're currently experiencing where for example NIST, NSA & BSI all have slightly different opinions on which hash algorithms are suitable (SHA-2 vs SHA-3, sizes etc) & the inclination from some to exclude products supporting what they deem unsafe algorithms, I expect we'll have something silly happen in the 'feasible for our industry' domain.
For interoperability the lowest common denominator would be better, but then we risk products excluded. For highest common denominator we'll be pushing the limit of hardware constraints.
Current landscape from entrusts RWPQC presentation this spring:

kieran242
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HansBusch Axel and Jose have both provided comprehensive update and I could not add to those comments.
The document reads very well and I had only noticed while reviewing in the development specs portal that there was a minor update that I have made as a suggestion only.
doc/SecurityBaseline.xml
Outdated
| <title>Scope</title> | ||
| <para>This document defines the security baseline for ONVIF specifications. Its content is based on state of the art technology as published by NIST or BSI. </para> | ||
| <para>Note, that any updates to this specification require a review of implications on technical, profile and addon specifications. | ||
| Publication of updates must synchronized with ONVIF Technical and Technical Service Committee</para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Publication of updates must synchronized with ONVIF Technical and Technical Service Committee</para> | |
| Publication of updates must be synchronized with the ONVIF Technical and Technical Service Committees</para> |
Mark PBKDF2 for password usage.
|
For the following definitions that are in streaming spec for PR #555, should I create an Annex C for SRTP streaming containing the following information? Should we also completely remove the AES_CM_128_HMAC_SHA1_80 section since we expect SHA1 to be deprecated and leave the old way of doing SRTP undocumented? |
kieran242
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HansBusch re-reviewed this PR again before F2F as it will be important to finalize this document in that time frame.
| @@ -4,12 +4,12 @@ | |||
| <info> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is hard to see what has really changed in the Security.xml i.e. Added or removed as the document has been restructured.
kieran242
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just caught my eye as I was reading in the Developer rendering.
| <title>Overview</title> | ||
| <para>The content of this document is based on state of the art technology as published by the American institute NIST and the German department BSI. | ||
| Note, that any updates to this specification require a review of implications on technical, profile and addon specifications.</para> | ||
| <para>Publication of updates must synchronized with ONVIF Technical and Technical Service Committee</para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <para>Publication of updates must synchronized with ONVIF Technical and Technical Service Committee</para> | |
| <para>Publication of updates to this document must be synchronized with ONVIF Technical and Technical Service Committees</para> |
Remove MAC requirements from specification.
Added informative section on SRTP.
| <xs:attribute name="PKCS10ExternalCertificationWithRSA" type="xs:boolean"> | ||
| <xs:annotation> | ||
| <xs:documentation>Indicates support for creating PKCS#10 requests for RSA keys and uploading the certificate obtained from a CA..</xs:documentation> | ||
| <xs:documentation>Deprecated - Indicates support for creating PKCS#10 requests for RSA keys and uploading the certificate obtained from a CA..</xs:documentation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per ONVIF TLS Add On specification, "Device shall support certificate configuration as defined by each of the
"SelfSignedCertificateCreationWithRSA" and the "PKCS10ExternalCertificationWithRSA" capabilities". Now PKCS10ExternalCertificationWithRSA and SelfSignedCertificateCreationWithRSA parameters are deprecated, what is the affect of TLS Addon with these parameters deprecation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be consistent:
TLS-Addon states "Implementation of ONVIF Network Interface Specifications, version 22.06 or later". The TSC plans to update the TLS Addon to v2 as the current version indirectly requires SHA1.
kieran242
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HansBusch You have done a great job with this document. I have revied it on the developer portal as well as Github. Where possible discussed the content with others and I feel it hits the mark.
| </thead> | ||
| <tbody valign="top"> | ||
| <row> | ||
| <entry> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest adding SHA-3 at 256 Bit minimum as well, we'll need it sooner or later anyway for PQC algorithm support.
Reference: FIPS 202
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I was also for a higher security bar, but colleagues wanted to stick to AES-128, RSA-3072 with SHA-2. With that decision I expect to have another update in some years from now deprecating RSA and upgrading the others.
| </info> | ||
| <chapter> | ||
| <title>Scope</title> | ||
| <para>This document defines the security baseline for ONVIF specifications. Its content is based on state of the art technology as published by NIST or BSI. </para> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Its content is based on state of the art technology" - should be "state-of-the-art" (hyphenated when used as an adjective)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked www.thesaurus.com which also uses the term without hyphens. If this is the only issue in the document I would be happy :-).
| </chapter> | ||
| <chapter> | ||
| <title>Overview</title> | ||
| <para>The content of this document is based on state of the art technology as published by the American institute NIST and the German department BSI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The content of this document is based on state of the art technology" - should be "state-of-the-art"
* development: (94 commits) Fix for #644: Add parameter EarlyReport and clarify intermediate behavior. (#658) clarify data removal event not applicable for external storage targets (#665) ObjectType enumeration extended with Fire and Smoke Add clarification that curve names are defined by IANA (#652) Add CertificateThumbprintAlgorithm field in PSSH box & define acceptable values Video/cloudfwupgrade merge (#607) Multicast Audio decoder configuration - Rebased (#631) corrected TargetFrameRate to FrameRateLimit (#660) Fix DataSize field length in Asymmetric key system to fix Mp4 spec (#666) Security-Baseline (#571) Remove duplicate row in Security.xml docbook validation error Explicitly reference table and figures Remove square brackets around references Sort normative references in alphanumeric order Correct RFC 8656 link in normative reference Media Signing: Added requirement on SEI order Remove EncodingInterval description Cleanup and reword WebRTC session expiration handling (#639) auto adjust clarification added ...


Move security algorithm requirements from Security-Specification to new document Security-Baseline.
This activity serves as base for future addons.