aws / aws-ec2-instance-connect-config

This is the ssh daemon configuration and necessary EC2 instance scripting to enable EC2 Instance Connect. Also included is various package manager configurations for packaging for various Linux distributions.

Apache License 2.0

83 stars 35 forks source link

Fixed #20 AuthorizedKeysCommand error #21

Closed yoshifumi-kinoshita closed 4 years ago

yoshifumi-kinoshita commented 4 years ago

Issue #, if available:

20 "error: AuthorizedKeysCommand /opt/aws/bin/eic_run_authorized_keys ...snip... failed, status 22" error

Description of changes: Because of the combination of shell option set -e and curl -f, curl returns 22 when HTTP response is 4xx or 5xx, and eic_curl_authorized_keys stops immediately because of the curl's failure. Added && true at the end of the curl command line to turn the command into non-Simple Command.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

nodakai commented 4 years ago

Googling the error message took me here. Isn't it || true rather than && true ?

CptTZ commented 4 years ago

Googling the error message took me here. Isn't it || true rather than && true ?

Yes it should be || true.

CptTZ commented 4 years ago

Closing as we are currently going through a internal process for this change

yoshifumi-kinoshita commented 4 years ago

Googling the error message took me here. Isn't it || true rather than && true ?

NO. If you add || true, the code never fails, it always succeeds. && true is intentional. Adding useless&& true is for cancelling set -e temporarily.

|| true works as well for cancelling set -e, but it clears the error code like below. && true is better in general for cancelling set -e.

#!/bin/bash

set -e

/usr/bin/false || true
echo "Error code: $?"

/usr/bin/false && true
echo "Error code: $?"

$ ./test.sh 
Error code: 0
Error code: 1

HughWarrington commented 4 years ago

set -e is a feature considered by some to be more confusing than it is helpful: see e.g. The Bash FAQ, and the confused responses above. Instead of relying on The shell does not exit if the command that fails is part of the command list immediately following a while or until keyword, part of the test in an if statement, part of any command executed in a && or || list except the command following the final && or ||... (from the manual) to 'cancel' set -e, perhaps consider making it explicit with:

set +e
keys_status="$(/usr/bin/curl -s -f -m 1 -H "${IMDS_TOKEN_HEADER}" -o /dev/null -I -w %{http_code} "${IMDS}/managed-ssh-keys/active-keys/${1}/")"
set -e

ulidtko commented 1 year ago

Why does it query the non-existing endpoint http://169.254.169.254/latest/meta-data/managed-ssh-keys/active-keys/{} in the first place?

That endpoint simply returns 404. Even when queried with the X-aws-ec2-metadata-token (which BTW looks completely useless to me, but that's beside the point):

$ curl http://169.254.169.254/latest/meta-data/managed-ssh-keys/active-keys/ubuntu -H "X-aws-ec2-metadata-token: $TOK"
<?xml version="1.0" encoding="iso-8859-1"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
                 "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
 <head>
  <title>404 - Not Found</title>
 </head>
 <body>
  <h1>404 - Not Found</h1>
 </body>
</html>

managed-ssh-keys is also nowhere to be found on the documented API of IMDS.

@CptTZ any ETAs for that "internal process" of yours?.. #20 keeps happening.