Warn in cert-request if CSR doesn't contain SAN #781
Conversation
|
Hi everyone, after a long long day, I did a great job deleting the branch from PR #773, then Github closed it. Sorry about that :( Notifying people that were following that thread: @stlaz @MartinBasti @frasertweedale @HonzaCholasta @pvoborni |
ipaserver/plugins/cert.py
Outdated
| has_dns_in_san_ext = True | ||
|
|
||
| if not ext_san or not has_dns_in_san_ext: | ||
| print('Warning: The SAN extension ' |
There was a problem hiding this comment.
I think we should say "Subject Alternative Name" instead of the abbreviation "SAN".
ipaserver/plugins/cert.py
Outdated
| has_dns_in_san_ext = True | ||
|
|
||
| if not ext_san or not has_dns_in_san_ext: | ||
| print('Warning: The SAN extension ' |
There was a problem hiding this comment.
Also recalling the review comments from #773 (review) about sending the warning back to the client, and the print is just placeholder while final version
of patch is worked out.
ipaserver/plugins/cert.py
Outdated
|
|
||
| if not ext_san or not has_dns_in_san_ext: | ||
| print('Warning: The SAN extension ' | ||
| 'should be provided. Please, check the RFC 2818.') |
There was a problem hiding this comment.
I don't think we need to reference the RFC.
Ideally we should suggest to the user what to do, but because we don't know what tool created the CSR
(certmonger, certutil, openssl, ...?) it is a bit hard to give concrete steps to user about how to solve their problem.
I would say something like: "Certificate request for a {host,service} certificate did not contain Subject Alternative Name, which is required for many use cases."
This warning will be suprious if the profile uses the new CommonNameToSANDefault component (assuming the CN looks like a DNS name). I have a PR to add this component to caIPAserviceCert (the default profile) for new installs only.
I'm just not sure if this won't create more support load than it avoids. It is hard to guess what will happen.
There was a problem hiding this comment.
I changed the message, please, checks if it looks good.
I'm just not sure if this won't create more support load than it avoids.
IMHO I think it more useful to show the message to the user instead of not showing it. As you said, if the user does not provide SAN in certificate this could lead to fails in the future, since it is required for many use cases.
Adding a warning in the cert-request command when the request certificate doesn't contain a Subject Alternative Name according to RFC 2818. https://pagure.io/freeipa/issue/6663
|
@frasertweedale I did the changes you proposed, however I'm not sure if there is an agreement in the team that this ticket should be done. I'm saying that based on the comments on previous PR #773 |
|
@felipevolpone I've put the ticket up for re-triage. Let's see what comes out of that. |
felipevolpone
changed the title
|
Closing it. It was decided to do not have this feature/warning. |
The code is not "production-ready", however, I would like to know if I'm on the right path.
AFAIK we should check if the SAN extension is provided and if it has DNSName info.
Fix: https://pagure.io/freeipa/issue/6663