Skip to content

Allow an exact date/time range for validity when signing a CSR - #20330

Open
StephenWall wants to merge 4 commits into
php:masterfrom
StephenWall:validity
Open

Allow an exact date/time range for validity when signing a CSR#20330
StephenWall wants to merge 4 commits into
php:masterfrom
StephenWall:validity

Conversation

@StephenWall

Copy link
Copy Markdown
Contributor

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).

Comment thread ext/openssl/openssl.c Outdated
@StephenWall
StephenWall force-pushed the validity branch 2 times, most recently from d38c0c8 to 0cbdc1c Compare October 30, 2025 21:17

@bukka bukka left a comment

Copy link
Copy Markdown
Member

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

Comment thread ext/openssl/openssl.stub.php Outdated
* @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 {}

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Contributor Author

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?

Copy link
Copy Markdown
Member

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.

@StephenWall

StephenWall commented Dec 9, 2025

Copy link
Copy Markdown
Contributor Author

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):

Not really sure what you're getting at here. The new format for the parameter is array( $notBefore, $notAfter ), each of which can be a unix timestamp as an integer or a string, or an ASN.1 timestamp, which is the [YY]YYMMDDHHMMSSZ format, and must be a string because of the 'Z' at the end. I can drop the unix timestamp as a string, if that's what you mean, but I think the ASN.1 format is useful to have.
If you just want number of days string from right now, that still works as currently. The new format allows precise starting and ending times (for example, starting 2 days from now at midnight, valid to the exact end of the year).

@bukka

bukka commented Dec 9, 2025

Copy link
Copy Markdown
Member

It's more that name $days suggest number of dates and not date in string so I would just accept only int (or array of ints).

@StephenWall

StephenWall commented Dec 10, 2025

Copy link
Copy Markdown
Contributor Author

Making the notBefore and notAfter a count of days lacks the precision of using a timestamp, and is inconsistent with the use of those values within OpenSSL itself. I'd really like to find a way to incorporate that precision without breaking existing usage. It's unfortunate in this instance that PHP does not support overloading functions.
One possibility (though a bit ugly in my opinion) is to replace the positional $days with the $validity as implemented, and to add an optional $days parameter at the end of the parameter list, to preserve the named parameter functionality.
Another is to add $validity to the end as an optional parameter, and ignore $days if $validity is provided.
I think I am going to have to learn the RFC process for this one... I think I read something about needing a code to be able to register to create RFCs? Is that correct?

@bukka

bukka commented Jan 13, 2026

Copy link
Copy Markdown
Member

One possibility (though a bit ugly in my opinion) is to replace the positional $days with the $validity as implemented, and to add an optional $days parameter at the end of the parameter list, to preserve the named parameter functionality.

Yeah this might not be nice and quite confusing for users

Another is to add $validity to the end as an optional parameter, and ignore $days if $validity is provided.

I think this is the best solution here. You don't need RFC for that.

I think I am going to have to learn the RFC process for this one... I think I read something about needing a code to be able to register to create RFCs? Is that correct?

Yes you would need to request wiki account first - just register and ping internals so someone approves it. But you need RFC just if you plan to rename days to validity. This goes against recommendation for minor version so you should state why this should get exception. Personally I'm not sure such change will going to pass but not sure.

… 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).
@StephenWall

Copy link
Copy Markdown
Contributor Author

I've re-written this to preserve the days parameter, and add a new optional validity parameter at the end, which will cause days to be ignored if it is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants