Skip to content

Commit 2f83303

Browse files
enricovianelloenricovianello
committed
Fix LSC parser
Fix multi-chain LSC files by reading only the first chain
1 parent 69190c8 commit 2f83303

4 files changed

Lines changed: 103 additions & 101 deletions

File tree

‎src/main/java/org/italiangrid/voms/store/impl/DefaultLSCFileParser.java‎

Lines changed: 36 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -35,104 +35,69 @@
3535
*/
3636
public class DefaultLSCFileParser implements LSCFileParser {
3737

38-
public static final String EMPTY_LINE_REGEX = "(?m)^\\s*?$";
3938
public static final String MALFORMED_LSC_FILE_ERROR_TEMPLATE = "LSC file parsing error: Malformed LSC file (vo=%s, host=%s): %s";
4039

4140
private void checkFileExistanceAndReadabilty(File f) {
4241

43-
if (!f.exists())
42+
if (!f.exists()) {
4443
throw new VOMSError("LSC file does not exist: " + f.getAbsolutePath());
45-
46-
if (!f.canRead())
44+
}
45+
if (!f.canRead()) {
4746
throw new VOMSError("LSC file is not readable: " + f.getAbsolutePath());
48-
49-
}
50-
51-
public LSCFile parse(String vo, String hostname, String filename) {
52-
53-
LSCFile lsc = null;
54-
55-
try {
56-
57-
File f = new File(filename);
58-
59-
checkFileExistanceAndReadabilty(f);
60-
61-
lsc = parse(vo, hostname, new FileInputStream(f));
62-
63-
lsc.setFilename(filename);
64-
65-
} catch (IOException e) {
66-
throw new VOMSError("LSC file parsing error: " + e.getMessage(), e);
6747
}
68-
69-
return lsc;
7048
}
7149

7250
public synchronized LSCFile parse(String vo, String hostname, InputStream is) {
7351

52+
final String NEXT_CHAIN = "------NEXT CHAIN------";
53+
7454
LSCFile lsc = new LSCFile();
7555

7656
lsc.setHostname(hostname);
7757
lsc.setVo(vo);
7858

79-
try {
80-
81-
BufferedReader lscReader = new BufferedReader(new InputStreamReader(is));
82-
83-
String line = null;
59+
try (BufferedReader reader = new BufferedReader(new InputStreamReader(is))) {
8460
List<String> certificateChainDescription = new ArrayList<String>();
85-
86-
do {
87-
line = lscReader.readLine();
88-
89-
// This is EOF
90-
if (line == null)
91-
break;
92-
93-
// Ignore comments
94-
if (line.startsWith("#"))
61+
String line;
62+
while ((line = reader.readLine()) != null) {
63+
if (line.trim().startsWith("#") || line.isBlank()) {
9564
continue;
96-
97-
// Ignore ---NEXT CHAIN---
98-
if (line.startsWith("-"))
99-
continue;
100-
101-
// Ignore empty lines
102-
if (line.matches(EMPTY_LINE_REGEX))
103-
continue;
104-
105-
if (line.startsWith("/"))
65+
}
66+
if (line.equalsIgnoreCase(NEXT_CHAIN)) {
67+
/* This parser doesn't support multi-chain LSC files */
68+
break;
69+
}
70+
if (line.startsWith("/")) {
10671
certificateChainDescription.add(line);
107-
108-
} while (true);
109-
110-
lscReader.close();
111-
112-
if (certificateChainDescription.size() % 2 != 0) {
113-
String errorMessage = String.format(MALFORMED_LSC_FILE_ERROR_TEMPLATE,
114-
vo, hostname, "Odd number of distinguished name entries.");
115-
116-
throw new VOMSError(errorMessage);
117-
72+
}
11873
}
119-
120-
if (certificateChainDescription.size() == 0) {
121-
String errorMessage = String.format(MALFORMED_LSC_FILE_ERROR_TEMPLATE,
122-
vo, hostname, "No distinguished name entries found.");
123-
124-
throw new VOMSError(errorMessage);
125-
}
126-
74+
validateChain(certificateChainDescription, vo, hostname);
12775
lsc.setCertificateChainDescription(certificateChainDescription);
128-
12976
} catch (IOException e) {
13077
throw new VOMSError("LSC file parsing error: " + e.getMessage(), e);
13178
}
132-
79+
if (lsc.getCertificateChainDescription().isEmpty()) {
80+
String errorMessage =
81+
String.format(MALFORMED_LSC_FILE_ERROR_TEMPLATE, vo, hostname, "No chains found.");
82+
throw new VOMSError(errorMessage);
83+
}
13384
return lsc;
13485
}
13586

87+
private void validateChain(List<String> certificateChainDescription, String vo, String hostname) {
88+
if (certificateChainDescription.size() % 2 != 0) {
89+
String errorMessage = String.format(MALFORMED_LSC_FILE_ERROR_TEMPLATE, vo, hostname,
90+
"Odd number of distinguished name entries.");
91+
throw new VOMSError(errorMessage);
92+
}
93+
if (certificateChainDescription.size() == 0) {
94+
String errorMessage = String.format(MALFORMED_LSC_FILE_ERROR_TEMPLATE, vo, hostname,
95+
"No distinguished name entries found.");
96+
97+
throw new VOMSError(errorMessage);
98+
}
99+
}
100+
136101
public LSCFile parse(String vo, String hostname, File file) {
137102

138103
LSCFile lsc = null;
@@ -150,7 +115,5 @@ public LSCFile parse(String vo, String hostname, File file) {
150115
}
151116

152117
return lsc;
153-
154118
}
155-
156119
}

‎src/main/java/org/italiangrid/voms/store/impl/LSCFile.java‎

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
package org.italiangrid.voms.store.impl;
1717

1818
import java.security.cert.X509Certificate;
19+
import java.util.ArrayList;
1920
import java.util.List;
2021

22+
import javax.security.auth.x500.X500Principal;
23+
2124
import org.italiangrid.voms.store.LSCInfo;
2225

2326
import eu.emi.security.authn.x509.impl.OpensslNameUtils;
@@ -51,7 +54,7 @@ public class LSCFile implements LSCInfo {
5154
String hostname;
5255

5356
/** The certificate chain description contained in this LSC file **/
54-
List<String> certChainDescription;
57+
List<String> certChainDescription = new ArrayList<>();
5558

5659
public String getVOName() {
5760

@@ -95,7 +98,7 @@ public void setHostname(String hostname) {
9598

9699
public void setCertificateChainDescription(List<String> certChainDesc) {
97100

98-
this.certChainDescription = certChainDesc;
101+
this.certChainDescription = new ArrayList<>(certChainDesc);
99102
}
100103

101104
@Override
@@ -138,39 +141,33 @@ public String toString() {
138141
+ hostname + ", certChainDescription=" + certChainDescription + "]";
139142
}
140143

141-
@SuppressWarnings("deprecation")
142144
public boolean matches(X509Certificate[] certChain) {
143145

144-
if (certChainDescription == null || certChainDescription.isEmpty())
146+
if (certChainDescription == null || certChainDescription.isEmpty()) {
145147
return false;
148+
}
146149

147-
if (certChain == null || certChain.length == 0)
150+
if (certChain == null || certChain.length == 0) {
148151
return false;
152+
}
149153

150-
if (certChainDescription.size() == certChain.length * 2) {
151-
152-
for (int i = 0; i < certChain.length; i++) {
153-
154-
String lscSubjectRFC2253 = OpensslNameUtils
155-
.opensslToRfc2253(certChainDescription.get(i));
156-
String lscIssuerRFC2253 = OpensslNameUtils
157-
.opensslToRfc2253(certChainDescription.get(i + 1));
158-
159-
boolean subjectDoesMatch = X500NameUtils.equal(
160-
certChain[i].getSubjectX500Principal(), lscSubjectRFC2253);
161-
boolean issuerDoesMatch = X500NameUtils.equal(
162-
certChain[i].getIssuerX500Principal(), lscIssuerRFC2253);
163-
164-
if (!subjectDoesMatch || !issuerDoesMatch)
165-
return false;
166-
167-
}
168-
} else {
169-
// Cert chain description does not match certificate chain length
154+
if (certChainDescription.size() != certChain.length * 2) {
170155
return false;
171156
}
172157

158+
for (int i = 0; i < certChain.length; i++) {
159+
if (!matches(certChain[i].getSubjectX500Principal(), certChainDescription.get(2 * i))) {
160+
return false;
161+
}
162+
if (!matches(certChain[i].getIssuerX500Principal(), certChainDescription.get(2 * i + 1))) {
163+
return false;
164+
}
165+
}
173166
return true;
174167
}
175168

169+
@SuppressWarnings("deprecation")
170+
private boolean matches(X500Principal certDn, String lscDn) {
171+
return X500NameUtils.equal(certDn, OpensslNameUtils.opensslToRfc2253(lscDn));
172+
}
176173
}

‎src/test/java/org/italiangrid/voms/test/TestLSCParser.java‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static org.junit.Assert.fail;
2222

2323
import java.io.ByteArrayInputStream;
24+
import java.io.File;
2425

2526
import org.italiangrid.voms.VOMSError;
2627
import org.italiangrid.voms.store.impl.DefaultLSCFileParser;
@@ -51,8 +52,7 @@ public void testParse() {
5152

5253
assertEquals(2, f.getCertificateChainDescription().size());
5354

54-
assertEquals("/C=it/O=org/CN=commonName", f
55-
.getCertificateChainDescription().get(0));
55+
assertEquals("/C=it/O=org/CN=commonName", f.getCertificateChainDescription().get(0));
5656
assertEquals("/C=it/O=org/CN=CA", f.getCertificateChainDescription().get(1));
5757

5858
}
@@ -111,6 +111,43 @@ public void testEmptyLSCFileParseError() {
111111
fail("No error caught for malformed, empty LSC file parsing.");
112112
}
113113

114+
@Test
115+
public void testSlashInsideCommonNameIsIgnored() {
116+
117+
DefaultLSCFileParser parser = new DefaultLSCFileParser();
118+
119+
String malformedLSCContent = "/C=it/O=org/CN=commonName\n" + "/C=it/O=org/CN=common/Name";
120+
121+
LSCFile f =
122+
parser.parse("vo", "host", new ByteArrayInputStream(malformedLSCContent.getBytes()));
123+
124+
assertEquals(2, f.getCertificateChainDescription().size());
125+
}
126+
127+
@Test
128+
public void testUnsupportedMultichainLSCFileParseSuccess() {
129+
130+
DefaultLSCFileParser parser = new DefaultLSCFileParser();
131+
132+
String multichainLSCContent = "/C=IT/O=IGI/CN=test-host.cnaf.infn.it\n"
133+
+ "/C=IT/O=IGI/CN=Test CA\n" + "------NEXT CHAIN------\n"
134+
+ "/C=IT/O=IGI/CN=test-host2.cnaf.infn.it\n" + "/C=IT/O=IGI/CN=Test CA";
135+
136+
try {
137+
138+
LSCFile f =
139+
parser.parse("vo", "host", new ByteArrayInputStream(multichainLSCContent.getBytes()));
140+
assertEquals(2, f.getCertificateChainDescription().size());
141+
assertEquals("/C=IT/O=IGI/CN=test-host.cnaf.infn.it", f.getCertificateChainDescription().get(0));
142+
assertEquals("/C=IT/O=IGI/CN=Test CA", f.getCertificateChainDescription().get(1));
143+
144+
} catch (VOMSError e) {
145+
fail("No error expected for malformed, empty LSC file parsing.");
146+
return;
147+
}
148+
}
149+
150+
114151
@Test
115152
public void testNonExistingFileParse() {
116153

@@ -121,7 +158,7 @@ public void testNonExistingFileParse() {
121158
try {
122159

123160
@SuppressWarnings("unused")
124-
LSCFile f = parser.parse("vo", "host", nonExistentFile);
161+
LSCFile f = parser.parse("vo", "host", new File(nonExistentFile));
125162

126163
} catch (VOMSError e) {
127164

‎src/test/resources/vomsdir/test.vo/test-multichain.cnaf.infn.it.lsc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/C=IT/O=IGI/CN=test-host.cnaf.infn.it
2+
/C=IT/O=IGI/CN=Test CA
3+
------NEXT CHAIN------
4+
/C=IT/O=IGI/CN=test-host2.cnaf.infn.it
5+
/C=IT/O=IGI/CN=Test CA

0 commit comments

Comments
 (0)