Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt agama-live to work with the new architecture #1119

Merged
merged 27 commits into from Apr 4, 2024

Conversation

imobachgs
Copy link
Member

@imobachgs imobachgs commented Mar 26, 2024

Changes summary

  • Adds a --generate-token option to agama-web-server. Using such a token you can skip the authentication (given that you have access to the token).
  • Adds a new Firefox start-up script that injects the token, skipping the authentication when running locally.
  • Defines an agama-web-server systemd service.
  • Temporarily disables the no-unused-vars linter.
  • Fixes agama-web-ui.spec.
  • Adjusts the KIWI definition.

To do

  • Replace the sqlite3 hack with support in the backend.
  • Use standard HTTP and HTTPS ports.
  • Add uncompressed version of the files to the package.

agama-live sources in the repo

In the future, we plan to move the source we use to build agama-live to this repository.

@coveralls
Copy link

coveralls commented Mar 26, 2024

Coverage Status

coverage: 71.896% (-0.09%) from 71.981%
when pulling 86afb33 on agama-live-update
into 11cda28 on architecture_2024.

live/startup Outdated
VALUE=$(cat $TOKEN_FILE)
EXPIRATION=$(date --date '1 day' +%s)

sqlite3 $COOKIES_DB "INSERT INTO moz_cookies \
Copy link
Member

@lslezak lslezak Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like hacking the internal Firefox cookies database directly. That's very fragile and works only with Firefox.

What about creating a /run/agama/token.html file with root-only permissions with content like

<!DOCTYPE html>
<html>
<body>
  <script>
    document.location = "http://localhost/login?token=" + <JS-escaped-token-value>;
  </script>
</body>
</html>

and implement /login endpoint which would set the agamaToken cookie and redirect to /.

And then use that file in the firefox command below like

firefox --kiosk --profile $HOME/.mozilla/firefox/profile /run/agama/token.html

That special file would avoid exposing the token in the process list (in contrast when using the token directly in the firefox command).

The advantage would not hacking the SQLite internals and would be generic for all browsers.

Note: Originally I wanted to put something like

 document.cookie = "agamaToken=" + tokenValue + ";domain=localhost;path=/api";

into that HTML file. But for security reasons it is not possible to set cookie for a different domain and it is not possible to set an httpOnly cookie...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, I am open to other approaches and I see your point. However bear in mind that we only need to support Firefox and, if something changes, we can detect soon enough in our CI.

To be honest, I would rather prefer some kind of API to import the cookie and do not have to extend the backend (and add a redirection). But I am not against this solution.

Copy link
Member Author

@imobachgs imobachgs Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about your proposal we can get rid of the token.html file. The startup script would be the responsible for reading and injecting the token in the URL.

In the server side, the following code seems to work:

pub async fn get_cookie(
    State(state): State<ServiceState>,
    Query(params): Query<GetCookieParams>,
) -> impl IntoResponse {
    let decoding = DecodingKey::from_secret(state.config.jwt_secret.as_ref());
    let mut headers = HeaderMap::new();

    if jsonwebtoken::decode::<TokenClaims>(&params.token, &decoding, &Validation::default()).is_ok()
    {
        let cookie = format!("agamaToken={}; HttpOnly", params.token);
        headers.insert(
            SET_COOKIE,
            cookie.parse().expect("could not build a valid cookie"),
        );
    }

    headers.insert(LOCATION, "/".parse().unwrap());
    (StatusCode::PERMANENT_REDIRECT, headers)
}

If it is OK for you, I will clean-up the code a bit (removing some duplication) and update the PR.

@imobachgs
Copy link
Member Author

imobachgs commented Apr 3, 2024

I have update the PR to follow @lslezak's approach (thanks!) removing the ugly sqlite3 hack. About automating this process, we already have a card for it: https://trello.com/c/1G6TeRzy/337-agama-put-agama-live-sources-in-a-github-repository. Perhaps it is something to plan for in the following sprint.

Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor questions and suggestions...

rust/agama-server/src/web/http.rs Outdated Show resolved Hide resolved
rust/agama-server/src/web/http.rs Outdated Show resolved Hide resolved
rust/agama-server/src/web/http.rs Outdated Show resolved Hide resolved
live/startup Outdated
TOKEN_FILE=/run/agama/token
TOKEN=$(cat $TOKEN_FILE)

firefox --kiosk --profile $HOME/.mozilla/firefox/profile "http://localhost/login?token=$TOKEN"
Copy link
Member

@lslezak lslezak Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is simpler without using a helper token.html file. On the other hand the token is then visible in the ps output. I'm not sure how serious is this problem... 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you might need to access to the system (as root, as it is the only user in the ISO) and, at that point, you can read the token from the file anyway. Alternatively, we could set the URL as the homepage for the browser (we are doing other tweaks anyway), so we do not need to specify any URL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, once you are root you can do everything...

The problem is that for running simple ps you do not need to be root. If e.g. the avahi daemon running as avahi user is vulnerable and allows arbitrary code execution then you could still get the root token if you force it to run ps and get the output although it's not running as root.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW setting the home page looks like a good trick...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, let's give the homepage setting a try.

Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks, that looks much better than the SQLite hack! 👍

@imobachgs imobachgs merged commit 94c46bc into architecture_2024 Apr 4, 2024
3 checks passed
@imobachgs imobachgs deleted the agama-live-update branch April 4, 2024 12:37
@imobachgs
Copy link
Member Author

Yes, you are right. Now the next step is to submit all this stuff automatically. But that's another story. 😄

imobachgs added a commit that referenced this pull request May 6, 2024
After a few months of work, it is time to merge the `architecture_2024`
branch into `master`. It is still a work-in-progress, but all the
efforts should be go now against that branch.

## Pull requests

* #1061
* #1064
* #1073
* #1074
* #1080
* #1089
* #1091
* #1092
* #1094
* #1095
* #1099
* #1100
* #1102
* #1103
* #1112
* #1114
* #1116
* #1117
* #1119
* #1120
* #1123
* #1126
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1136
* #1139
* #1140
* #1143
* #1146

## Other commits

* 8efa41f
* 9e2dec0
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants