Add cert auth module. #78
Conversation
| //! CA certificates are associated with a role; role names and CRL names are normalized | ||
| //! to lower-case. | ||
| //! | ||
| //! Please note that to use this auth method, `tls_disable` and `tls_disable_client_certs` |
There was a problem hiding this comment.
This is a little confusing. If there is no 'cert auth' configured, can RustyVault validate a client certificate in a TLS connection?
There was a problem hiding this comment.
This is a little confusing. If there is no 'cert auth' configured, can RustyVault validate a client certificate in a TLS connection?
It has been fixed in #79.
|
|
||
| impl Module for CertModule { | ||
| fn name(&self) -> String { | ||
| return self.name.clone(); |
There was a problem hiding this comment.
It seems CertModule.name is always a constant string through the whole lifetime of a RustyVault instance, then why don't return a referent &string instead of cloning a new string?
There was a problem hiding this comment.
It seems CertModule.name is always a constant string through the whole lifetime of a RustyVault instance, then why don't return a referent &string instead of cloning a new string?
This field and function was originally used for debugging or logging. Using references with lifecycles is relatively complex. For simplicity, String was used. Later, let's see whether this field and function should be removed or optimized.
| let cert_backend_ref1 = Arc::clone(&self.inner); | ||
| let cert_backend_ref2 = Arc::clone(&self.inner); | ||
| let cert_backend_ref3 = Arc::clone(&self.inner); |
There was a problem hiding this comment.
It seems you can just use Arc::clone(...) in the places where they need it, thus the 3 local variables can be avoided.
There was a problem hiding this comment.
It seems you can just use
Arc::clone(...)in the places where they need it, thus the 3 local variables can be avoided.
This won't work because these variables are used by calling their member functions rather than using the variables themselves.
| that are allowed to authenticate. | ||
|
|
||
| Deleting a certificate will not revoke auth for prior authenticated connections. | ||
| To do this, do a revoke on "login". If you don'log need to revoke login immediately, |
There was a problem hiding this comment.
Nit:
don'log->don't
fixed
| let data = cert_entry_data.as_object_mut().unwrap(); | ||
| //TODO | ||
|
|
||
| Ok(Some(Response::data_response(Some(data.clone())))) |
There was a problem hiding this comment.
If data is not mutable, then why a clone() is needed?
There was a problem hiding this comment.
If
datais not mutable, then why aclone()is needed?
data is a reference and needs to clone an object.
| Ok(verified_chains) | ||
| } | ||
|
|
||
| fn matches_constraints( |
There was a problem hiding this comment.
why there are so many constraints to be matched?
There was a problem hiding this comment.
why there are so many constraints to be matched?
Vault can configure these matching items. In order to be compatible with Vault...
| && self.matches_names(client_cert, config) | ||
| && self.matches_common_name(client_cert, config) | ||
| && self.matches_dns_sans(client_cert, config) | ||
| && self.matches_email_sans(client_cert, config) | ||
| && self.matches_uri_sans(client_cert, config) | ||
| && self.matches_organizational_units(client_cert, config) | ||
| && self.matches_certificate_extensions(client_cert, config); |
There was a problem hiding this comment.
Does underlying cryptography library like OpenSSL or Tongsuo already provide such kind of functions? If so, we don't need to re-implement them again.
There was a problem hiding this comment.
Does underlying cryptography library like OpenSSL or Tongsuo already provide such kind of functions? If so, we don't need to re-implement them again.
There are many matching items. The underlying OpenSSL/Tongsuo does not have a corresponding interface, so regular expression matching is used.
No description provided.