-
Notifications
You must be signed in to change notification settings - Fork 880
OpenVPN: Allow Nonce Validation to be disabled when using OCSP #9241
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
base: master
Are you sure you want to change the base?
Conversation
well, technically, that's not completely true, the code originates from 318a1ae ;) I don't mind that much adding an advanced option, although I do find it a bit odd they don't implement nonce on their end as apparently it's quite common to require them (hashicorp/vault#29364) |
| 'digest' => (string)$node->auth, | ||
| 'description' => (string)$node->description, | ||
| 'use_ocsp' => !$node->use_ocsp->isEmpty(), | ||
| 'ocsp_nonce' => !$node->ocsp_no_nonce->isEmpty(), |
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 think ocsp_no_nonce is indeed the better name in this case, disabled by default, the rest of the code just doesn't seem to reflect that (e.g. ocsp_nonce).
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.
Well, that's the logic I used initially (and an artifact I forgot to change before commit), but I found it a little confusing to have a negation.
anyway, now it's ocsp_no_nonce :)
AdSchellevis
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.
@ybayart I've added a couple of suggestions for the labels and help texts, the code itself looks ok.
| <style>role role_server</style> | ||
| <advanced>true</advanced> | ||
| <type>checkbox</type> | ||
| <help>When your OCSP Responder does not support Nonce Validation, a warning is returned and validation fails.</help> |
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.
| <help>When your OCSP Responder does not support Nonce Validation, a warning is returned and validation fails.</help> | |
| <help>When your OCSP Responder does not support the Nonce Extension, validation will fail. This option disables Nonces at the cost of loosing protection against replay attacks</help> |
| </field> | ||
| <field> | ||
| <id>instance.ocsp_no_nonce</id> | ||
| <label>Disable OCSP Nonce validation</label> |
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.
| <label>Disable OCSP Nonce validation</label> | |
| <label>Disable OCSP Nonce extension</label> |
Hi,
Here is an implementation of Nonce validation disabling, initially proposed by the author of OCSP validation: #7082 (comment)
Validation is done this way, by checking if the first line is an OK response (
$outputcorresponds to the lines)core/src/opnsense/mvc/app/library/OPNsense/Trust/Store.php
Line 646 in 5eddbce
In cases where the OCSP Responder does not support Nonce, a response of this type is returned
By changing the
-nonceparameter to-no_nonce, we get a request that can be parsed correctlyI use the OCSP Responder integrated into Vault community (Hashicorp).