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.

Sign up for GitHub

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-3789 Unify DN Representation #4042

Merged
merged 11 commits into from
Jun 17, 2019
Merged

PAYARA-3789 Unify DN Representation #4042

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8b4a2d1
PAYARA-3789 Unify DN output and allow EMAILADDRESS
Pandrex247 Jun 11, 2019
00f2eca
PAYARA-3789 Don't use deprecated methods
Pandrex247 Jun 11, 2019
5f8551c
PAYARA-3789 Add more robust name generation
Pandrex247 Jun 12, 2019
a48c875
Remove unused imports
Pandrex247 Jun 12, 2019
8ebfbec
PAYARA-3789 Make sure all usages are in the same format
Pandrex247 Jun 12, 2019
c1ce43e
PAYARA-3789 Another missed getName
Pandrex247 Jun 12, 2019
143eb93
PAYARA-3789 Don't use variables from class
Pandrex247 Jun 12, 2019
93af14b
PAYARA-3789 Move OIDs into constants class
Pandrex247 Jun 13, 2019
1f96686
PAYARA-3789 Correct DN attribute name
Pandrex247 Jun 13, 2019
9e5d6ae
Merge branch 'master' of https://github.com/payara/Payara into PAYARA…
Pandrex247 Jun 14, 2019
9611222
PAYARA-3789 Variable formatting
Pandrex247 Jun 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...urity/core-ee/src/main/java/com/sun/enterprise/security/web/integration/WebPrincipal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,14 @@
import java.security.cert.X509Certificate;
import java.util.Arrays;

import com.sun.enterprise.security.auth.realm.certificate.CertificateRealm;
import org.glassfish.security.common.PrincipalImpl;

import com.sun.enterprise.security.SecurityContext;
import com.sun.enterprise.security.SecurityContextProxy;

import javax.security.auth.x500.X500Principal;

public class WebPrincipal extends PrincipalImpl implements SecurityContextProxy {

private static final long serialVersionUID = 1L;
Expand Down Expand Up @@ -173,7 +176,7 @@ private static String getPrincipalName(X509Certificate[] certificates, SecurityC
// Use the full DN name from the certificates. This should normally be the same as
// context.getCallerPrincipal(), but a realm could have decided to map the name in which
// case they will be different.
return certificates[0].getSubjectX500Principal().getName();
return certificates[0].getSubjectX500Principal().getName(X500Principal.RFC2253, CertificateRealm.oidMap);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using getName(X500Principal.RFC2253, CertificateRealm.oidMap) several times. Make a utility method so that X500Principal representation is only in 1 place for future?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's really worth it? Unless I'm misunderstanding you we'd just be replacing certificates[0].getSubjectX500Principal().getName(X500Principal.RFC2253, CertificateRealm.oidMap) with CertificateRealm.getX500PrincipalSubjectName(certificates[0]).

It seems fragile to me to store the name of the certificate somewhere outside of the actual certificate itself.

Also, it isn't strictly necessary for me to put the full X500Principal.RFC2253 stuff everywhere, it's me just being safe. From my testing as long as you get the name in this manner from the CertificateRealm it propagates outward such that simply doing getName() will still have the OIDs translated.

}

}
7 changes: 5 additions & 2 deletions ....security/src/main/java/com/sun/enterprise/iiop/security/SecServerRequestInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@

import javax.security.auth.Subject;

import com.sun.enterprise.security.auth.realm.certificate.CertificateRealm;
import org.glassfish.enterprise.iiop.api.GlassFishORBHelper;
import org.omg.CORBA.Any;
import org.omg.CORBA.BAD_PARAM;
Expand Down Expand Up @@ -342,7 +343,8 @@ private void createIdCred(SecurityContext securityContext, IdentityToken identit
for (int i = 0; i < certchain.length; i++) {
certchain[i] = new X509CertImpl(derval[i]);
if (logger.isLoggable(FINE)) {
logger.log(FINE, " " + certchain[i].getSubjectDN().getName());
logger.log(FINE, " " + certchain[i].getSubjectX500Principal()
.getName(X500Principal.RFC2253, CertificateRealm.oidMap));
}
}
if (logger.isLoggable(FINE)) {
Expand All @@ -353,7 +355,8 @@ private void createIdCred(SecurityContext securityContext, IdentityToken identit
* "dummy".
*
*/
X509CertificateCredential cred = new X509CertificateCredential(certchain, certchain[0].getSubjectDN().getName(),
X509CertificateCredential cred = new X509CertificateCredential(certchain,
certchain[0].getSubjectX500Principal().getName(X500Principal.RFC2253, CertificateRealm.oidMap),
"default");
if (logger.isLoggable(FINE)) {
logger.log(FINE, "Adding X509CertificateCredential to subject's PublicCredentials");
Expand Down
2 changes: 1 addition & 1 deletion appserver/security/webintegration/src/main/java/com/sun/web/security/RealmAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ public void setCurrentSecurityContext(Principal principal) {

private Subject createSubjectWithCerts(X509Certificate[] certificates) {
Subject subject = new Subject();

// Specifically not using getName() as we aren't interested with the name here, we're interested in the X500Principal itself
subject.getPublicCredentials().add(certificates[0].getSubjectX500Principal());
subject.getPublicCredentials().add(asList(certificates));

Expand Down
8 changes: 6 additions & 2 deletions ...re/src/main/java/org/glassfish/admin/amx/util/stringifier/X509CertificateStringifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@

package org.glassfish.admin.amx.util.stringifier;

import com.sun.enterprise.security.auth.realm.certificate.CertificateRealm;
import org.glassfish.admin.amx.util.StringUtil;

import javax.security.auth.x500.X500Principal;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.cert.X509Certificate;
Expand Down Expand Up @@ -91,8 +93,10 @@ public final class X509CertificateStringifier implements Stringifier
final StringBuilder buf = new StringBuilder();
final String NL = "\n";

buf.append("Issuer: ").append(cert.getIssuerDN().getName()).append(NL);
buf.append("Issued to: ").append(cert.getSubjectDN().getName()).append(NL);
buf.append("Issuer: ").append(cert.getIssuerX500Principal()
.getName(X500Principal.RFC2253, CertificateRealm.oidMap)).append(NL);
buf.append("Issued to: ").append(cert.getSubjectX500Principal()
.getName(X500Principal.RFC2253, CertificateRealm.oidMap)).append(NL);
buf.append("Version: ").append(cert.getVersion()).append(NL);
buf.append("Not valid before: ").append(cert.getNotBefore()).append(NL);
buf.append("Not valid after: ").append(cert.getNotAfter()).append(NL);
Expand Down
3 changes: 2 additions & 1 deletion nucleus/security/core/src/main/java/com/sun/enterprise/security/auth/JaspicToJaasBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import static java.util.logging.Level.FINE;
import static java.util.logging.Level.INFO;

import java.io.IOException;
import java.util.Enumeration;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -138,7 +139,7 @@ public static Subject jaasX500Login(Subject subject, X500Principal x500Principal

String callerPrincipalName = "";
try {
callerPrincipalName = x500Principal.getName(X500Principal.RFC1779);
callerPrincipalName = x500Principal.getName(X500Principal.RFC2253, CertificateRealm.oidMap);

privileged(() -> validSubject.getPublicCredentials().add(x500Principal));

Expand Down
4 changes: 2 additions & 2 deletions ...s/security/core/src/main/java/com/sun/enterprise/security/auth/WebAndEjbToJaasBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ public static void doX500Login(Subject subject, String appModuleID) {
// Should never happen
return;
}
user = x500principal.getName();

user = x500principal.getName(X500Principal.RFC2253, CertificateRealm.oidMap);

// In the RI-inherited implementation this directly creates
// some credentials and sets the security context.
Expand Down
5 changes: 4 additions & 1 deletion ...re/src/main/java/com/sun/enterprise/security/auth/login/ClientCertificateLoginModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
import javax.security.auth.callback.UnsupportedCallbackException;
import javax.security.auth.login.LoginException;
import javax.security.auth.spi.LoginModule;
import javax.security.auth.x500.X500Principal;

import com.sun.enterprise.security.auth.realm.certificate.CertificateRealm;
import org.glassfish.internal.api.Globals;
import org.glassfish.security.common.PrincipalImpl;

Expand Down Expand Up @@ -160,7 +162,8 @@ public boolean login() throws LoginException {
Enumeration<String> aliases = keyStore.aliases();
for (int i = 0; i < keyStore.size(); i++) {
aliasNames[i] = aliases.nextElement();
certificateNames[i] = ((X509Certificate) keyStore.getCertificate(aliasNames[i])).getSubjectDN().getName();
certificateNames[i] = ((X509Certificate) keyStore.getCertificate(aliasNames[i]))
.getSubjectX500Principal().getName(X500Principal.RFC2253, CertificateRealm.oidMap);
}

Callback[] callbacks = new Callback[] {createChoiceCallback(certificateNames)};
Expand Down
33 changes: 30 additions & 3 deletions ...re/src/main/java/com/sun/enterprise/security/auth/realm/certificate/CertificateRealm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@
import java.security.Principal;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;

Expand Down Expand Up @@ -101,7 +103,32 @@ public final class CertificateRealm extends BaseRealm {

// Descriptive string of the authentication type of this realm.
public static final String AUTH_TYPE = "certificate";

public static final Map<String, String> oidMap;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OID_MAP as name?

static {
Map<String, String> oidMapInitialiser = new HashMap<>();
oidMapInitialiser.put("0.9.2342.19200300.100.1.1", "UID");
oidMapInitialiser.put("0.9.2342.19200300.100.1.25", "DC");
oidMapInitialiser.put("1.2.840.113549.1.9.1", "EMAIL");
oidMapInitialiser.put("1.2.840.113549.1.9.1", "EMAILADDRESS");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate, so always EMAILADDRESS used and never EMAIL.

oidMapInitialiser.put("1.3.6.1.4.1.42.2.11.2.1", "IP");
oidMapInitialiser.put("2.5.4.3", "CN");
oidMapInitialiser.put("2.5.4.4", "SURNAME");
oidMapInitialiser.put("2.5.4.5", "SERIALNUMBER");
oidMapInitialiser.put("2.5.4.6", "C");
oidMapInitialiser.put("2.5.4.7", "L");
oidMapInitialiser.put("2.5.4.8", "S");
oidMapInitialiser.put("2.5.4.8", "ST");
oidMapInitialiser.put("2.5.4.9", "STREET");
oidMapInitialiser.put("2.5.4.10", "O");
oidMapInitialiser.put("2.5.4.11", "OU");
oidMapInitialiser.put("2.5.4.12", "T");
oidMapInitialiser.put("2.5.4.42", "GIVENNAME");
oidMapInitialiser.put("2.5.4.43", "INITIALS");
oidMapInitialiser.put("2.5.4.44", "GENERATION");
oidMapInitialiser.put("2.5.4.46", "DNQUALIFIER");
oidMap = Collections.unmodifiableMap(oidMapInitialiser);
}

private List<String> defaultGroups = new LinkedList<>();

/**
Expand Down Expand Up @@ -175,8 +202,8 @@ public Enumeration<String> getGroupNames(String username) throws NoSuchUserExcep
public String authenticate(Subject subject, X500Principal callerPrincipal) {
// It is important to use X500Principal.getName() as that will
// return the LDAP name in RFC2253
String callerPrincipalName = callerPrincipal.getName();
String callerPrincipalName = callerPrincipal.getName(X500Principal.RFC2253, oidMap);

// Checks if the property for using common name is set
if (Boolean.valueOf(getProperty("useCommonName"))) {
callerPrincipalName = extractCN(callerPrincipalName);
Expand Down
1 change: 1 addition & 0 deletions nucleus/security/core/src/main/java/com/sun/enterprise/security/ssl/JarSigner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ public void signJar(File input, ZipOutputStream zout, String alias, final Attrib
sig.update(sigFileContent);

// Create PKCS7 block
// Can't use X500Principal here as SignerInfo requires X500Name
PKCS7 pkcs7 = new PKCS7(new AlgorithmId[] { AlgorithmId.get(digestAlgorithm) }, new ContentInfo(sigFileContent), certChain,
new SignerInfo[] { new SignerInfo((X500Name) certChain[0].getIssuerDN(), certChain[0].getSerialNumber(),
AlgorithmId.get(digestAlgorithm), AlgorithmId.get(keyAlgorithm), sig.sign()) });
Expand Down