Skip to content
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

CSR creation with CN only #15

Closed
dol opened this issue Mar 25, 2016 · 5 comments
Closed

CSR creation with CN only #15

dol opened this issue Mar 25, 2016 · 5 comments

Comments

@dol
Copy link

dol commented Mar 25, 2016

For the creation of an CSR the dn parameter of openssl_csr_new only needs the definition of 'CN' to work with domain validated certificate.

https://github.com/kelunik/acme/blame/dd01ee543932b8ca51cb2b9d3fe2efd097fbba66/lib/AcmeService.php#L363 could be reduced to just:

$csr = openssl_csr_new([
            "CN" => reset($domains),
        ], $privateKey, [
            "digest_alg" => "sha256",
            "req_extensions" => "v3_req",
            "config" => $tempFile,
        ]);
@kelunik
Copy link
Owner

kelunik commented Mar 25, 2016

You're right, it could be reduced. The reason I put these values there is that otherwise it would be prefilled with strange values, that might not be the case with the always passed config now anymore.

Are there any known issues when adding additional properties?

@kelunik
Copy link
Owner

kelunik commented Mar 26, 2016

I think I will move the CSR generation out of that method anyway with 0.4.0

@dol
Copy link
Author

dol commented Mar 26, 2016

Are there any known issues when adding additional properties?

The only issue I see is that to much information sent to the the ACME server. The current spec states the following:

The CSR encodes the client’s requests with regard to the content of the certificate to be issued. The CSR MUST indicate the requested identifiers, either in the commonName portion of the requested subject name, or in an extensionRequest attribute [RFC2985] requesting a subjectAltName extension.

If you extract the CSR generation a little hint. The CSR generation could also be done with a environment variable (kind of a strange thing built into the core of OpenSSL, but avoids the need of generating a temporary file). See https://gist.github.com/dol/e0b7f084e2e7158efc87 as an example.
An other hint. When using heredoc notation, I prefer the nowdoc notation to prevent possible inclusion of PHP variables. Most of the time heredoc ist just fine. It's only a security precaution.

@kelunik
Copy link
Owner

kelunik commented Mar 26, 2016

The current spec states the following

It doesn't say anything about additional fields.

See https://gist.github.com/dol/e0b7f084e2e7158efc87 as an example.

That's really strange, don't know which method I prefer.

An other hint. When using heredoc notation, I prefer the nowdoc notation to prevent possible inclusion of PHP variables. Most of the time heredoc ist just fine. It's only a security precaution.

Don't know which editor / IDE you use, but mine highlights variables in there, so accidental inclusion isn't that likely to happen. Additionally, it would also throw a notice because of undefined variables probably. :-)

@dol
Copy link
Author

dol commented Mar 26, 2016

It's a matter of taste. Can't argue with that. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants