-
Notifications
You must be signed in to change notification settings - Fork 8k
Allow an exact date/time range for validity when signing a CSR #20330
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
… a CSR This changes the `days` parameter of `openssl_csr_sign()` to a `validity` parameter, which can be either an integer specifying the number of days the certificate is to be valid for (compatible with current usage), or it can be an array of two integer or string values, representing the notBefore and notAfter times to use for the certificate. If they are integers or numeric strings, they are to be a time_t value. If they are non-numeric strings, they are to be an ASN.1 timestamp (YYMMDDHHMMSSZ or YYYYMMDDHHMMSSZ).
bukka
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.
So unless you want to do RFC proposing this BC break, I would suggest to go with additional parameter. We could maybe just go with something like $not_before_num_days or something like that so we don't have 2 params to change the same thing.
Also users can just use this to get number of days so the string form is not really that necessary (might be actually slightly confusing as that format is limited):
(new DateTime('now'))->diff(new DateTime($string_date))->days| * @param OpenSSLAsymmetricKey|OpenSSLCertificate|array|string $private_key | ||
| */ | ||
| function openssl_csr_sign(OpenSSLCertificateSigningRequest|string $csr, OpenSSLCertificate|string|null $ca_certificate, #[\SensitiveParameter] $private_key, int $days, ?array $options = null, int $serial = 0, ?string $serial_hex = null): OpenSSLCertificate|false {} | ||
| function openssl_csr_sign(OpenSSLCertificateSigningRequest|string $csr, OpenSSLCertificate|string|null $ca_certificate, #[\SensitiveParameter] $private_key, int|array $validity, ?array $options = null, int $serial = 0, ?string $serial_hex = null): OpenSSLCertificate|false {} |
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 is a BC break because of named params. Renaming param is not acceptable without RFC.
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.
Would leaving the parameter named "$days" but accepting either an integer (existing usage) or an array (new usage) alleviate the breakage satisfactorily?
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.
Ah yeah that would be fine and it's actually better than extra param.
Not really sure what you're getting at here. The new format for the parameter is |
|
It's more that name |
|
Making the |
This changes the
daysparameter ofopenssl_csr_sign()to avalidityparameter, which can be either an integer specifying the number of days the certificate is to be valid for (compatible with current usage), or it can be an array of two integer or string values, representing the notBefore and notAfter times to use for the certificate. If they are integers or numeric strings, they are to be a time_t value. If they are non-numeric strings, they are to be an ASN.1 timestamp (YYMMDDHHMMSSZ or YYYYMMDDHHMMSSZ).