Crypto cleanup and enable usage of FIPS compliant crypto when required (#806)

* Use FIPS compliant crypto when required

* Comment cleanup

* Store OAuth signing scheme in credentialData instead of runner setting

Add encryption scheme for job message encyption key to session

Further cleanup of unused crypto code

* Update windows rsa key manager to use crossplat dotnet RSA api

* Undo unneeded ConfigurationManager change
This commit is contained in:
David Kale
2020-12-04 11:35:16 -05:00
committed by GitHub
parent a2e32170fd
commit 80bf68db81
9 changed files with 40 additions and 293 deletions

View File

@@ -263,6 +263,7 @@ namespace GitHub.Runner.Listener.Configuration
{
{ "clientId", agent.Authorization.ClientId.ToString("D") },
{ "authorizationUrl", agent.Authorization.AuthorizationUrl.AbsoluteUri },
{ "requireFipsCryptography", agent.Properties.GetValue("RequireFipsCryptography", false).ToString() }
},
};

View File

@@ -20,7 +20,7 @@ namespace GitHub.Runner.Listener.Configuration
/// key is returned to the caller.
/// </summary>
/// <returns>An <c>RSACryptoServiceProvider</c> instance representing the key for the runner</returns>
RSACryptoServiceProvider CreateKey();
RSA CreateKey();
/// <summary>
/// Deletes the RSA key managed by the key manager.
@@ -32,7 +32,7 @@ namespace GitHub.Runner.Listener.Configuration
/// </summary>
/// <returns>An <c>RSACryptoServiceProvider</c> instance representing the key for the runner</returns>
/// <exception cref="CryptographicException">No key exists in the store</exception>
RSACryptoServiceProvider GetKey();
RSA GetKey();
}
// Newtonsoft 10 is not working properly with dotnet RSAParameters class

View File

@@ -36,7 +36,7 @@ namespace GitHub.Runner.Listener.Configuration
// We expect the key to be in the machine store at this point. Configuration should have set all of
// this up correctly so we can use the key to generate access tokens.
var keyManager = context.GetService<IRSAKeyManager>();
var signingCredentials = VssSigningCredentials.Create(() => keyManager.GetKey());
var signingCredentials = VssSigningCredentials.Create(() => keyManager.GetKey(), StringUtil.ConvertToBoolean(CredentialData.Data.GetValueOrDefault("requireFipsCryptography"), false));
var clientCredential = new VssOAuthJwtBearerClientCredential(clientId, authorizationUrl, signingCredentials);
var agentCredential = new VssOAuthCredential(new Uri(oauthEndpointUrl, UriKind.Absolute), VssOAuthGrant.ClientCredentials, clientCredential);

View File

@@ -13,14 +13,14 @@ namespace GitHub.Runner.Listener.Configuration
private string _keyFile;
private IHostContext _context;
public RSACryptoServiceProvider CreateKey()
public RSA CreateKey()
{
RSACryptoServiceProvider rsa = null;
RSA rsa = null;
if (!File.Exists(_keyFile))
{
Trace.Info("Creating new RSA key using 2048-bit key length");
rsa = new RSACryptoServiceProvider(2048);
rsa = RSA.Create(2048);
// Now write the parameters to disk
SaveParameters(rsa.ExportParameters(true));
@@ -30,7 +30,7 @@ namespace GitHub.Runner.Listener.Configuration
{
Trace.Info("Found existing RSA key parameters file {0}", _keyFile);
rsa = new RSACryptoServiceProvider();
rsa = RSA.Create();
rsa.ImportParameters(LoadParameters());
}
@@ -46,7 +46,7 @@ namespace GitHub.Runner.Listener.Configuration
}
}
public RSACryptoServiceProvider GetKey()
public RSA GetKey()
{
if (!File.Exists(_keyFile))
{
@@ -55,7 +55,7 @@ namespace GitHub.Runner.Listener.Configuration
Trace.Info("Loading RSA key parameters from file {0}", _keyFile);
var rsa = new RSACryptoServiceProvider();
var rsa = RSA.Create();
rsa.ImportParameters(LoadParameters());
return rsa;
}

View File

@@ -14,14 +14,14 @@ namespace GitHub.Runner.Listener.Configuration
private string _keyFile;
private IHostContext _context;
public RSACryptoServiceProvider CreateKey()
public RSA CreateKey()
{
RSACryptoServiceProvider rsa = null;
RSA rsa = null;
if (!File.Exists(_keyFile))
{
Trace.Info("Creating new RSA key using 2048-bit key length");
rsa = new RSACryptoServiceProvider(2048);
rsa = RSA.Create(2048);
// Now write the parameters to disk
IOUtil.SaveObject(new RSAParametersSerializable(rsa.ExportParameters(true)), _keyFile);
@@ -54,7 +54,7 @@ namespace GitHub.Runner.Listener.Configuration
{
Trace.Info("Found existing RSA key parameters file {0}", _keyFile);
rsa = new RSACryptoServiceProvider();
rsa = RSA.Create();
rsa.ImportParameters(IOUtil.LoadObject<RSAParametersSerializable>(_keyFile).RSAParameters);
}
@@ -70,7 +70,7 @@ namespace GitHub.Runner.Listener.Configuration
}
}
public RSACryptoServiceProvider GetKey()
public RSA GetKey()
{
if (!File.Exists(_keyFile))
{
@@ -80,7 +80,7 @@ namespace GitHub.Runner.Listener.Configuration
Trace.Info("Loading RSA key parameters from file {0}", _keyFile);
var parameters = IOUtil.LoadObject<RSAParametersSerializable>(_keyFile).RSAParameters;
var rsa = new RSACryptoServiceProvider();
var rsa = RSA.Create();
rsa.ImportParameters(parameters);
return rsa;
}

View File

@@ -319,7 +319,8 @@ namespace GitHub.Runner.Listener
var keyManager = HostContext.GetService<IRSAKeyManager>();
using (var rsa = keyManager.GetKey())
{
return aes.CreateDecryptor(rsa.Decrypt(_session.EncryptionKey.Value, RSAEncryptionPadding.OaepSHA1), message.IV);
var padding = _session.UseFipsEncryption ? RSAEncryptionPadding.OaepSHA256 : RSAEncryptionPadding.OaepSHA1;
return aes.CreateDecryptor(rsa.Decrypt(_session.EncryptionKey.Value, padding), message.IV);
}
}
else

View File

@@ -65,5 +65,15 @@ namespace GitHub.DistributedTask.WebApi
get;
set;
}
/// <summary>
/// Gets or sets whether to use FIPS compliant encryption scheme for job message key
/// </summary>
[DataMember]
public bool UseFipsEncryption
{
get;
set;
}
}
}

View File

