Add support to take key files with CRLF new line ending on windows (#301)

1. Add support to take key files with windows new line ending (PowerShell/Win32-OpenSSH#1130)
2. add test cases for CRLF
3. Update test helper script to catch the exitcode of unittest and report the failure
4. Enable uni test unittest-sshkey and unittest-sshkey
5. Disable resource check for signal tests due to some API issue to follow.
6. Remove workaround for windows new line ending in test scripts
7. Add test validation for ACL of registry entries when perform ssh-add
This commit is contained in:
Yanbing 2018-04-12 14:24:38 -07:00 committed by GitHub
parent 1616b21ecb
commit 41e4e89376
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 138 additions and 58 deletions

View File

@ -6,10 +6,6 @@ branches:
- latestw_all
- hotfix
init:
- ps: |
[Environment]::OSVersion | fl *
build_script:
- ps: |
Import-Module $env:APPVEYOR_BUILD_FOLDER\contrib\win32\openssh\AppveyorHelper.psm1

View File

@ -184,15 +184,8 @@ WARNING: Following changes will be made to OpenSSH configuration
#copy sshtest keys
Copy-Item "$($Script:E2ETestDirectory)\sshtest*hostkey*" $OpenSSHConfigPath -Force
Get-ChildItem "$($OpenSSHConfigPath)\sshtest*hostkey*"| % {
#workaround for the cariggage new line added by git before copy them
$filePath = "$($_.FullName)"
$con = (Get-Content $filePath | Out-String).Replace("`r`n","`n")
Set-Content -Path $filePath -Value "$con"
if (-not ($_.Name.EndsWith(".pub")))
{
Get-ChildItem "$($OpenSSHConfigPath)\sshtest*hostkey*" -Exclude *.pub| % {
Repair-SshdHostKeyPermission -FilePath $_.FullName -confirm:$false
}
}
#copy ca pubkey to ssh config path
@ -200,9 +193,7 @@ WARNING: Following changes will be made to OpenSSH configuration
#copy ca private key to test dir
$ca_priv_key = (Join-Path $Global:OpenSSHTestInfo["TestDataPath"] sshtest_ca_userkeys)
Copy-Item (Join-Path $Script:E2ETestDirectory sshtest_ca_userkeys) $ca_priv_key -Force
$con = (Get-Content $ca_priv_key | Out-String).Replace("`r`n","`n")
Set-Content -Path $ca_priv_key -Value "$con"
Copy-Item (Join-Path $Script:E2ETestDirectory sshtest_ca_userkeys) $ca_priv_key -Force
Repair-UserSshConfigPermission -FilePath $ca_priv_key -confirm:$false
$Global:OpenSSHTestInfo["CA_Private_Key"] = $ca_priv_key
@ -256,9 +247,7 @@ WARNING: Following changes will be made to OpenSSH configuration
Repair-AuthorizedKeyPermission -FilePath $authorizedKeyPath -confirm:$false
copy-item (Join-Path $Script:E2ETestDirectory sshtest_userssokey_ed25519) $Global:OpenSSHTestInfo["TestDataPath"]
$testPriKeypath = Join-Path $Global:OpenSSHTestInfo["TestDataPath"] sshtest_userssokey_ed25519
$con = (Get-Content $testPriKeypath | Out-String).Replace("`r`n","`n")
Set-Content -Path $testPriKeypath -Value "$con"
$testPriKeypath = Join-Path $Global:OpenSSHTestInfo["TestDataPath"] sshtest_userssokey_ed25519
cmd /c "ssh-add -D 2>&1 >> $Script:TestSetupLogFile"
Repair-UserKeyPermission -FilePath $testPriKeypath -confirm:$false
cmd /c "ssh-add $testPriKeypath 2>&1 >> $Script:TestSetupLogFile"
@ -616,31 +605,49 @@ function Invoke-OpenSSHUnitTest
{
$null = Remove-Item -Path $Script:UnitTestResultsFile -Force -ErrorAction SilentlyContinue
}
$testFolders = Get-ChildItem -filter unittest-*.exe -Recurse -Exclude unittest-sshkey.exe,unittest-kex.exe |
$testFolders = Get-ChildItem -filter unittest-*.exe -Recurse |
ForEach-Object{ Split-Path $_.FullName} |
Sort-Object -Unique
$testfailed = $false
if ($testFolders -ne $null)
{
$testFolders | % {
$testFolders | % {
$unittestFile = "$(Split-Path $_ -Leaf).exe"
$unittestFilePath = join-path $_ $unittestFile
$Error.clear()
$LASTEXITCODE=0
if(Test-Path $unittestFilePath -pathtype leaf)
{
Push-Location $_
Write-Log "Running OpenSSH unit $unittestFile ..."
& "$unittestFilePath" >> $Script:UnitTestResultsFile
Pop-Location
}
$errorCode = $LASTEXITCODE
if ($errorCode -ne 0)
{
$testfailed = $true
$errorMessage = "$_ test failed for OpenSSH.`nExitCode: $errorCode. Detail test log is at $($Script:UnitTestResultsFile)."
Write-Warning $errorMessage
{
$pinfo = New-Object System.Diagnostics.ProcessStartInfo
$pinfo.FileName = "$unittestFilePath"
$pinfo.RedirectStandardError = $true
$pinfo.RedirectStandardOutput = $true
$pinfo.UseShellExecute = $false
$pinfo.WorkingDirectory = "$_"
$p = New-Object System.Diagnostics.Process
$p.StartInfo = $pinfo
$p.Start() | Out-Null
$stdout = $p.StandardOutput.ReadToEnd()
$stderr = $p.StandardError.ReadToEnd()
$p.WaitForExit()
$errorCode = $p.ExitCode
Write-Host "Running unit test: $unittestFile ..."
if(-not [String]::IsNullOrWhiteSpace($stdout))
{
Add-Content $Script:UnitTestResultsFile $stdout
}
if(-not [String]::IsNullOrWhiteSpace($stderr))
{
Add-Content $Script:UnitTestResultsFile $stderr
}
if ($errorCode -ne 0)
{
$testfailed = $true
$errorMessage = "$unittestFile failed.`nExitCode: $errorCode. Detail test log is at $($Script:UnitTestResultsFile)."
Write-Warning $errorMessage
}
else
{
Write-Host "$unittestFile passed!"
}
}
}
}

View File

@ -1580,6 +1580,7 @@
/* #undef socklen_t */
#define WIN32_LEAN_AND_MEAN 1
#define WINDOWS 1
#define SUPPORT_CRLF 1
#define BROKEN_READV_COMPARISON

View File

@ -36,6 +36,53 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$objUserSid = Get-UserSID -User $ssouser
$everyoneSid = Get-UserSID -WellKnownSidType ([System.Security.Principal.WellKnownSidType]::WorldSid)
function ValidateRegistryACL {
param([string]$UserSid = $currentUserSid, $count)
$agentPath = "Registry::HKEY_Users\$UserSid\Software\OpenSSH\Agent"
$myACL = Get-ACL $agentPath
$OwnerSid = Get-UserSid -User $myACL.Owner
$OwnerSid.Equals($adminsSid) | Should Be $true
$myACL.Access | Should Not Be $null
$FullControlPerm = [System.UInt32] [System.Security.AccessControl.RegistryRights]::FullControl.value__
$identities = @($systemSid, $adminsSid)
foreach ($a in $myACL.Access) {
$id = Get-UserSid -User $a.IdentityReference
$identities -contains $id | Should Be $true
([System.UInt32]$a.RegistryRights.value__) | Should Be $FullControlPerm
$a.AccessControlType | Should Be ([System.Security.AccessControl.AccessControlType]::Allow)
$a.IsInherited | Should Be $false
$a.InheritanceFlags | Should Be ([System.Security.AccessControl.InheritanceFlags]::None)
$a.PropagationFlags | Should Be ([System.Security.AccessControl.PropagationFlags]::None)
}
$entries = Get-ChildItem $agentPath\keys
$entries.Count | Should Be $count
if($count -gt 0)
{
Test-Path $agentPath\keys | Should be $true
$entries | % {
$keyentryAcl = Get-Acl $_.pspath
$OwnerSid = Get-UserSid -User $keyentryAcl.Owner
$OwnerSid.Equals($adminsSid) | Should Be $true
$keyentryAcl.Access | Should Not Be $
foreach ($a in $keyentryAcl.Access) {
$id = Get-UserSid -User $a.IdentityReference
$identities -contains $id | Should Be $true
([System.UInt32]$a.RegistryRights.value__) | Should Be $FullControlPerm
$a.AccessControlType | Should Be ([System.Security.AccessControl.AccessControlType]::Allow)
$a.IsInherited | Should Be $false
$a.InheritanceFlags | Should Be ([System.Security.AccessControl.InheritanceFlags]::None)
$a.PropagationFlags | Should Be ([System.Security.AccessControl.PropagationFlags]::None)
}
}
}
else
{
Test-Path $agentPath\keys | Should be $false
}
}
#only validate owner and ACEs of the file
function ValidateKeyFile {
param(
@ -194,8 +241,20 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
foreach($type in $keytypes)
{
$keyPath = Join-Path $testDir "id_$type"
$keyPathDifferentEnding = Join-Path $testDir "id_$($type)_DifferentEnding"
if((Get-Content -Path $keyPath -raw).Contains("`r`n"))
{
$newcontent = (Get-Content -Path $keyPath -raw).Replace("`r`n", "`n")
}
else
{
$newcontent = (Get-Content -Path $keyPath -raw).Replace("`n", "`r`n")
}
Set-content -Path $keyPathDifferentEnding -value "$newcontent"
Repair-UserKeyPermission $keyPathDifferentEnding -confirm:$false
# for ssh-add to consume SSh_ASKPASS, stdin should not be TTY
iex "cmd /c `"ssh-add $keyPath < $nullFile 2> nul `""
iex "cmd /c `"ssh-add $keyPathDifferentEnding < $nullFile 2> nul `""
}
#remove SSH_ASKPASS
@ -204,6 +263,7 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
#ensure added keys are listed
$allkeys = ssh-add -L
$allkeys | Set-Content (Join-Path $testDir "$tC.$tI.allkeyonAdd.txt")
ValidateRegistryACL -count $allkeys.Count
foreach($type in $keytypes)
{
@ -228,7 +288,10 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$keyPath = Join-Path $testDir "id_$type"
$pubkeyraw = ((Get-Content "$keyPath.pub").Split(' '))[1]
@($allkeys | where { $_.contains($pubkeyraw) }).count | Should Be 0
}
}
$allkeys = ssh-add -L
ValidateRegistryACL -count $allkeys.count
}
}
@ -237,15 +300,7 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$keyFileName = "sshadd_userPermTestkey_ed25519"
$keyFilePath = Join-Path $testDir $keyFileName
Remove-Item -path "$keyFilePath*" -Force -ErrorAction SilentlyContinue
if($OpenSSHTestInfo["NoLibreSSL"])
{
ssh-keygen.exe -t ed25519 -f $keyFilePath -P $keypassphrase -Z aes128-ctr
}
else
{
ssh-keygen.exe -t ed25519 -f $keyFilePath -P $keypassphrase
}
ssh-keygen.exe -t ed25519 -f $keyFilePath -P $keypassphrase
#set up SSH_ASKPASS
Add-PasswordSetting -Pass $keypassphrase
$tI=1

View File

@ -71,7 +71,7 @@ signal_test_wait_for_multiple_objects()
{
TEST_START("Signal: APC wakeup with select event counts (WAIT_IO_COMPLETION_ENHANCED)");
TEST_RESOURCES(TRUE);
//TEST_RESOURCES(TRUE);
for (int i = 0; i < objects_size; i++) ResetEvent(hObjects[i]);
for (int i = 0; i < objects_size; i++) {
@ -83,13 +83,13 @@ signal_test_wait_for_multiple_objects()
}
}
TEST_RESOURCES(FALSE);
//TEST_RESOURCES(FALSE);
TEST_DONE();
}
{
TEST_START("Signal: Wait-any with one invalid event in positions 1-300 (WAIT_FAILED_ENHANCED)");
TEST_RESOURCES(TRUE);
//TEST_RESOURCES(TRUE);
for (int i = 0; i < objects_size; i++) ResetEvent(hObjects[i]);
for (int i = 0; i < objects_size; i++) {
@ -100,13 +100,13 @@ signal_test_wait_for_multiple_objects()
hObjects[i] = event;
}
TEST_RESOURCES(FALSE);
//TEST_RESOURCES(FALSE);
TEST_DONE();
}
{
TEST_START("Signal: Wait-any with signaled event in positions 1-300 (WAIT_OBJECT_0_ENHANCED)");
TEST_RESOURCES(TRUE);
//TEST_RESOURCES(TRUE);
for (int i = 0; i < objects_size; i++) {
SetEvent(hObjects[i]);
@ -115,25 +115,25 @@ signal_test_wait_for_multiple_objects()
ResetEvent(hObjects[i]);
}
TEST_RESOURCES(FALSE);
//TEST_RESOURCES(FALSE);
TEST_DONE();
}
{
TEST_START("Signal: Wait-any with latent events (WAIT_TIMEOUT_ENHANCED)");
TEST_RESOURCES(TRUE);
//TEST_RESOURCES(TRUE);
for (int i = 0; i < objects_size; i++) ResetEvent(hObjects[i]);
DWORD ret = wait_for_multiple_objects_enhanced(objects_size, hObjects, 250, FALSE);
ASSERT_INT_EQ(ret, WAIT_TIMEOUT_ENHANCED);
TEST_RESOURCES(FALSE);
//TEST_RESOURCES(FALSE);
TEST_DONE();
}
{
TEST_START("Signal: Wait-any with async event in positions 1-300 (WAIT_OBJECT_0_ENHANCED offset)");
TEST_RESOURCES(TRUE);
//TEST_RESOURCES(TRUE);
for (int i = 0; i < objects_size; i++) ResetEvent(hObjects[i]);
for (int i = 0; i < objects_size; i++) {
@ -143,13 +143,13 @@ signal_test_wait_for_multiple_objects()
ResetEvent(hObjects[i]);
}
TEST_RESOURCES(FALSE);
//TEST_RESOURCES(FALSE);
TEST_DONE();
}
{
TEST_START("Signal: Wait-any with abandoned mutex in positions 1-300 (WAIT_ABANDONED_0_ENHANCED offset)");
TEST_RESOURCES(TRUE);
//TEST_RESOURCES(TRUE);
for (int i = 0; i < objects_size; i++) ResetEvent(hObjects[i]);
for (int i = 0; i < objects_size; i++) {
@ -160,7 +160,7 @@ signal_test_wait_for_multiple_objects()
hObjects[i] = original_event;
}
TEST_RESOURCES(FALSE);
//TEST_RESOURCES(FALSE);
TEST_DONE();
}

View File

@ -65,6 +65,12 @@
#define MARK_END "-----END OPENSSH PRIVATE KEY-----\n"
#define MARK_BEGIN_LEN (sizeof(MARK_BEGIN) - 1)
#define MARK_END_LEN (sizeof(MARK_END) - 1)
#ifdef SUPPORT_CRLF
#define MARK_BEGIN_CRLF "-----BEGIN OPENSSH PRIVATE KEY-----\r\n"
#define MARK_END_CRLF "-----END OPENSSH PRIVATE KEY-----\r\n"
#define MARK_BEGIN_LEN_CRLF (sizeof(MARK_BEGIN_CRLF) - 1)
#define MARK_END_LEN_CRLF (sizeof(MARK_END_CRLF) - 1)
#endif // SUPPORT_CRLF
#define KDFNAME "bcrypt"
#define AUTH_MAGIC "openssh-key-v1"
#define SALT_LEN 16
@ -3415,8 +3421,16 @@ sshkey_parse_private2(struct sshbuf *blob, int type, const char *passphrase,
/* check preamble */
cp = sshbuf_ptr(blob);
encoded_len = sshbuf_len(blob);
#ifdef SUPPORT_CRLF
if ((encoded_len < (MARK_BEGIN_LEN + MARK_END_LEN) ||
memcmp(cp, MARK_BEGIN, MARK_BEGIN_LEN) != 0) &&
(encoded_len < (MARK_BEGIN_LEN_CRLF + MARK_END_LEN_CRLF) ||
memcmp(cp, MARK_BEGIN_CRLF, MARK_BEGIN_LEN_CRLF) != 0)) {
#else /* !SUPPORT_CRLF */
if (encoded_len < (MARK_BEGIN_LEN + MARK_END_LEN) ||
memcmp(cp, MARK_BEGIN, MARK_BEGIN_LEN) != 0) {
memcmp(cp, MARK_BEGIN, MARK_BEGIN_LEN) != 0) {
#endif /* !SUPPORT_CRLF */
r = SSH_ERR_INVALID_FORMAT;
goto out;
}
@ -3433,8 +3447,15 @@ sshkey_parse_private2(struct sshbuf *blob, int type, const char *passphrase,
encoded_len--;
cp++;
if (last == '\n') {
#ifdef SUPPORT_CRLF
if ((encoded_len >= MARK_END_LEN &&
memcmp(cp, MARK_END, MARK_END_LEN) == 0) ||
(encoded_len >= MARK_END_LEN_CRLF &&
memcmp(cp, MARK_END_CRLF, MARK_END_LEN_CRLF) == 0)) {
#else /* !SUPPORT_CRLF */
if (encoded_len >= MARK_END_LEN &&
memcmp(cp, MARK_END, MARK_END_LEN) == 0) {
#endif /* !SUPPORT_CRLF */
/* \0 terminate */
if ((r = sshbuf_put_u8(encoded, 0)) != 0)
goto out;