Skip to content

Commit 2db92ee

Browse files
Vikas-SajananiVikas-Sajanani
committed
fix: more robust handling of whitespace and alternate pem format with tests
1 parent 9493ca9 commit 2db92ee

File tree

4 files changed

+296
-29
lines changed

4 files changed

+296
-29
lines changed

‎src/main/java/com/spotify/github/v3/clients/PKCS1PEMKey.java‎

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
* Licensed under the Apache License, Version 2.0 (the "License");
88
* you may not use this file except in compliance with the License.
99
* You may obtain a copy of the License at
10-
*
10+
*
1111
* http://www.apache.org/licenses/LICENSE-2.0
12-
*
12+
*
1313
* Unless required by applicable law or agreed to in writing, software
1414
* distributed under the License is distributed on an "AS IS" BASIS,
1515
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -32,26 +32,68 @@
3232
*/
3333
final class PKCS1PEMKey {
3434

35+
// Matches RSA PEM format
3536
private static final Pattern PKCS1_PEM_KEY_PATTERN =
36-
Pattern.compile("(?m)(?s)^---*BEGIN RSA PRIVATE KEY.*---*$(.*)^---*END.*---*$.*");
37+
Pattern.compile("(?m)(?s)^\\s*-{3,}\\s*BEGIN\\s+RSA\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$(.*)^\\s*-{3,}\\s*END\\s+RSA\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$.*");
38+
39+
// Matches PKCS8 PEM format
40+
private static final Pattern PKCS8_PEM_KEY_PATTERN =
41+
Pattern.compile("(?m)(?s)^\\s*-{3,}\\s*BEGIN\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$(.*)^\\s*-{3,}\\s*END\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$.*");
3742

3843
private PKCS1PEMKey() {}
3944

4045
/**
41-
* Try to interpret the supplied key as a PKCS#1 PEM file.
46+
* Try to interpret the supplied key as a PEM file (PKCS#1 or PKCS#8).
4247
*
4348
* @param privateKey the private key to use
4449
*/
4550
public static Optional<KeySpec> loadKeySpec(final byte[] privateKey) {
46-
final Matcher isPEM = PKCS1_PEM_KEY_PATTERN.matcher(new String(privateKey));
47-
if (!isPEM.matches()) {
48-
return Optional.empty();
51+
final String keyString = new String(privateKey);
52+
53+
// Try to match PKCS1 (RSA) format first
54+
Matcher isPKCS1 = PKCS1_PEM_KEY_PATTERN.matcher(keyString);
55+
if (isPKCS1.matches()) {
56+
return extractKeySpec(isPKCS1.group(1), true);
57+
}
58+
59+
// Try to match PKCS8 format
60+
Matcher isPKCS8 = PKCS8_PEM_KEY_PATTERN.matcher(keyString);
61+
if (isPKCS8.matches()) {
62+
return extractKeySpec(isPKCS8.group(1), false);
4963
}
5064

51-
byte[] pkcs1Key = Base64.getMimeDecoder().decode(isPEM.group(1));
52-
byte[] pkcs8Key = toPkcs8(pkcs1Key);
53-
final PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(pkcs8Key);
54-
return Optional.of(keySpec);
65+
// Not a recognized PEM format
66+
return Optional.empty();
67+
}
68+
69+
/**
70+
* Extract a KeySpec from the base64 content.
71+
*
72+
* @param base64Content the base64 content from the PEM file
73+
* @param isPKCS1 whether this is PKCS1 format (needs conversion to PKCS8)
74+
* @return an Optional containing the KeySpec if successful
75+
*/
76+
private static Optional<KeySpec> extractKeySpec(String base64Content, boolean isPKCS1) {
77+
try {
78+
// Remove all whitespace
79+
base64Content = base64Content.replaceAll("\\s+", "");
80+
81+
// Check if content is empty after whitespace removal
82+
if (base64Content.isEmpty()) {
83+
return Optional.empty();
84+
}
85+
86+
// Decode the base64 content
87+
byte[] decodedKey = Base64.getDecoder().decode(base64Content);
88+
89+
// Convert to PKCS8 if necessary
90+
byte[] pkcs8Key = isPKCS1 ? toPkcs8(decodedKey) : decodedKey;
91+
92+
return Optional.of(new PKCS8EncodedKeySpec(pkcs8Key));
93+
} catch (IllegalArgumentException e) {
94+
// Failed to decode base64 content
95+
return Optional.empty();
96+
}
5597
}
5698

5799
/**
@@ -68,15 +110,15 @@ private static byte[] toPkcs8(final byte[] pkcs1Bytes) {
68110
final int pkcs1Length = pkcs1Bytes.length;
69111
final int totalLength = pkcs1Length + 22;
70112
byte[] pkcs8Header = new byte[] {
71-
0x30, (byte) 0x82, (byte) ((totalLength >> 8) & 0xff), (byte) (totalLength & 0xff), // Sequence + total length
72-
0x2, 0x1, 0x0, // Integer (0)
73-
0x30, 0xD, 0x6, 0x9, 0x2A, (byte) 0x86, 0x48, (byte) 0x86, (byte) 0xF7, 0xD, 0x1, 0x1, 0x1, 0x5, 0x0, // Sequence: 1.2.840.113549.1.1.1, NULL
74-
0x4, (byte) 0x82, (byte) ((pkcs1Length >> 8) & 0xff), (byte) (pkcs1Length & 0xff) // Octet string + length
113+
0x30, (byte) 0x82, (byte) ((totalLength >> 8) & 0xff), (byte) (totalLength & 0xff), // Sequence + total length
114+
0x2, 0x1, 0x0, // Integer (0)
115+
0x30, 0xD, 0x6, 0x9, 0x2A, (byte) 0x86, 0x48, (byte) 0x86, (byte) 0xF7, 0xD, 0x1, 0x1, 0x1, 0x5, 0x0, // Sequence: 1.2.840.113549.1.1.1, NULL
116+
0x4, (byte) 0x82, (byte) ((pkcs1Length >> 8) & 0xff), (byte) (pkcs1Length & 0xff) // Octet string + length
75117
};
76118

77119
byte[] pkcs8bytes = new byte[pkcs8Header.length + pkcs1Bytes.length];
78120
System.arraycopy(pkcs8Header, 0, pkcs8bytes, 0, pkcs8Header.length);
79121
System.arraycopy(pkcs1Bytes, 0, pkcs8bytes, pkcs8Header.length, pkcs1Bytes.length);
80122
return pkcs8bytes;
81123
}
82-
}
124+
}

‎src/test/java/com/spotify/github/v3/clients/GitHubClientPEMTest.java‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*-
2+
* -\-\-
3+
* github-api
4+
* --
5+
* Copyright (C) 2016 - 2025 Spotify AB
6+
* --
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
* -/-/-
19+
*/
20+
21+
package com.spotify.github.v3.clients;
22+
23+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
24+
import static org.junit.jupiter.api.Assertions.assertNotNull;
25+
26+
import com.google.common.io.Resources;
27+
import java.io.IOException;
28+
import java.nio.charset.StandardCharsets;
29+
import java.util.regex.Matcher;
30+
import java.util.regex.Pattern;
31+
import org.junit.jupiter.api.Test;
32+
33+
/**
34+
* Tests for GitHub with various PEM key formats
35+
*/
36+
public class GitHubClientPEMTest {
37+
38+
private static final Pattern BEGIN_PATTERN = Pattern.compile("(-----)BEGIN RSA PRIVATE KEY(-----)");
39+
private static final Pattern END_PATTERN = Pattern.compile("(-----)END RSA PRIVATE KEY(-----)");
40+
41+
@Test
42+
public void testJwtWithPoorlyFormattedPEM() throws Exception {
43+
// Load a PEM key resource
44+
byte[] originalKey = Resources.toByteArray(
45+
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem"));
46+
String pemContent = new String(originalKey, StandardCharsets.UTF_8);
47+
48+
// Manipulate just the BEGIN/END delimiters without touching the key content
49+
String poorlyFormattedPem = pemContent;
50+
51+
// Add spaces to BEGIN
52+
Matcher beginMatcher = BEGIN_PATTERN.matcher(poorlyFormattedPem);
53+
if (beginMatcher.find()) {
54+
poorlyFormattedPem = poorlyFormattedPem.replace(
55+
beginMatcher.group(0),
56+
beginMatcher.group(1) + " BEGIN RSA PRIVATE KEY " + beginMatcher.group(2)
57+
);
58+
}
59+
60+
// Add spaces to END
61+
Matcher endMatcher = END_PATTERN.matcher(poorlyFormattedPem);
62+
if (endMatcher.find()) {
63+
poorlyFormattedPem = poorlyFormattedPem.replace(
64+
endMatcher.group(0),
65+
endMatcher.group(1) + " END RSA PRIVATE KEY " + endMatcher.group(2)
66+
);
67+
}
68+
69+
// Test that we can parse this PEM
70+
JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(
71+
poorlyFormattedPem.getBytes(StandardCharsets.UTF_8));
72+
String token = tokenIssuer.getToken(123456);
73+
assertNotNull(token);
74+
}
75+
76+
@Test
77+
public void testJwtWithExtraDashesPEM() throws Exception {
78+
// Load a PEM key resource
79+
byte[] originalKey = Resources.toByteArray(
80+
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem"));
81+
String pemContent = new String(originalKey, StandardCharsets.UTF_8);
82+
83+
// Manipulate just the BEGIN/END delimiters without touching the key content
84+
String extraDashesPem = pemContent;
85+
86+
// Add extra dashes to BEGIN
87+
Matcher beginMatcher = BEGIN_PATTERN.matcher(extraDashesPem);
88+
if (beginMatcher.find()) {
89+
extraDashesPem = extraDashesPem.replace(
90+
beginMatcher.group(0),
91+
"-------BEGIN RSA PRIVATE KEY-------"
92+
);
93+
}
94+
95+
// Add extra dashes to END
96+
Matcher endMatcher = END_PATTERN.matcher(extraDashesPem);
97+
if (endMatcher.find()) {
98+
extraDashesPem = extraDashesPem.replace(
99+
endMatcher.group(0),
100+
"-------END RSA PRIVATE KEY-------"
101+
);
102+
}
103+
104+
// Test that we can parse this PEM
105+
JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(
106+
extraDashesPem.getBytes(StandardCharsets.UTF_8));
107+
String token = tokenIssuer.getToken(123456);
108+
assertNotNull(token);
109+
}
110+
}

‎src/test/java/com/spotify/github/v3/clients/JwtTokenIssuerTest.java‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
* Licensed under the Apache License, Version 2.0 (the "License");
88
* you may not use this file except in compliance with the License.
99
* You may obtain a copy of the License at
10-
*
10+
*
1111
* http://www.apache.org/licenses/LICENSE-2.0
12-
*
12+
*
1313
* Unless required by applicable law or agreed to in writing, software
1414
* distributed under the License is distributed on an "AS IS" BASIS,
1515
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -20,30 +20,31 @@
2020

2121
package com.spotify.github.v3.clients;
2222

23-
import static org.hamcrest.CoreMatchers.not;
24-
import static org.hamcrest.CoreMatchers.nullValue;
25-
import static org.hamcrest.MatcherAssert.assertThat;
23+
import static org.junit.jupiter.api.Assertions.assertNotNull;
2624

2725
import com.google.common.io.Resources;
2826
import java.net.URL;
2927
import org.junit.jupiter.api.Test;
3028

29+
/**
30+
* Simple test for JwtTokenIssuer to focus on core functionality
31+
*/
3132
public class JwtTokenIssuerTest {
3233

3334
private static final URL DER_KEY_RESOURCE =
34-
Resources.getResource("com/spotify/github/v3/github-private-key");
35+
Resources.getResource("com/spotify/github/v3/github-private-key");
3536

36-
// generated using this command: "openssl genrsa -out fake-github-app-key.pem 2048"
3737
private static final URL PEM_KEY_RESOURCE =
38-
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem");
38+
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem");
3939

4040
@Test
41-
public void loadsDERFileWithPKCS8Key() throws Exception {
41+
public void loadsDERFile() throws Exception {
4242
final byte[] key = Resources.toByteArray(DER_KEY_RESOURCE);
4343
final JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(key);
4444

4545
final String token = tokenIssuer.getToken(42);
46-
assertThat(token, not(nullValue()));
46+
assertNotNull(token);
47+
System.out.println("DER key JWT token generated successfully: " + token);
4748
}
4849

4950
@Test
@@ -52,7 +53,7 @@ public void loadsPEMFile() throws Exception {
5253
final JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(key);
5354

5455
final String token = tokenIssuer.getToken(42);
55-
assertThat(token, not(nullValue()));
56+
assertNotNull(token);
57+
System.out.println("PEM key JWT token generated successfully: " + token);
5658
}
57-
58-
}
59+
}

0 commit comments

Comments
 (0)