@@ -130,55 +130,6 @@ namespace GitHub.Services.WebApi.Jwt
return credentials.SignatureAlgorithm;
}
public static ClaimsPrincipal ValidateToken(this JsonWebToken token, JsonWebTokenValidationParameters parameters)
{
ArgumentUtility.CheckForNull(token, nameof(token));
ArgumentUtility.CheckForNull(parameters, nameof(parameters));
ClaimsIdentity actorIdentity = ValidateActor(token, parameters);
ValidateLifetime(token, parameters);
ValidateAudience(token, parameters);
ValidateSignature(token, parameters);
ValidateIssuer(token, parameters);
ClaimsIdentity identity = new ClaimsIdentity("Federation", parameters.IdentityNameClaimType, ClaimTypes.Role);
if (actorIdentity != null)
{
identity.Actor = actorIdentity;
}
IEnumerable<Claim> claims = token.ExtractClaims();
foreach (Claim claim in claims)
{
identity.AddClaim(new Claim(claim.Type, claim.Value, claim.ValueType, token.Issuer));
}
return new ClaimsPrincipal(identity);
}
private static ClaimsIdentity ValidateActor(JsonWebToken token, JsonWebTokenValidationParameters parameters)
{
ArgumentUtility.CheckForNull(token, nameof(token));
ArgumentUtility.CheckForNull(parameters, nameof(parameters));
if (!parameters.ValidateActor)
{
return null;
}
//this recursive call with check the parameters
ClaimsPrincipal principal = token.Actor.ValidateToken(parameters.ActorValidationParameters);
if (!(principal?.Identity is ClaimsIdentity))
{
throw new ActorValidationException();
}
return (ClaimsIdentity)principal.Identity;
}
private static void ValidateLifetime(JsonWebToken token, JsonWebTokenValidationParameters parameters)
{
ArgumentUtility.CheckForNull(token, nameof(token));
@@ -241,59 +192,6 @@ namespace GitHub.Services.WebApi.Jwt
throw new InvalidAudienceException(); //validation exception;
}
private static void ValidateSignature(JsonWebToken token, JsonWebTokenValidationParameters parameters)
{
ArgumentUtility.CheckForNull(token, nameof(token));
ArgumentUtility.CheckForNull(parameters, nameof(parameters));
if (!parameters.ValidateSignature)
{
return;
}
string encodedData = token.EncodedToken;
string[] parts = encodedData.Split('.');
if (parts.Length != 3)
{
throw new InvalidTokenException(JwtResources.EncodedTokenDataMalformed()); //validation exception
}
if (string.IsNullOrEmpty(parts[2]))
{
throw new InvalidTokenException(JwtResources.SignatureNotFound()); //validation exception
}
if (token.Algorithm == JWTAlgorithm.None)
{
throw new InvalidTokenException(JwtResources.InvalidSignatureAlgorithm()); //validation exception
}
ArgumentUtility.CheckForNull(parameters.SigningCredentials, nameof(parameters.SigningCredentials));
//ArgumentUtility.CheckEnumerableForNullOrEmpty(parameters.SigningToken.SecurityKeys, nameof(parameters.SigningToken.SecurityKeys));
byte[] sourceInput = Encoding.UTF8.GetBytes(string.Format("{0}.{1}", parts[0], parts[1]));
byte[] sourceSignature = parts[2].FromBase64StringNoPadding();
try
{
if (parameters.SigningCredentials.VerifySignature(sourceInput, sourceSignature))
{
return;
}
}
catch (Exception)
{
//swallow exceptions here, we'll throw if nothing works...
}
throw new SignatureValidationException(); //valiation exception
}
private static void ValidateIssuer(JsonWebToken token, JsonWebTokenValidationParameters parameters)
{
ArgumentUtility.CheckForNull(token, nameof(token));

View File

@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using GitHub.Services.Common;
using GitHub.Services.WebApi.Jwt;
@@ -75,7 +74,6 @@ namespace GitHub.Services.WebApi
{
throw new InvalidOperationException();
}
return GetSignature(input);
}
@@ -86,48 +84,13 @@ namespace GitHub.Services.WebApi
/// <returns>A blob of data representing the signature of the input data</returns>
protected abstract Byte[] GetSignature(Byte[] input);
/// <summary>
/// Verifies the signature of the input data, returning true if the signature is valid.
/// </summary>
/// <param name="input">The data which should be signed</param>
/// <param name="signature">The signature which should be verified</param>
/// <returns>True if the provided signature matches the current signing token; otherwise, false</returns>
public abstract Boolean VerifySignature(Byte[] input, Byte[] signature);
/// <summary>
/// Creates a new <c>VssSigningCredentials</c> instance using the specified <paramref name="certificate"/> instance
/// as the signing key.
/// </summary>
/// <param name="certificate">The certificate which contains the key used for signing and verification</param>
/// <returns>A new <c>VssSigningCredentials</c> instance which uses the specified certificate for signing</returns>
public static VssSigningCredentials Create(X509Certificate2 certificate)
{
ArgumentUtility.CheckForNull(certificate, nameof(certificate));
if (certificate.HasPrivateKey)
{
var rsa = certificate.GetRSAPrivateKey();
if (rsa == null)
{
throw new SignatureAlgorithmUnsupportedException(certificate.SignatureAlgorithm.FriendlyName);
}
if (rsa.KeySize < c_minKeySize)
{
throw new InvalidCredentialsException(JwtResources.SigningTokenKeyTooSmall());
}
}
return new X509Certificate2SigningToken(certificate);
}
/// <summary>
/// Creates a new <c>VssSigningCredentials</c> instance using the specified <paramref name="factory"/>
/// callback function to retrieve the signing key.
/// </summary>
/// <param name="factory">The factory which creates <c>RSA</c> keys used for signing and verification</param>
/// <returns>A new <c>VssSigningCredentials</c> instance which uses the specified provider for signing</returns>
public static VssSigningCredentials Create(Func<RSA> factory)
public static VssSigningCredentials Create(Func<RSA> factory, bool requireFipsCryptography)
{
ArgumentUtility.CheckForNull(factory, nameof(factory));
@@ -143,80 +106,19 @@ namespace GitHub.Services.WebApi
throw new InvalidCredentialsException(JwtResources.SigningTokenKeyTooSmall());
}
return new RSASigningToken(factory, rsa.KeySize);
if (requireFipsCryptography)
{
return new RSASigningToken(factory, rsa.KeySize, RSASignaturePadding.Pss);
}
return new RSASigningToken(factory, rsa.KeySize, RSASignaturePadding.Pkcs1);
}
}
/// <summary>
/// Creates a new <c>VssSigningCredentials</c> instance using the specified <paramref name="key"/> as the signing
/// key. The returned signing token performs symmetric key signing and verification.
/// </summary>
/// <param name="rsa">The key used for signing and verification</param>
/// <returns>A new <c>VssSigningCredentials</c> instance which uses the specified key for signing</returns>
public static VssSigningCredentials Create(Byte[] key)
{
ArgumentUtility.CheckForNull(key, nameof(key));
// Probably should have validation here, but there was none previously
return new SymmetricKeySigningToken(key);
}
private const Int32 c_minKeySize = 2048;
private readonly DateTime m_effectiveDate;
#region Concrete Implementations
private class SymmetricKeySigningToken : VssSigningCredentials
{
public SymmetricKeySigningToken(Byte[] key)
{
m_key = new Byte[key.Length];
Buffer.BlockCopy(key, 0, m_key, 0, m_key.Length);
}
public override Boolean CanSignData
{
get
{
return true;
}
}
public override Int32 KeySize
{
get
{
return m_key.Length * 8;
}
}
public override JWTAlgorithm SignatureAlgorithm
{
get
{
return JWTAlgorithm.HS256;
}
}
protected override Byte[] GetSignature(Byte[] input)
{
using (var hash = new HMACSHA256(m_key))
{
return hash.ComputeHash(input);
}
}
public override Boolean VerifySignature(
Byte[] input,
Byte[] signature)
{
var computedSignature = SignData(input);
return SecureCompare.TimeInvariantEquals(computedSignature, signature);
}
private readonly Byte[] m_key;
}
private abstract class AsymmetricKeySigningToken : VssSigningCredentials
{
protected abstract Boolean HasPrivateKey();
@@ -244,70 +146,14 @@ namespace GitHub.Services.WebApi
private Boolean? m_hasPrivateKey;
}
private class X509Certificate2SigningToken : AsymmetricKeySigningToken, IJsonWebTokenHeaderProvider
{
public X509Certificate2SigningToken(X509Certificate2 certificate)
{
m_certificate = certificate;
}
public override Int32 KeySize
{
get
{
return m_certificate.GetRSAPublicKey().KeySize;
}
}
public override DateTime ValidFrom
{
get
{
return m_certificate.NotBefore;
}
}
public override DateTime ValidTo
{
get
{
return m_certificate.NotAfter;
}
}
public override Boolean VerifySignature(
Byte[] input,
Byte[] signature)
{
var rsa = m_certificate.GetRSAPublicKey();
return rsa.VerifyData(input, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
}
protected override Byte[] GetSignature(Byte[] input)
{
var rsa = m_certificate.GetRSAPrivateKey();
return rsa.SignData(input, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
}
protected override Boolean HasPrivateKey()
{
return m_certificate.HasPrivateKey;
}
void IJsonWebTokenHeaderProvider.SetHeaders(IDictionary<String, Object> headers)
{
headers[JsonWebTokenHeaderParameters.X509CertificateThumbprint] = m_certificate.GetCertHash().ToBase64StringNoPadding();
}
private readonly X509Certificate2 m_certificate;
}
private class RSASigningToken : AsymmetricKeySigningToken
{
public RSASigningToken(
Func<RSA> factory,
Int32 keySize)
Int32 keySize,
RSASignaturePadding signaturePadding)
{
m_signaturePadding = signaturePadding;
m_keySize = keySize;
m_factory = factory;
}
@@ -324,7 +170,7 @@ namespace GitHub.Services.WebApi
{
using (var rsa = m_factory())
{
return rsa.SignData(input, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
return rsa.SignData(input, HashAlgorithmName.SHA256, m_signaturePadding);
}
}
@@ -335,7 +181,7 @@ namespace GitHub.Services.WebApi
// As unfortunate as this is, there is no way to tell from an RSA implementation, based on querying
// properties alone, if it supports signature operations or has a private key. This is a one-time
// hit for the signing credentials implementation, so it shouldn't be a huge deal.
GetSignature(new Byte[1] { 1 });
GetSignature(new Byte[1] { 1 });
return true;
}
catch (CryptographicException)
@@ -344,18 +190,9 @@ namespace GitHub.Services.WebApi
}
}
public override Boolean VerifySignature(
Byte[] input,
Byte[] signature)
{
using (var rsa = m_factory())
{
return rsa.VerifyData(input, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
}
}
private readonly Int32 m_keySize;
private readonly Func<RSA> m_factory;
private readonly RSASignaturePadding m_signaturePadding;
}
#endregion