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

[PAYARA-3417] Flag to use CN as principal-name in glassfish-web.xml #3900

Merged
merged 6 commits into from Apr 26, 2019

Conversation

Projects
None yet
4 participants
@cubastanley
Copy link
Contributor

commented Apr 16, 2019

Implemented a new feature to enable users to optionally use the value of common name as the principal name in an application's glassfish-web.xml for role mapping as opposed to a full string version of the client certificate - will need documentation on passing

@cubastanley cubastanley requested review from arjantijms, OndroMih and AlanRoth Apr 16, 2019

@AlanRoth
Copy link
Contributor

left a comment

Update license header from 2018 to 2018-2019 :^)

@cubastanley cubastanley requested a review from AlanRoth Apr 16, 2019

@cubastanley cubastanley requested a review from Pandrex247 Apr 16, 2019

@mulderbaba mulderbaba added this to the 5.192 milestone Apr 16, 2019

@@ -162,11 +168,27 @@ public String getAuthType() {
* @param principal The Principal object from the user certificate.
*
*/
public void authenticate(Subject subject, Principal principal) {
public String authenticate(Subject subject, Principal principal) {

This comment has been minimized.

Copy link
@arjantijms

arjantijms Apr 24, 2019

Member

At a glance, just wondering if this actually works? The common name is returned here, but not installed in the security context. The returned name is only used for auditing. Am I missing something?

This comment has been minimized.

Copy link
@cubastanley

cubastanley Apr 24, 2019

Author Contributor

It does work against the written tests in ee7 and against the effective reproducer that I've been working with. It's a change to meet a request made by Ondrej for the sakes of consistency across the authentication logic

This comment has been minimized.

Copy link
@arjantijms

arjantijms Apr 24, 2019

Member

@cubastanley I did overlook something indeed, and that's that name is used as the first argument of the security context, my bad, sorry. So that makes it work wrt role mapping. What I was actually looking at however was setting the principal in the Subject to one with the CN as the name". This is pending clarification of the original issue.

@arjantijms

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

@cubastanley Don't forget that com.sun.enterprise.security.auth.JaspicToJaasBridge also uses CertificateRealm.

@arjantijms

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

After looking at this a bit better, it seems that the principal name is now inconsistent in the system. Sub-systems that look at the SecurityContext (such as EJB) see the cn only name, but sub-systems that look at the WebPrincipal (such as Servlets) see the full name.

To reproduce this I added this EJB to the existing ee7 client-cert samples test:

@Stateless
public class SecureEJB {

    @Resource
    SessionContext sessionContext;

    public String getEJBPrincipal() {
        return sessionContext.getCallerPrincipal().getName();
    } 
}

and updated the existing Servlet in that test to:

@EJB
SecureEJB secureEJB;

@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {

    response.getWriter().println("\nWeb principal " + request.getUserPrincipal() + " in role g1:" + request.isUserInRole("g1"));

    response.getWriter().println("EJB principal " + secureEJB.getEJBPrincipal());
}

The output is:

INFO: 

Web principal C=UK, ST=lak, L=zak, O=kaz, OU=bar, CN=lfoo in role g1:true
EJB principal lfoo

This is obviously not correct. You should probably look at this bit of code:

com.sun.web.security.RealmAdapter.authenticate:

@Override
    public Principal authenticate(X509Certificate certs[]) {
        if (authenticate(null, null, certs, null)) {
            return new WebPrincipal(certs, SecurityContext.getCurrent());
        }
        return null;
    }

And then specifically in the constructor:

  public WebPrincipal(X509Certificate[] certs, SecurityContext context) {
        super(certs[0].getSubjectDN().getName());
        this.certs = certs;
        this.useCertificate = true;
        this.securityContext = context;
    }

Maybe adding another constructor with a boolean that instructs it to use the name from the security context instead will fix the issue here.

@arjantijms
Copy link
Member

left a comment

The cn only name should be put into the WebPrincipal as well.

@arjantijms

This comment has been minimized.

Copy link
Member

commented Apr 26, 2019

Jenkins test

@arjantijms arjantijms merged commit d1be025 into payara:master Apr 26, 2019

59 checks passed

Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/javaee-api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.